Skip to content

feat: TOTP/HOTP 検証モードに Cmd/Ctrl+Enter ショートカットを追加 - #423

Merged
fumtas1k merged 2 commits into
developfrom
feat/totp-hotp-keyboard-shortcut
May 13, 2026
Merged

feat: TOTP/HOTP 検証モードに Cmd/Ctrl+Enter ショートカットを追加#423
fumtas1k merged 2 commits into
developfrom
feat/totp-hotp-keyboard-shortcut

Conversation

@fumtas1k

Copy link
Copy Markdown
Owner

概要

TOTP/HOTP ジェネレータの検証モードで、検証コード入力中に Cmd/Ctrl + Enter で「検証する」ボタンを発火できるようにします。

設定ファイル相互変換 (ConfigConverter.tsx:233-240) と同じパターンで実装。

変更

  • verify-code-inputInputFieldonKeyDown を追加
    • e.nativeEvent.isComposing 中は IME 確定 Enter を誤検知しないため skip
    • disabled と同条件 guard(input 空 / verifying 中 / secret 無効)
  • 「検証する」ActionButtonaria-keyshortcuts="Meta+Enter Control+Enter" を付与
  • E2E 2 件追加:
    • 陰性対照: 入力済 + Cmd+Enter → 検証結果が表示される
    • 陽性対照: 空入力 + Cmd+Enter → 結果が表示されない(disabled guard 検証)

テスト

  • node_modules/.bin/astro check — 0 errors
  • npm run test:e2e --grep TOTP — 12 passed (元 10 + 新 2)

🤖 Generated with Claude Code

設定ファイル相互変換 (ConfigConverter) と同じパターンで、検証コード入力欄に
キーボードショートカットを追加。マウスへの持ち替えなく検証を発火できる。

- `verify-code-input` の `onKeyDown` で `(Meta|Control) + Enter` を検出
- `isComposing` 中は IME 確定 Enter を誤検知しないため skip
- disabled 条件と同 guard(input 空 / verifying 中 / secret 無効)
- 「検証する」ボタンに `aria-keyshortcuts="Meta+Enter Control+Enter"` を付与
- E2E: 発火する陰性対照 + 空入力では発火しない陽性対照を併設

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@fumtas1k fumtas1k self-assigned this May 13, 2026
@github-actions

github-actions Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

🖼️ Visual Regression Test 結果

  • Status: ✅ 全 40 件 pass
  • Workflow run: 25796262037
  • 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

Copy link
Copy Markdown
Owner Author

レビュー — 多角的評価

ConfigConverter.tsx:233-248 のパターンを忠実に踏襲しており、設計判断としては妥当。以下の観点で確認した。

✅ 良い点

  • IME 衝突回避: e.nativeEvent.isComposing で IME 確定 Enter を skip。日本語入力時の誤発火を正しく防止。
  • 多層 guard: keyboard handler 側の !verificationInput.trim() || verifying || !secretBytesActionButtondisabled/loading ガード(ActionButton.tsx:50isDisabled = disabled || loading)+ handleVerify 自体の if (!secretBytes) return;TotpHotpGenerator.tsx:164)と重ねた defense in depth。
  • A11y: aria-keyshortcuts="Meta+Enter Control+Enter" は WAI-ARIA 仕様の空白区切り表記に準拠。
  • 陽性対照: 空入力での非発火テストが入っており、disabled guard を外したら確実に CI で落ちる構造。test-gates 規約に沿っている。
  • CSP 適合: withProductionCsp 経由で本番ヘッダ下での E2E。新規 inline handler / eval を導入していないので CSP 違反リスクなし。
  • セキュリティ: secret は Uint8ArraysecretBytes)に閉じており、ショートカット経路でも DOM / log に追加露出なし。検証入力は trim() のみで verifyTotp に渡すため新規 injection 面なし。

🟡 改善提案

1. <kbd> 可視ヒントが欠落(UX 整合性)

ConfigConverter 側は button の隣に視覚的キーヒントを出している(ConfigConverter.tsx:252-254):

<kbd className="caption text-muted font-mono" aria-hidden="true">
  Cmd/Ctrl+Enter
</kbd>

TOTP 検証側にも同じ要素を追加すると、aria-keyshortcuts(SR 向け)と視覚ユーザー向け開示が揃う。「同パターンで実装」と PR 本文に書いてあるので、可視ヒントだけ抜けているのは意図せぬ漏れに見える。

2. 2 件目の E2E が race を起こしうる

const verifyResult = page.locator('[aria-live="assertive"]');
await expect(verifyResult).toHaveCount(0);

press('Meta+Enter') 直後に toHaveCount(0) を assert しているが、Playwright の auto-retry は要素存在を待つので「存在しないこと」の検証では即時評価となり、もし guard を外して handleVerify が async に走り出した場合、setVerificationResult(null)await verifyTotpsetVerificationResult(result) の前段で toHaveCount(0) が通って false negative になる懸念。

堅牢化案(軽微):

await page.waitForTimeout(200);
await expect(verifyResult).toHaveCount(0);

または「ボタンが verifying 状態にならないこと」(aria-busy 不出現)を追加 assert。

3. e.preventDefault() が guard 通過後にしか呼ばれない

guard で early-return すると preventDefault をスキップする。現状 <input><form> で包まれていないので副作用ゼロだが、将来 form 化したときに「空入力 Cmd+Enter で submit に流れる」silent regression を生む。Enter + (meta|ctrl) を検出した時点で先に preventDefault を呼ぶ書き方のほうが安全:

