Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Step つき FormDialog の実装 #4972

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
96 changes: 96 additions & 0 deletions packages/smarthr-ui/src/components/Dialog/Dialog.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import React, { ComponentProps, useRef, useState } from 'react'
import styled, { css } from 'styled-components'

import { Button } from '../Button'
import { CheckBox } from '../CheckBox'
import { DatePicker } from '../DatePicker'
import { Fieldset } from '../Fieldset'
import { FormControl } from '../FormControl'
Expand Down Expand Up @@ -312,6 +313,101 @@ Form_Dialog.parameters = {
},
}

export const Form_Dialog_With_Step: StoryFn = () => {
const [openedDialog, setOpenedDialog] = useState<'normal' | 'opened' | null>(null)
const [value, setValue] = React.useState('Apple')
const [responseMessage, setResponseMessage] =
useState<ComponentProps<typeof ActionDialog>['responseMessage']>()
const onClickClose = () => {
setOpenedDialog(null)
setResponseMessage(undefined)
}
const onChange = (e: React.ChangeEvent<HTMLInputElement>) => setValue(e.currentTarget.name)

return (
<Cluster>
<Button
onClick={() => setOpenedDialog('normal')}
aria-haspopup="dialog"
aria-controls="dialog-form"
data-test="dialog-trigger"
>
FormDialog with Step
</Button>
<FormDialog
isOpen={openedDialog === 'normal'}
title="FormDialog"
subtitle="副題"
actionText="保存"
decorators={{ closeButtonLabel: (txt) => `cancel.(${txt})` }}
onSubmit={(closeDialog, e, step) => {
action('executed')()
setResponseMessage(undefined)
if (step && step === 2) {
closeDialog()
}
}}
Comment on lines +343 to +349
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

もやポイント1

closeDialog の実行は FormDialog の使用する側なので、使用する側で最後のページかのチェックをする必要があります!

onClickClose={onClickClose}
responseMessage={responseMessage}
id="dialog-form"
data-test="form-dialog-content"
width="40em"
steppable={true}
>
<Fieldset title="fruits" innerMargin={0.5}>
<RadioListCluster forwardedAs="ul">
<li>
<RadioButton name="Apple" checked={value === 'Apple'} onChange={onChange}>
Apple
</RadioButton>
</li>
<li>
<RadioButton name="Orange" checked={value === 'Orange'} onChange={onChange}>
Orange
</RadioButton>
</li>
<li>
<RadioButton name="Grape" checked={value === 'Grape'} onChange={onChange}>
Grape
</RadioButton>
</li>
</RadioListCluster>
</Fieldset>
<FormControl title="Sample">
<Input type="text" name="text" />
</FormControl>
<Fieldset title="fruits" innerMargin={0.5}>
<ul>
<li>
<CheckBox name="1">CheckBox</CheckBox>
</li>

<li>
<CheckBox name="error" error>
CheckBox / error
</CheckBox>
</li>

<li>
<CheckBox name="disabled" disabled>
CheckBox / disabled
</CheckBox>
</li>
</ul>
</Fieldset>
</FormDialog>
</Cluster>
)
}

Form_Dialog_With_Step.parameters = {
docs: {
description: {
story: '`FormDialog with step` is a form dialog that can be divided into multiple steps.',
},
},
}

