refactor(config-converter): Cmd/Ctrl+Enter ハンドラの preventDefault を guard 前に移動

[fumtas1k]

概要

ConfigConverter の Cmd/Ctrl+Enter ハンドラで e.preventDefault() を guard より前に移動。将来 <form> で包まれた場合に「未入力 Cmd+Enter で form submit に流れる」silent regression を予防する (PR #423 の TOTP 側と同等のパターン)。

Closes #424

変更内容

 onKeyDown={(e) => {
   if (e.nativeEvent.isComposing) return;
   if (e.key === 'Enter' && (e.metaKey || e.ctrlKey)) {
-    if (!output || !schemaText || isValidating) return;
     e.preventDefault();
+    if (!output || !schemaText || isValidating) return;
     handleValidate();
   }
 }}

E2E (陽性対照)

tests/e2e/config-converter.spec.ts に以下のテストを追加:

「JSON Schema 検証パネル: スキーマ未入力で Cmd/Ctrl+Enter は検証を発火しない（陽性対照）」

• スキーマパネルを開き、スキーマ未入力のまま Cmd/Ctrl+Enter 押下
• スキーマ検証成功 / スキーマ検証失敗 のいずれも表示されないことを assert (guard が効くことを確認)

test-gates 補足

本 fix の preventDefault 順序入れ替えは 現状 <form> で包まれていないため観測可能な振る舞い差分なし。追加した test は guard 自体の陽性対照であり、preventDefault 順序の陽性対照ではない。

PR #423 (TOTP 側の同等修正) も guard 陽性対照のみ追加して <form> シナリオの synthetic test は書いていない先例に揃えた。issue #424 自体も「現状 <form> で包まれておらず実害なし。将来の form 化時の予防策」と低優先度判定。

テスト計画

• node_modules/.bin/astro check 0 errors
• 追加 E2E + 既存 Cmd/Ctrl+Enter E2E 2 件 local pass
• CI green

---

Generated by Claude Code

[github-actions]

🖼️ Visual Regression Test 結果

• Status: ✅ 全 40 件 pass
• Workflow run: 25980056309
• Artifact (diff 画像 / playwright-report): 上記 workflow run の Artifacts セクションから download

diff が 意図的な visual 変更の場合: Update Visual Regression Baseline workflow を本 PR ブランチで workflow_dispatch trigger して baseline を更新。
diff が 意図しない regression の場合: 該当変更を fix。
本 check は required ではないため fail のままでも merge は可能（reviewer 判断）。

[fumtas1k]

レビュー結果: LGTM ✅

概要

ConfigConverter の Cmd/Ctrl+Enter ハンドラで e.preventDefault() を guard の前に移動する 1 行 refactor + guard 自体の陽性対照 E2E。PR #423 (TOTP 側) と同パターンの予防的 fix。Issue #424 closes。

検証結果

1. 「観測可能な振る舞い変更なし」の主張を確認 ✅

grep -n "<form\|</form>" src/components/tools/ConfigConverter.tsx で <form> ラッパが存在しないことを確認。PR 本文の「現状 <form> で包まれていないため観測可能な差分なし」は正確。将来 form 化された際の silent regression 予防 refactor として位置付けが妥当。

2. guard 自体の陽性対照は機能 ✅

追加 E2E (tests/e2e/config-converter.spec.ts:298-322) は:

• schemaText 未入力で Cmd/Ctrl+Enter 押下
• スキーマ検証成功 / スキーマ検証失敗 の 両方 が toHaveCount(0) を assert

将来 guard (if (!output || !schemaText || isValidating) return;) が削除されると handleValidate が空 schema で呼ばれ、いずれかの結果メッセージが必ず表示される (Ajv が空 schema を成功扱いするか throw → 失敗扱いするかは別として、状態 setter は走る)。したがって guard の陽性対照として有効。

3. test-gates の例外パターン明示も妥当 ✅

PR 本文で「preventDefault 順序入れ替えの陽性対照ではない」と明示し、PR #423 の precedent に揃えている点が透明性高い。<form> ラッパが無い状態で「順序入れ替え自体の陽性対照」を書くと synthetic な <form> を test 内で組み立てる必要があり、本番コードを反映しない artificial test になるためスキップ判断は適切。

4. プロジェクト規約準拠 ✅

• Conventional Commits refactor(config-converter): 形式、日本語 PR 本文、develop ベース。
• 既存 selector パターン (getByRole('group', ...) / getByLabel(...)) と整合。
• aria-keyshortcuts="Meta+Enter Control+Enter" を含む既存実装の a11y 属性は本 PR で削除されておらず、9.6 章の a11y 保護規則に準拠。

軽微なコメント (任意 / 非ブロッキング)

await page.waitForTimeout(300) の hard-coded wait

Playwright 公式は固定 wait を非推奨としているが、「何も起きないことを assert する」用途では妥当な妥協。300ms で handleValidate の async 完了を担保しているわけではないため、将来 handleValidate が異常に遅延するケースで誤検知の理論的余地はあるが、現状の同期的 guard 早期 return では問題ない。

代替案 (任意): await expect(page.getByText(/スキーマ検証(成功|失敗)/)).toHaveCount(0, { timeout: 500 }) で 500ms 待ちつつ「現れないこと」を assert する形に統合すれば固定 wait を消せる。本 PR 必須ではない。

セキュリティ / パフォーマンス

• 影響なし。1 行入れ替えのみ、レンダリングコスト変化なし。

結論

Approve 相当。予防 refactor として最小・低リスク、test-gates 例外の説明も明示済み。CI green で merge OK。waitForTimeout 改善は任意フォロー (issue 化するほどでもない範囲)。

---

Generated by Claude Code

[fumtas1k]

再レビュー: LGTM ✅ (フィードバック反映確認)

差分確認

前回レビューの軽微コメント (waitForTimeout(300) の hard-coded wait) が反映されたことを確認:

- await page.waitForTimeout(300);
- await expect(page.getByText('スキーマ検証成功')).toHaveCount(0);
- await expect(page.getByText('スキーマ検証失敗')).toHaveCount(0);
+ // toHaveCount の timeout で「現れないこと」を確認 (固定 wait より auto-wait が確実)
+ await expect(page.getByText(/スキーマ検証(成功|失敗)/)).toHaveCount(0, { timeout: 500 });

改善点

• 固定 wait → Playwright auto-wait: 500ms 以内に成功/失敗メッセージが現れないことを assert する形に統合。handleValidate が将来何らかの理由で遅延する場合でも、500ms 以内に状態が反映されないなら誤検知ではなく実体ある regression として fail させられる。
• regex 統合: 2 つの toHaveCount(0) を /スキーマ検証(成功|失敗)/ の 1 行に集約。テスト意図 (「どちらのメッセージも出ない」) がより読みやすい。
• コメント追記: 「固定 wait より auto-wait が確実」の根拠が将来の reader にも伝わる。

mergeable_state 状態

blocked → clean に変化を確認。CI green ならそのまま merge 可能。

結論

Approve。前回指摘した軽微改善も取り込まれており、preventDefault 順序の予防 refactor + guard 陽性対照 E2E として完成度高い。merge OK。

---

Generated by Claude Code