From 0892a25dd866468f8ac5a90089ca63517c33f256 Mon Sep 17 00:00:00 2001 From: dwebxr Date: Thu, 24 Sep 2026 23:05:00 +0900 Subject: [PATCH] =?UTF-8?q?fix(ui):=20=E5=85=B1=E9=80=9A=20modal=20?= =?UTF-8?q?=E3=81=8C=E8=A6=AA=E3=81=AE=E5=86=8D=E6=8F=8F=E7=94=BB=E3=81=94?= =?UTF-8?q?=E3=81=A8=E3=81=AB=20dialog=20=E3=81=B8=20focus=20=E3=82=92?= =?UTF-8?q?=E6=88=BB=E3=81=99=E3=81=AE=E3=82=92=20modal=20=E5=81=B4?= =?UTF-8?q?=E3=81=A7=E7=9B=B4=E3=81=97=E3=80=81=E9=96=89=E3=81=98=E3=81=9F?= =?UTF-8?q?=E3=82=89=E5=85=83=E3=81=AE=E8=A6=81=E7=B4=A0=E3=81=AB=E6=88=BB?= =?UTF-8?q?=E3=81=99=20(B-R11e)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 第 6 回レビュー B-R11d のレビューで判明。QrPreviewModal (QR 生成・レジの QR modal) と CsvPassModal の focus effect が inline の onClose の識別に依存し、親の再描画のたびに dialog へ focus を戻していた。onClose は latest-ref に持ち、focus effect は open の変化だけで動かす (開いたときの focus と Escape は従来どおり)。閉じるときは、focus が body / null / dialog 内に あるときだけ開いた要素へ戻す (overlay の裏の入力欄へ移した focus は奪わない)。 focus trap と SuccessOverlay の同種の問題は B-R11f。 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01HeizmagJBgL5peL5mQpxkc --- components/CsvPassModal.tsx | 28 +++++-- components/QrPreviewModal.tsx | 30 ++++++-- tests/components/CsvPassModal.test.tsx | 94 ++++++++++++++++++++++++ tests/components/QrPreviewModal.test.tsx | 53 ++++++++++++- tests/components/RegisterMode.test.tsx | 42 +++++++++++ 5 files changed, 232 insertions(+), 15 deletions(-) create mode 100644 tests/components/CsvPassModal.test.tsx diff --git a/components/CsvPassModal.tsx b/components/CsvPassModal.tsx index 8ec0a2af..bd41d450 100644 --- a/components/CsvPassModal.tsx +++ b/components/CsvPassModal.tsx @@ -21,17 +21,35 @@ export function CsvPassModal({ }) { const t = useTranslations('CsvPass'); const dialogRef = useRef(null); + // inline onClose の更新で focus effect を再実行せず、ESC は最新のハンドラを読む。 + const onCloseRef = useRef(onClose); + onCloseRef.current = onClose; - // ESC で閉じる + dialog に focus (a11y・QrPreviewModal と同様)。 + // 開く時だけ focus を移し、close / unmount 時に dialog 側に残っていれば元の要素へ戻す。 + // TODO(B-R11f): focus trap は別 PR で扱う。 useEffect(() => { if (!open) return; + const previousFocus = document.activeElement; + // cleanup 時は ref が detach 済み (StrictMode の擬似 unmount も含む) なので node を捕捉する。 + const dialog = dialogRef.current; function onKey(e: KeyboardEvent) { - if (e.key === 'Escape') onClose(); + if (e.key === 'Escape') onCloseRef.current(); } window.addEventListener('keydown', onKey); - dialogRef.current?.focus(); - return () => window.removeEventListener('keydown', onKey); - }, [open, onClose]); + dialog?.focus(); + return () => { + window.removeEventListener('keydown', onKey); + // trap が無いので、利用者が背後の入力欄などへ移した focus を opener へ奪わない。 + // dialog 除去で body に落ちた場合と、StrictMode の擬似 cleanup で dialog 内に残る場合は戻す。 + const active = document.activeElement; + if ( + previousFocus instanceof HTMLElement && + (!active || active === document.body || dialog?.contains(active)) + ) { + previousFocus.focus(); + } + }; + }, [open]); if (!open) return null; diff --git a/components/QrPreviewModal.tsx b/components/QrPreviewModal.tsx index 6b79c3c3..6a0e3d81 100644 --- a/components/QrPreviewModal.tsx +++ b/components/QrPreviewModal.tsx @@ -3,7 +3,7 @@ // 決済用 QR を全画面モーダルで提示するためのコンポーネント (決済QR / レジ 共通)。 // 店員が金額の誤入力を確認した上で、お客様にスマホ/タブレット画面を見せやすく // するのが狙い。入力画面では即時に QR を出さず「QRコードを表示する」ボタン経由で -// 開く。a11y / ESC / focus の作法は SuccessOverlay に倣う。 +// 開く。ESC は LinkQrModal と同様に最新の onClose を ref 経由で読み、focus は開閉時に管理する。 // // 状態は持たず props で受ける (labels-as-props)。印刷はポスター部に print: クラスを // 持たせ、モーダルの chrome (header / ボタン) は print:hidden で隠す。 @@ -116,17 +116,35 @@ export function QrPreviewModal({ paymentStatus?: QrPreviewPaymentStatus; }) { const dialogRef = useRef(null); + // inline onClose の更新で focus effect を再実行せず、ESC は最新のハンドラを読む。 + const onCloseRef = useRef(onClose); + onCloseRef.current = onClose; - // ESC で閉じる + dialog に focus (a11y・SuccessOverlay と同様)。 + // 開く時だけ focus を移し、close / unmount 時に dialog 側に残っていれば元の要素へ戻す。 + // TODO(B-R11f): focus trap は別 PR で扱う。 useEffect(() => { if (!open) return; + const previousFocus = document.activeElement; + // cleanup 時は ref が detach 済み (StrictMode の擬似 unmount も含む) なので node を捕捉する。 + const dialog = dialogRef.current; function onKey(e: KeyboardEvent) { - if (e.key === 'Escape') onClose(); + if (e.key === 'Escape') onCloseRef.current(); } window.addEventListener('keydown', onKey); - dialogRef.current?.focus(); - return () => window.removeEventListener('keydown', onKey); - }, [open, onClose]); + dialog?.focus(); + return () => { + window.removeEventListener('keydown', onKey); + // trap が無いので、利用者が背後の入力欄などへ移した focus を opener へ奪わない。 + // dialog 除去で body に落ちた場合と、StrictMode の擬似 cleanup で dialog 内に残る場合は戻す。 + const active = document.activeElement; + if ( + previousFocus instanceof HTMLElement && + (!active || active === document.body || dialog?.contains(active)) + ) { + previousFocus.focus(); + } + }; + }, [open]); if (!open) return null; diff --git a/tests/components/CsvPassModal.test.tsx b/tests/components/CsvPassModal.test.tsx new file mode 100644 index 00000000..02235091 --- /dev/null +++ b/tests/components/CsvPassModal.test.tsx @@ -0,0 +1,94 @@ +import { describe, expect, it, vi } from 'vitest'; +import { screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { useState } from 'react'; +import { renderWithIntl as render } from '../_helpers/i18n'; +import { CsvPassModal } from '@/components/CsvPassModal'; +import ja from '@/messages/ja.json'; + +// 購入処理は境界で置換し、本物の modal の focus / close を検証する。 +vi.mock('@/components/CsvPassPaywall', () => ({ + CsvPassPaywall: () =>

CSV pass content

, +})); + +function Parent({ onClose }: { onClose: (value: string) => void }) { + const [open, setOpen] = useState(false); + const [value, setValue] = useState(''); + return ( + <> + + + { + onClose(value); + setOpen(false); + }} /> + + ); +} + +describe('CsvPassModal focus (B-R11e)', () => { + it('親の再描画で外側の入力から focus を奪わず、Escape は最新の onClose を使う', async () => { + const user = userEvent.setup(); + const onClose = vi.fn(); + render(); + const opener = screen.getByRole('button', { name: 'Open CSV pass' }); + await user.click(opener); + const dialog = screen.getByRole('dialog', { name: ja.CsvPass.modalTitle }); + expect(dialog).toHaveFocus(); + + const input = screen.getByRole('textbox', { name: 'Outside input' }); + expect(dialog).not.toContainElement(input); + await user.type(input, 'AB'); + expect(input).toHaveFocus(); + expect(input).toHaveValue('AB'); + expect(screen.getByRole('dialog')).toBe(dialog); + + await user.keyboard('{Escape}'); + expect(onClose).toHaveBeenCalledTimes(1); + expect(onClose).toHaveBeenCalledWith('AB'); + expect(screen.queryByRole('dialog')).toBeNull(); + expect(input).toHaveFocus(); + await user.keyboard('{Escape}'); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('close ボタンは最新の onClose を使い、閉じるたびに起点へ focus を戻す', async () => { + const user = userEvent.setup(); + const onClose = vi.fn(); + render(, { reactStrictMode: true }); + const opener = screen.getByRole('button', { name: 'Open CSV pass' }); + await user.click(opener); + await user.type(screen.getByRole('textbox', { name: 'Outside input' }), 'A'); + await user.click(screen.getByRole('button', { name: ja.CsvPass.close })); + expect(onClose).toHaveBeenCalledTimes(1); + expect(onClose).toHaveBeenCalledWith('A'); + expect(screen.queryByRole('dialog')).toBeNull(); + expect(opener).toHaveFocus(); + + await user.click(opener); + expect(screen.getByRole('dialog')).toHaveFocus(); + await user.keyboard('{Escape}'); + expect(onClose).toHaveBeenCalledTimes(2); + expect(opener).toHaveFocus(); + }); + + it('root StrictMode でも Escape は 1 回だけ処理し、unmount で focus を戻して listener を解除する', async () => { + const user = userEvent.setup(); + render(); + const opener = screen.getByRole('button', { name: 'Opener' }); + opener.focus(); + const onClose = vi.fn(); + // Provider より外側の StrictMode で、open 時の effect の setup / cleanup を再実行する。 + const { unmount } = render(, { reactStrictMode: true }); + expect(screen.getByRole('dialog')).toHaveFocus(); + await user.keyboard('{Escape}'); + expect(onClose).toHaveBeenCalledTimes(1); + unmount(); + expect(opener).toHaveFocus(); + await user.keyboard('{Escape}'); + expect(onClose).toHaveBeenCalledTimes(1); + }); +}); diff --git a/tests/components/QrPreviewModal.test.tsx b/tests/components/QrPreviewModal.test.tsx index 3e05c9d3..f4d5422d 100644 --- a/tests/components/QrPreviewModal.test.tsx +++ b/tests/components/QrPreviewModal.test.tsx @@ -16,7 +16,7 @@ const LABELS = { downloadPng: 'PNG保存', }; -function renderModal(overrides: Record = {}) { +function renderModal(overrides: Partial[0]> = {}) { const props = { open: true, onClose: vi.fn(), @@ -36,9 +36,7 @@ function renderModal(overrides: Record = {}) { onDownloadPng: vi.fn(), ...overrides, }; - const utils = render( - [0])} />, - ); + const utils = render(); return { ...utils, props }; } @@ -89,6 +87,53 @@ describe('QrPreviewModal', () => { expect(props.onClose).toHaveBeenCalledTimes(2); }); + it('B-R11e: onClose の更新で focus を奪わず Escape / close は最新のハンドラを呼ぶ', async () => { + const user = userEvent.setup(); + render(); + const { props, rerender } = renderModal(); + const dialog = screen.getByRole('dialog'); + expect(dialog).toHaveFocus(); + const input = screen.getByRole('textbox', { name: 'Outside input' }); + input.focus(); + const onClose = vi.fn(); + rerender(); + expect(input).toHaveFocus(); + expect(screen.getByRole('dialog')).toBe(dialog); + + await user.keyboard('{Escape}'); + expect(onClose).toHaveBeenCalledTimes(1); + await user.click(screen.getByRole('button', { name: LABELS.close })); + expect(onClose).toHaveBeenCalledTimes(2); + expect(props.onClose).not.toHaveBeenCalled(); + rerender(); + await user.keyboard('{Escape}'); + expect(onClose).toHaveBeenCalledTimes(2); + }); + + it('B-R11e: close / unmount で開く直前の focus を戻し、再表示時は起点を取り直す', async () => { + const user = userEvent.setup(); + render(<>); + const first = screen.getByRole('button', { name: 'First opener' }); + const second = screen.getByRole('button', { name: 'Second opener' }); + first.focus(); + const { props, rerender, unmount } = renderModal(); + expect(screen.getByRole('dialog')).toHaveFocus(); + await user.click(screen.getByRole('button', { name: LABELS.copy })); + const onClose = vi.fn(); + rerender(); + rerender(); + expect(screen.queryByRole('dialog')).toBeNull(); + expect(first).toHaveFocus(); + + second.focus(); + rerender(); + expect(screen.getByRole('dialog')).toHaveFocus(); + unmount(); + expect(second).toHaveFocus(); + await user.keyboard('{Escape}'); + expect(onClose).not.toHaveBeenCalled(); + }); + it('印刷/コピー/SVG/PNG ボタンが各ハンドラを呼ぶ', async () => { const user = userEvent.setup(); const { props } = renderModal(); diff --git a/tests/components/RegisterMode.test.tsx b/tests/components/RegisterMode.test.tsx index 4c837470..059cacfb 100644 --- a/tests/components/RegisterMode.test.tsx +++ b/tests/components/RegisterMode.test.tsx @@ -70,6 +70,7 @@ vi.mock('@/lib/env', async (importOriginal) => { import { RegisterMode } from '@/components/RegisterMode'; import { parseCheckoutParams } from '@/lib/url'; +import ja from '@/messages/ja.json'; const VALID = '0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913'; const QR_KEY = 'openpay:qr-settings:v2'; @@ -726,6 +727,47 @@ describe('RegisterMode', () => { await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); }); + it('B-R11e: 開いた QR の親が再描画されても外側の商品名入力から focus を奪わない', async () => { + const user = userEvent.setup(); + seedReceiver(); + render(); + await user.click(await screen.findByRole('button', { name: /コーヒー/ })); + const opener = screen.getAllByRole('button', { name: /QRコードを表示する/ })[0]; + await user.click(opener); + const dialog = screen.getByRole('dialog'); + expect(dialog).toHaveFocus(); + + // 先頭はカート行 (後続はプリセット編集欄)。 + const input = screen.getAllByRole('textbox', { name: ja.RegisterMode.productNameLabel })[0]; + expect(dialog).not.toContainElement(input); + // 実際の親 state 更新で inline onClose が変わる。focus trap はこの PR の対象外。 + await user.type(input, 'AB'); + expect(input).toHaveFocus(); + expect(input).toHaveValue('コーヒーAB'); + expect(screen.getByRole('dialog')).toBe(dialog); + + await user.keyboard('{Escape}'); + expect(screen.queryByRole('dialog')).toBeNull(); + expect(input).toHaveFocus(); + }); + + it('B-R11e: Copy URL の親 state 更新でボタンから focus を奪わず、閉じると起点へ戻す', async () => { + const user = userEvent.setup(); + seedReceiver(); + render(); + await user.click(await screen.findByRole('button', { name: /コーヒー/ })); + const opener = screen.getAllByRole('button', { name: /QRコードを表示する/ })[0]; + await user.click(opener); + const copy = screen.getByRole('button', { name: ja.RegisterMode.copyUrl }); + await user.click(copy); + expect(await screen.findByRole('button', { name: ja.RegisterMode.copied })).toBe(copy); + expect(copy).toHaveFocus(); + + await user.click(screen.getByRole('button', { name: ja.RegisterMode.qrModalClose })); + expect(screen.queryByRole('dialog')).toBeNull(); + expect(opener).toHaveFocus(); + }); + it('右サイドバーに注文サマリ (ご注文内容 + 小計/合計) を表示する', async () => { const user = userEvent.setup(); seedReceiver();