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: () => '閉じる', nextButtonLabel: () => 'Next' }}
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"
hasStep={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,10 +3,21 @@ 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'

type Props = Omit<FormDialogContentInnerProps, 'titleId'> & DialogProps
type Props = Omit<FormDialogContentInnerProps, 'titleId' | 'onSubmit'> &
DialogProps & {
/**
* アクションボタンをクリックした時に発火するコールバック関数
* @param closeDialog ダイアログを閉じる関数
* @param activeStep hasStep:true の場合のみ、次のページ数
*/
onSubmit: (closeDialog: () => void, e: FormEvent<HTMLFormElement>, activeStep?: number) => void
/** Stepつきダイアログか否か */
hasStep?: boolean
}
type ElementProps = Omit<ComponentProps<'div'>, keyof Props>

export const FormDialog: React.FC<Props & ElementProps> = ({
Expand All @@ -29,10 +40,20 @@ export const FormDialog: React.FC<Props & ElementProps> = ({
portalParent,
decorators,
id,
hasStep,
...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 +68,13 @@ export const FormDialog: React.FC<Props & ElementProps> = ({
return
}

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

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

return createPortal(
Expand All @@ -58,26 +83,27 @@ 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={hasStep ? `${title}${titleSuffix}` : title}
titleId={titleId}
subtitle={subtitle}
titleTag={titleTag}
contentBgColor={contentBgColor}
contentPadding={contentPadding}
actionText={actionText}
actionText={hasStep ? getActionText(actionText, decorators?.nextButtonLabel) : actionText}
actionTheme={actionTheme}
actionDisabled={actionDisabled}
closeDisabled={closeDisabled}
subActionArea={subActionArea}
subActionArea={hasStep ? renderSubActionButton() : subActionArea}
onClickClose={handleClickClose}
onSubmit={handleSubmitAction}
responseMessage={responseMessage}
decorators={decorators}
>
{children}
{hasStep ? childrenSteps[activeStep] : children}
</FormDialogContentInner>
</DialogContentInner>,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export type BaseProps = PropsWithChildren<
/** ダイアログフッターの左端操作領域 */
subActionArea?: ReactNode
/** コンポーネント内の文言を変更するための関数を設定 */
decorators?: DecoratorsType<'closeButtonLabel'>
decorators?: DecoratorsType<'closeButtonLabel' | 'nextButtonLabel'>
}
>

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

import { Button } from '../Button'

import { FocusTrapRef } from './FocusTrap'

import type { DecoratorType } from '../../types'

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, nextButtonLabel?: DecoratorType) =>
activeStep < childrenSteps.length - 1
? nextButtonLabel
? nextButtonLabel(NEXT_BUTTON_LABEL)
: 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,
}
}