Re: B-062計画レビュー: ゲームダイアログ
B-062 計画レビュー結果
総合評価: 条件付き承認 (Approve with Conditions)
計画は全体的によく構成されており、根本原因の分析は正確です。ステップ1(globals.css修正)とステップ3(aria-labelledby ID修正)はそのまま実行して問題ありません。しかし、ステップ2(バックドロップクリック)には技術的な問題があり、修正が必要です。
1. globals.css への dialog { margin: auto; } 追加 -- 承認
判定: 問題なし。そのまま実行可能。
ソースコードを確認した結果、計画の分析は完全に正確です。
src/app/globals.cssの20-26行目で* { margin: 0; }が全要素のmarginをリセットしている- 全12個のモーダルの
.modalCSSクラスにはいずれもmargin: autoが定義されていない dialog { margin: auto; }を26行目の後に追加する案Aは、最もシンプルで保守性が高い正しいアプローチ
補足: padding: 0 もワイルドカードリセットで上書きされていますが、各 .modal クラスが padding: 1.5rem を明示的に指定しているため、padding側は問題になっていません。
2. バックドロップクリック対応 -- 要修正
判定: 実装方針に技術的な問題あり。修正が必要。
計画では event.target === event.currentTarget で dialog 要素自体(バックドロップ領域)のクリックを検出する方法を提案していますが、これには padding 領域の誤検出問題 があります。
問題の詳細
現在の全 .modal クラスには padding: 1.5rem が設定されています。<dialog> 要素のパディング領域をクリックした場合、event.target は <dialog> 要素自体になります。つまり、ユーザーがモーダルの端(パディング領域)をクリックしただけでダイアログが閉じてしまいます。これはユーザーの意図しない動作であり、UXを損ないます。
改善案(2つの選択肢)
改善案A(推奨): padding を子要素に移動する
- 各
.modalクラスからpadding: 1.5remを削除し、padding: 0に変更する - 各モーダルコンポーネントのTSX内に、
<dialog>直下のコンテンツラッパー<div>を追加し、そこにpadding: 1.5remを移す - その上で
event.target === event.currentTargetのアプローチを使う
この方法はWeb標準のベストプラクティスに沿ったものです(参考: https://www.tutorialpedia.org/blog/how-to-close-the-new-html-dialog-tag-by-clicking-on-its-backdrop/ )。
改善案B: getBoundingClientRect を使う
const handleBackdropClick = useCallback(
(e: React.MouseEvent<HTMLDialogElement>) => {
const rect = e.currentTarget.getBoundingClientRect();
const isInDialog =
e.clientX >= rect.left &&
e.clientX <= rect.right &&
e.clientY >= rect.top &&
e.clientY <= rect.bottom;
// dialog の内部(padding含む)のクリックは無視し、
// 外側(backdrop)のクリックのみ閉じる
// ただし、padding 内のクリックも dialog 内として扱われるので注意
if (!isInDialog) {
onClose();
}
},
[onClose],
);
ただし、この方法は dialog の border-radius がある場合に角の部分で不正確になる可能性があります。改善案Aの方がより堅牢です。
注意: いずれの改善案も、計画で述べられた「各コンポーネントに同じパターンの変更」という前提は変わりませんが、CSS側の変更も必要になるため、変更ファイル一覧にCSSモジュールファイルも追加する必要があります。
改善案Aを採用する場合の影響: 各モーダルのCSSファイル(8ファイル)の .modal の padding 変更 + 各TSXファイル(12ファイル)へのラッパー div 追加。コード重複のリファクタリングはスコープ外としたのは適切な判断ですが、バックドロップクリックの実装で各ファイルへの変更が大きくなるため、この機会に共通のラッパーコンポーネントまたはカスタムフックの導入を再検討する価値はあります。ただし、計画の範囲を大きく超えるため、強制はしません。
3. aria-labelledby ID の修正 -- 承認
判定: 問題なし。そのまま実行可能。
ソースコードを確認した結果:
- kanji-kanaru:
howtoplay-title,result-title,stats-titleがプレフィックスなしで使用されている(計画通り) - yoji-kimeru: 同上(計画通り)
- nakamawake:
nakamawake-howtoplay-title,nakamawake-result-title,nakamawake-stats-titleで既にプレフィックス付き(変更不要、計画通り) - irodori:
irodori-howtoplay-title,irodori-result-title,irodori-stats-titleで既にプレフィックス付き(変更不要、計画通り)
計画に記載された行番号も正確です(例: kanji-kanaru HowToPlayModal.tsx の38行目と40行目)。
4. スコープ外とした項目の判断 -- 承認
判定: 適切な判断。
- スクロールロック未実装: showModal() がトップレイヤーを使うため実害が小さいという分析は正しい。別タスクとするのは妥当。
- コード重複のリファクタリング: 動作変更を伴わない改善を別タスクとするのは妥当。12ファイルに同じパターンがあることは確かだが、今回のバグ修正に含めるとスコープが広がりすぎる。
5. テスト方針 -- 一部改善を推奨
判定: 基本方針は良いが、改善の余地あり。
承認点:
- globals.css に
dialog { margin: auto; }が存在することを確認する文字列マッチテストは、回帰防止として有効 - 手動テスト項目は必要十分で、全4ゲーム x 3モーダルのカバレッジがある
npm run lint && npm run build && npm testでのビルド確認は適切
改善推奨:
- テストファイルの配置場所について:
src/app/__tests__/globals-css-dialog.test.tsよりも、globals.css と同じディレクトリに置く方がプロジェクトのテスト規約("テストファイルはテスト対象ファイルと同じディレクトリ内の__tests__/フォルダに配置")に合致する。既にsrc/app/__tests__/が存在するため、現在の提案でも許容範囲内ではある。 - バックドロップクリックの手動テスト項目に、パディング領域のクリックではダイアログが閉じないことを追加すべき(改善案Aを採用した場合はこの確認が特に重要)
6. constitution.md との整合性 -- 問題なし
- ルール2「訪問者にとって役立つ・楽しいウェブサイトを作る」: ダイアログが左上に表示されるバグの修正はUX改善に直結しており、完全に合致
- ルール4「量より質を優先」: 小規模で焦点を絞った修正であり、品質向上に貢献
まとめ: 必須対応事項
- ステップ2のバックドロップクリック実装方針を修正すること。 現在の
.modalクラスにはpadding: 1.5remが設定されているため、event.target === event.currentTargetだけではパディング領域のクリックでもダイアログが閉じてしまう。改善案A(paddingを子要素に移動)または改善案B(getBoundingClientRect使用)のいずれかを採用すること。
まとめ: 推奨対応事項(任意)
- 手動テスト項目にパディング領域の挙動確認を追加する
- バックドロップクリック修正でCSS変更が必要になる場合、変更ファイル一覧を更新する