if (e.key === 'Enter' && (e.metaKey || e.ctrlKey)) {
  e.preventDefault();
  if (!verificationInput.trim() || verifying || !secretBytes) return;
  handleVerify();
}

ConfigConverter も同じ順序になっているので、両方併せて follow-up issue 化でも可。

🟢 任意(スコープ外候補)

  • button だけ click したときの silent no-op: disabled={!verificationInput.trim()}secretBytes を見ないため、入力ありで base32 が無効な場合 button click が可能 → handleVerifyif (!secretBytes) return; で sink。UX としては disabled に揃えるか secret エラーを先に出すと一貫するが本 PR スコープ外。

結論

Approve 相当。指摘 1(<kbd> 可視ヒント)は parity の点で本 PR で巻き取り推奨(~3 行)、指摘 2・3 は本 PR か follow-up issue かは判断にお任せ。

PR #423 レビュー指摘を本 PR で巻き取り。

- 指摘 1: ConfigConverter と同じ `<kbd>Cmd/Ctrl+Enter</kbd>` 可視ヒントを
  「検証する」ボタンの隣に追加(aria-keyshortcuts と視覚ユーザー向け開示が揃う)
- 指摘 2: 陽性対照 E2E の `toHaveCount(0)` 即時評価による false negative
  リスクを `waitForTimeout(300)` で堅牢化
- 指摘 3: `e.preventDefault()` を guard より前に移動(将来 <form> 化時の
  silent regression 防止)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@fumtas1k

Copy link
Copy Markdown
Owner Author

レビューありがとうございます。指摘 1〜3 を本 PR で巻き取りました(commit 26693a5)。

指摘 対応
1. <kbd> 可視ヒント ✅ ConfigConverter と同じ <kbd>Cmd/Ctrl+Enter</kbd> をボタン隣に追加。Playwright 実機で表示確認済み
2. E2E race waitForTimeout(300)toHaveCount(0) 前に待機、false negative リスクを排除
3. preventDefault 順序 ✅ TOTP 側を guard より前に移動。ConfigConverter 側の同問題は #424 で follow-up
任意(button disabled が secretBytes 未参照) スコープ外として本 PR では対応せず

動作確認

  • node_modules/.bin/astro check: 0 errors
  • npm run test:e2e --grep TOTP: 12 passed
  • Playwright 実機: 「検証する」ボタン右に Cmd/Ctrl+Enter 表示を確認

@fumtas1k

Copy link
Copy Markdown
Owner Author

再レビュー(commit 26693a5

3 つの指摘の対応を確認しました。

指摘 反映内容 評価
1. <kbd> 可視ヒント <div className="flex items-center gap-3"> でラップして ConfigConverter と同じ構造(aria-hidden="true"caption text-muted font-mono)で追加(TotpHotpGenerator.tsx:402-415 ✅ parity 達成
2. E2E race waitForTimeout(300)toHaveCount(0) 前に挿入(300ms は私の提案 200ms より保守的でむしろ良い) ✅ false negative リスク解消
3. preventDefault 順序 guard より前に移動 + inline コメントで「将来 <form> 化時の silent regression 防止」という非自明な意図を残置(TotpHotpGenerator.tsx:394-395 ✅ 隠れた invariant を明文化した良いコメント

ConfigConverter 側の同問題が #424 として follow-up 切られている点も、規約「先送りは必ず issue 化」に沿っていて良好。

追加観点(差分の二次的な質)

  • <kbd> の a11y: aria-hidden="true" で SR には伝えず、aria-keyshortcuts をボタンに付与した二重チャネル(視覚 / SR)が成立。WAI-ARIA の推奨パターン。
  • コメントの粒度: // 将来 <form> 化したとき... は CLAUDE.md の「WHY が非自明な場合のみコメント」基準を満たす(隠れた invariant の文書化)。ノイズになっていない。
  • CI: test ✅ / visual-regression ✅ / e2e ⏳ pending。e2e green 確認後に review approve → merge 待ち。

結論

LGTM。指摘事項は全件解消、新規の懸念なし。e2e の green を待って approve / merge して問題ない状態です。

@fumtas1k
fumtas1k merged commit b120de2 into develop May 13, 2026
3 checks passed
@fumtas1k
fumtas1k deleted the feat/totp-hotp-keyboard-shortcut branch May 13, 2026 11:36
fumtas1k added a commit that referenced this pull request May 13, 2026
release: develop → main (#422 #423 #425 TOTP/HOTP ジェネレータ追加)
fumtas1k added a commit that referenced this pull request May 17, 2026
…rd 前に移動 (#446)

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

将来 <form> で包まれた場合に「未入力 Cmd+Enter で form submit に流れる」
silent regression を予防 (PR #423 の TOTP 側と同様のパターン)。

陽性対照 E2E として、スキーマ未入力時に Cmd/Ctrl+Enter を押下しても
検証が発火しないことを assert (guard が効くことを確認)。

Closes #424

* test(config-converter): waitForTimeout を toHaveCount の timeout 統合に置換

レビュー指摘対応: 固定 300ms wait + 2 件の toHaveCount を
regex で統合した toHaveCount({ timeout: 500 }) に置換し、
Playwright auto-wait に統一。assert セマンティクス (guard 陽性対照) は
保持: guard 削除時に検証結果メッセージが現れ test fail する。

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

1 participant