export const Remote_Trigger_Form_Dialog: StoryFn = () => (
<>
<div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { tv } from 'tailwind-variants'
import { useHandleEscape } from '../../hooks/useHandleEscape'

import { DialogOverlap } from './DialogOverlap'
import { FocusTrap } from './FocusTrap'
import { FocusTrap, FocusTrapRef } from './FocusTrap'
import { useBodyScrollLock } from './useBodyScrollLock'

export type DialogContentInnerProps = PropsWithChildren<{
Expand Down Expand Up @@ -49,6 +49,10 @@ export type DialogContentInnerProps = PropsWithChildren<{
* ダイアログの `aria-labelledby`
*/
ariaLabelledby?: string
/**
* ダイアログトップのフォーカストラップへの ref
*/
focusTrapRef?: RefObject<FocusTrapRef>
}>
type ElementProps = Omit<ComponentProps<'div'>, keyof DialogContentInnerProps>

Expand Down Expand Up @@ -77,6 +81,7 @@ export const DialogContentInner: FC<DialogContentInnerProps & ElementProps> = ({
ariaLabelledby,
children,
className,
focusTrapRef,
...rest
}) => {
const { layoutStyleProps, innerStyle, backgroundStyle } = useMemo(() => {
Expand Down Expand Up @@ -127,7 +132,9 @@ export const DialogContentInner: FC<DialogContentInnerProps & ElementProps> = ({
aria-modal="true"
className={innerStyle}
>
<FocusTrap firstFocusTarget={firstFocusTarget}>{children}</FocusTrap>
<FocusTrap ref={focusTrapRef} firstFocusTarget={firstFocusTarget}>
{children}
</FocusTrap>
</div>
</div>
</DialogOverlap>
Expand Down
36 changes: 29 additions & 7 deletions packages/smarthr-ui/src/components/Dialog/FocusTrap.tsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,42 @@
import React, { FC, PropsWithChildren, RefObject, useCallback, useEffect, useRef } from 'react'
import React, {
PropsWithChildren,
RefObject,
forwardRef,
useCallback,
useEffect,
useImperativeHandle,
useRef,
} from 'react'

import { tabbable } from '../../libs/tabbable'

type Props = PropsWithChildren<{
firstFocusTarget?: RefObject<HTMLElement>
}>

export const FocusTrap: FC<Props> = ({ firstFocusTarget, children }) => {
const ref = useRef<HTMLDivElement | null>(null)
export type FocusTrapRef = {
focus: () => void
}

export const FocusTrap = forwardRef<FocusTrapRef, Props>(({ firstFocusTarget, children }, ref) => {
const innerRef = useRef<HTMLDivElement | null>(null)
const dummyFocusRef = useRef<HTMLDivElement>(null)

useImperativeHandle(ref, () => ({
focus: () => {
if (firstFocusTarget?.current) {
firstFocusTarget.current.focus()
} else {
dummyFocusRef.current?.focus()
}
},
}))

const handleKeyDown = useCallback((e: KeyboardEvent) => {
if (e.key !== 'Tab' || ref.current === null) {
if (e.key !== 'Tab' || innerRef.current === null) {
return
}
const tabbables = tabbable(ref.current).filter((elm) => elm.tabIndex >= 0)
const tabbables = tabbable(innerRef.current).filter((elm) => elm.tabIndex >= 0)
if (tabbables.length === 0) {
return
}
Expand Down Expand Up @@ -57,10 +79,10 @@ export const FocusTrap: FC<Props> = ({ firstFocusTarget, children }) => {
}, [firstFocusTarget])

return (
<div ref={ref}>
<div ref={innerRef}>
{/* dummy element for focus management. */}
<div ref={dummyFocusRef} tabIndex={-1} />
{children}
</div>
)
}
})
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import React, { ComponentProps, FormEvent, useCallback, useId } from 'react'
import { DialogContentInner } from '../DialogContentInner'
import { DialogProps } from '../types'
import { useDialogPortal } from '../useDialogPortal'
import { useStepDialog } from '../useStepDialog'

import { FormDialogContentInner, FormDialogContentInnerProps } from './FormDialogContentInner'

Expand All @@ -29,10 +30,20 @@ export const FormDialog: React.FC<Props & ElementProps> = ({
portalParent,
decorators,
id,
steppable,
...props
}) => {
const { createPortal } = useDialogPortal(portalParent, id)
const titleId = useId()
const {
titleSuffix,
focusTrapRef,
childrenSteps,
activeStep,
getActionText,
handleNextSteps,
renderSubActionButton,
} = useStepDialog(children)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nits]
今から根本から作り変えることになってしまいますし、今のコードでもちゃんと動いていそうなのでnits程度にとどめておくのですが、 FormDialog とは別のコンポーネントに切り出すのが良いのかなぁ...と思いました。
理由としては次の感じです。

  • コンポーネントのインターフェースがちょっと特殊
    • propsが hasStep:true の場合のみ、次のページ数 のようにステップ付きかどうかで関数に渡される引数が変わる
    • 暗黙的に各ステップとして Dialog の子コンポーネントを下記のように配置されることが利用者側に要求される
<FormDialog>
  // これは一個目のステップ
  <Hoge />
  // これは二個目のステップ
  <Fuga />
</FormDialog>
  • ステップ付きダイアログ専用のロジックが混入する
    • 特に useStepDialog のロジックも hasStep を使わない場合は不要なhooksになってしまう。

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

レビューありがとうございますー!全然アリだと思います!

定例で相談したとき、簡単に書けそうだったら FormDialog に hasStep 生やす程度でできたら嬉しいということを聞いたんですが、もやポイントに書いた通り onClose の発火タイミングを使用している側で調整したりする必要があったりで複雑そうな雰囲気があったので!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ちょっとPR別で作ってみます!


const handleClickClose = useCallback(() => {
if (!props.isOpen) {
Expand All @@ -47,9 +58,13 @@ export const FormDialog: React.FC<Props & ElementProps> = ({
return
}

onSubmit(close, e)
if (steppable) {
handleNextSteps()
}

onSubmit(close, e, activeStep)
},
[onSubmit, props.isOpen],
[onSubmit, props.isOpen, steppable, handleNextSteps, activeStep],
)

return createPortal(
Expand All @@ -58,26 +73,28 @@ export const FormDialog: React.FC<Props & ElementProps> = ({
ariaLabelledby={titleId}
className={className}
onPressEscape={onPressEscape}
focusTrapRef={focusTrapRef}
>
{/* eslint-disable-next-line smarthr/a11y-delegate-element-has-role-presentation */}
<FormDialogContentInner
title={title}
title={steppable ? `${title}${titleSuffix}` : title}
titleId={titleId}
subtitle={subtitle}
titleTag={titleTag}
contentBgColor={contentBgColor}
contentPadding={contentPadding}
actionText={actionText}
actionText={steppable ? getActionText(actionText) : actionText}
actionTheme={actionTheme}
actionDisabled={actionDisabled}
closeDisabled={closeDisabled}
subActionArea={subActionArea}
subActionArea={steppable ? renderSubActionButton() : subActionArea}
onClickClose={handleClickClose}
onSubmit={handleSubmitAction}
responseMessage={responseMessage}
decorators={decorators}
steppable={steppable}
>
{children}
{steppable ? childrenSteps[activeStep] : children}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

もやポイント2?

props 渡す箇所で三項演算子書き過ぎかもなぁと考えています!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

steppablefalse のときでも actionTextactionText={getActionText(actionText)} としても動くから、まとめてしまっても良いかも?(stepが1個しか無いとして扱える)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あ、嘘だった。

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useStepDialog に hasStep を渡してあげて、hooks 内で同じ条件をかけばいけるんですけど、通常 FormDialog の actionText ロジックをカスタムフックス内で持っちゃうとわかりにくくなっちゃうかな〜と思ってきました!

</FormDialogContentInner>
</DialogContentInner>,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,13 @@ export type BaseProps = PropsWithChildren<
/**
* アクションボタンをクリックした時に発火するコールバック関数
* @param closeDialog ダイアログを閉じる関数
* @param activeStep steppable:true の場合のみ、次のページ数
*/
onSubmit: (closeDialog: () => void, e: FormEvent<HTMLFormElement>) => void
onSubmit: (
closeDialog: () => void,
e: FormEvent<HTMLFormElement>,
activeStep?: number,
) => void
/** アクションボタンを無効にするかどうか */
actionDisabled?: boolean
/** 閉じるボタンを無効にするかどうか */
Expand All @@ -36,6 +41,8 @@ export type BaseProps = PropsWithChildren<
subActionArea?: ReactNode
/** コンポーネント内の文言を変更するための関数を設定 */
decorators?: DecoratorsType<'closeButtonLabel'>
/** Stepつきダイアログか否か */
steppable?: boolean
}
>

Expand Down
61 changes: 61 additions & 0 deletions packages/smarthr-ui/src/components/Dialog/useStepDialog.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import React, { Children, ReactNode, useCallback, useMemo, useRef, useState } from 'react'

import { Button } from '../Button'

import { FocusTrapRef } from './FocusTrap'

const NEXT_BUTTON_LABEL = '次へ'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

聞きたいポイント

次へのラベルは変えることがなさそうと思ったのですが、変更している例があれば教えてほしいです 🙏

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

多言語化などで翻訳する場合に変えるときがあります〜

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

submit のボタンテキストは props.actionText で定義されているのに対して戻るボタンは props.decorators.closeButtonLabel() で渡せるようになっているので次へテキストをどうわたせるようにするか迷っています

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

一旦一番実直な方法で書いてみました!

0c6e6ef


export const useStepDialog = (children: ReactNode) => {
const [activeStep, setActiveStep] = useState(0)
const focusTrapRef = useRef<FocusTrapRef>(null)

const childrenSteps = useMemo(() => {
const steps: ReactNode[] = []
Children.map(children, (child) => {
steps.push(child)
})
return steps
}, [children])

const getActionText = (submitActionText: ReactNode) =>
activeStep < childrenSteps.length - 1 ? NEXT_BUTTON_LABEL : submitActionText

const handleNextSteps = useCallback(() => {
if (activeStep + 1 === childrenSteps.length) {
setActiveStep(0)
return
}
focusTrapRef.current?.focus()
setActiveStep((pre) => pre + 1)
}, [activeStep, childrenSteps])

const handleBackSteps = useCallback(() => {
if (activeStep > 0) {
focusTrapRef.current?.focus()
setActiveStep((pre) => pre - 1)
}
}, [activeStep])

const renderSubActionButton = useCallback(() => {
if (activeStep === 0) {
return null
}
return <Button onClick={handleBackSteps}>戻る</Button>
}, [activeStep, handleBackSteps])

const titleSuffix = useMemo(
() => ` (${activeStep + 1}/${childrenSteps.length})`,
[activeStep, childrenSteps],
)

return {
focusTrapRef,
activeStep,
childrenSteps,
titleSuffix,
getActionText,
handleNextSteps,
renderSubActionButton,
}
}