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: StepFormDialogコンポーネントの作成 #5004

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

schktjm
Copy link
Contributor

@schktjm schktjm commented Oct 11, 2024

関連URL

https://app.asana.com/0/1206535203416259/1208178713972396/f
※ アクセシビリティ本部のAsanaです!

概要

StepFormDialog の実装をしました。

目的

  • 数ステップを持つDialogのコンポーネントをライブラリとして提供する
    • キーボード操作をはじめとしたアクセシビリティ対応をライブラリとして提供する
      • 次へや前へを押したときに、フォーカス位置がダイアログトップに戻っている

使用イメージ

const onSubmit = (onClose, e, currentStep) => {
    // フォームの中身の処理をする
    // { id: string; stepNumber: number;}
    return nextStep 
}

<StepFormDialog onSubmit={onSubmit}>
    <StepFormDialogItem id="step-1" stepNum={1}>{ステップ1の内容}</StepFormDialogItem> 
    <StepFormDialogItem id="step-2" stepNum={2}>{ステップ2の内容}</StepFormDialogItem>
    <StepFormDialogItem id="step-3" stepNum={3}>{ステップ3の内容}</StepFormDialogItem>
</StepFormDialog>

変更内容

  • StepFormDialog, StepFormDialogItem の作成
    • 進む, 戻るを押した際にフォーカス位置をトップへ戻す実装の追加
    • ステップを飛ばせる処理の追加
  • テストの実装

確認方法

Copy link

pkg-pr-new bot commented Oct 15, 2024

Open in Stackblitz

npm i https://pkg.pr.new/kufu/smarthr-ui@5004

commit: f2622cf

@schktjm schktjm marked this pull request as ready for review October 15, 2024 12:54
@schktjm schktjm requested a review from a team as a code owner October 15, 2024 12:54
@schktjm schktjm requested review from s-sasaki-0529 and uknmr and removed request for a team October 15, 2024 12:54
@schktjm schktjm self-assigned this Oct 15, 2024
@schktjm schktjm requested a review from masuP9 October 15, 2024 12:54
@schktjm schktjm marked this pull request as draft October 24, 2024 06:55
Copy link
Contributor Author

Choose a reason for hiding this comment

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

次へボタンの追加など以外はほとんど FormDialog と同じ構成になってます

@schktjm schktjm changed the title feat: Step 付き FormDialog の新規作成(FormDialogの拡張ではないバージョン) feat: StepFormDialogの作成 Dec 23, 2024
@schktjm schktjm changed the title feat: StepFormDialogの作成 feat: StepFormDialogコンポーネントの作成 Dec 23, 2024
@schktjm schktjm marked this pull request as ready for review December 23, 2024 05:34
@schktjm schktjm requested review from daiHash and removed request for daiHash December 23, 2024 05:48
await act(() => userEvent.click(screen.getByRole('button', { name: '戻る' })))
expect(screen.getByRole('dialog', { name: 'StepFormDialog 1/2' })).toBeVisible()

await act(() => userEvent.click(screen.getByRole('button', { name: '次へ' })))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

HELP

今は通っているんですが、この次へを押した次に 2/2 ステップ目かの assertion を入れるとCI上でだけ落ちる事件が発生していました。。。原因わからずです

if (!props.isOpen) {
return undefined
}
focusTrapRef.current?.focus()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ここでは FormDialog.tsx とほぼ構成を変えず、この focusTrap の操作を追加しています!

Comment on lines +126 to +129
const actionText =
activeStep === stepLength
? submitLabel
: decorators?.nextButtonLabel?.(NEXT_BUTTON_LABEL) || NEXT_BUTTON_LABEL
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FormDialog では actionLabel はpropsで必須で、それ以外は decorators で変更という方針でした!
基本 次へ ボタンのラベルのバリエーションを増やすことは無いと考え、次へは decorators で渡すという方法にしています!

@schktjm schktjm requested review from maiha2, daiHash, a team, oti and Qs-F and removed request for a team December 23, 2024 09:20
{/* eslint-disable-next-line smarthr/best-practice-for-layouts */}
<Stack gap={0} className={wrapper()}>
<DialogHeader
title={`${title} ${activeStep}/${stepLength}`}
Copy link
Contributor

@Qs-F Qs-F Dec 25, 2024

Choose a reason for hiding this comment

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

単純な興味なんですが、このあたり読み上げ問題なさそうですか?分数が今どうなってたか追えておらず… 🙇

Copy link
Contributor Author

Choose a reason for hiding this comment

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

日本語としての読み上げなら「◯ぶんの◯」となるのが一番いいところなんですが、「◯スラッシュ◯」となるのは一般的な記述かつ文脈的に判断できるところなので今回はこれで問題なしとしてますー!

freeeさんもこんなQnAを出してましたー! スラッシュ(/)を含む数字はどのように読み上げられるべきか — freeeアクセシビリティー・ガイドライン Ver. 202411.0-RELEASE+5.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants