Skip to content

feat(career): 未保存離脱ガードを全ページ横断に拡張#314

Merged
yusuke0610 merged 2 commits into
mainfrom
feat/career-unsaved-guard-global
Jun 9, 2026
Merged

feat(career): 未保存離脱ガードを全ページ横断に拡張#314
yusuke0610 merged 2 commits into
mainfrom
feat/career-unsaved-guard-global

Conversation

@yusuke0610

@yusuke0610 yusuke0610 commented Jun 9, 2026

Copy link
Copy Markdown
Owner

概要

職務経歴書に未保存の変更がある状態で GitHub連携 / ブログ連携ページへ移動した後でも、× 閉じ・リロード時にブラウザ標準の離脱確認ダイアログが出るようにします。

背景・課題

直前の PR #313 で導入した beforeunload ガードは、職務経歴書フォーム(CareerResumeForm)がマウントされている間しか有効でなく、別ページ(GitHub/ブログ)へ移動するとフォームがアンマウントされてガードが外れていました。そのため「未保存のまま別ページに移動 → そこからタブを閉じる/リロード」でデータが失われても警告が出ませんでした。

変更内容

  • CareerUnsavedGuard を追加: 未保存判定をフォームのローカル state ではなく Redux formCache(in-app 遷移をまたいで保持される)から行うコンポーネント。App 直下に常設し、どのページにいても職務経歴書の未保存を監視する。描画は null のため周辺ツリーへ再描画は波及しない。
  • App.tsx: <CareerUnsavedGuard isAuthenticated={user !== null} /> を組み込み。
  • CareerResumeForm.tsx: フォーム内の useUnsavedChangesWarning 呼び出しを削除し、ガードを App 側に一本化(二重登録の回避)。
  • 対象はログイン済みのみ(未ログインは baseline が null で dirty.any が常に false =対象外。入力は sessionStorage に自動退避される)。

アプリ内ページ遷移時のカスタム確認モーダルは出しません。あくまで「このプロダクトから離脱する時(× 閉じ/リロード)のブラウザ標準ダイアログ」のみです。

検証

  • make test-frontend … 296 pass(CareerUnsavedGuard の 3 ケース追加: ログイン済み×未保存=抑止 / 未ログイン=非抑止 / 未保存なし=非抑止)
  • make lint-frontend / make lint-frontend-messages … clean
  • make build-frontend … 型エラーなし
  • E2E(npm run test:e2e)… 30 pass(既存ナビゲーションに回帰なし)
  • 手動: ログイン後 /career で編集 → GitHub連携/ブログへ移動 → タブ閉じ/リロードで標準ダイアログが出ること。保存後は出ないこと。

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Career resume unsaved changes protection now monitors edits across the entire app and prevents accidental navigation away without saving changes.
  • Tests

    • Added test coverage for unsaved changes detection in career resume.

職務経歴書に未保存があるとき、GitHub連携/ブログ連携ページに移動した後でも
× 閉じ・リロード時にブラウザ標準の離脱確認が出るようにする。

これまで beforeunload ガードは職務経歴書フォームのマウント中のみ有効で、
別ページへ移ると未保存があっても発火しなかった。判定を Redux formCache 基準に
変え、App 直下の CareerUnsavedGuard へ移設して全ページで有効化する。

- CareerUnsavedGuard を追加(formCache を購読し dirty 判定、描画は null)
- App に組み込み、CareerResumeForm 内の重複ガード呼び出しを削除
- ログイン済みのみ対象(未ログインは baseline=null で dirty.any 常に false)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@yusuke0610, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 45 minutes and 33 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3b541eed-2dcd-4d28-a892-9ad0bc8f8ffc

📥 Commits

Reviewing files that changed from the base of the PR and between fc18387 and 6f5f6c3.

📒 Files selected for processing (2)
  • frontend/src/components/CareerUnsavedGuard.test.tsx
  • frontend/src/components/CareerUnsavedGuard.tsx
📝 Walkthrough

Walkthrough

The PR refactors unsaved-changes warning behavior by extracting logic from CareerResumeForm into a new CareerUnsavedGuard component registered at the application top level, allowing it to monitor form state across all pages and conditionally trigger browser leave confirmations based on authentication and dirty state.

Changes

Career Form Unsaved Changes Guard

Layer / File(s) Summary
CareerUnsavedGuard component implementation
frontend/src/components/CareerUnsavedGuard.tsx
New component derives career form and baseline from Redux formCache, computes dirty state via useCareerDirty, and conditionally registers useUnsavedChangesWarning when authenticated and dirty; renders nothing.
CareerUnsavedGuard test coverage
frontend/src/components/CareerUnsavedGuard.test.tsx
Vitest suite with minimal Redux store validates guard prevents beforeunload default only when authenticated with unsaved changes; allows navigation when unauthenticated or form is clean.
App-level integration
frontend/src/App.tsx
Imports and renders CareerUnsavedGuard at the top level inside ErrorBoundary/LoginPromptProvider, passing isAuthenticated={user !== null} to monitor form state globally.
CareerResumeForm cleanup
frontend/src/components/forms/CareerResumeForm.tsx
Removes useUnsavedChangesWarning import and hook call; component now only computes dirty state without registering browser warnings, delegating that responsibility to the top-level guard.

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • yusuke0610/devforge#313: Implements the same unsaved beforeunload confirmation refactoring by extracting warning logic from CareerResumeForm into a separate top-level component.
  • yusuke0610/devforge#273: Modifies useCareerDirty to use the shared isDeepEqual utility, which is the dirty-state computation hook that the new CareerUnsavedGuard depends on.

Poem

🐰 A guard at the top keeps watch o'er the form,
While forms whisper changes through app's gentle norm,
No saving, no leaving—just "wait!" cries the warn,
But only for those who've logged in this morn! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is in Japanese and refers to extending an unsaved-changes guard globally across pages. It aligns with the main change (moving the guard from CareerResumeForm to a top-level App component), but uses non-English text and is somewhat vague without knowing Japanese. Consider using English for clarity in commit history and to be accessible to all team members. A more specific title like 'feat(career): Extract unsaved guard to App-level component' would better convey the refactoring intent.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/career-unsaved-guard-global

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend/src/components/CareerUnsavedGuard.tsx`:
- Around line 20-22: The code currently force-casts cache?.form and
cache?.baseline to CareerFormState which can crash if the stored unknown payload
is stale or corrupt; update the CareerUnsavedGuard to validate the cached values
before using them by adding a runtime type guard or safe parser (e.g.,
isCareerFormState(value)) and only assign form = validated value or fallback to
createInitialCareerForm(), and set baseline = validated baseline or null; ensure
useCareerDirty is always called with valid CareerFormState or null to prevent
nested-access crashes and reference the existing symbols form, baseline,
createInitialCareerForm, and useCareerDirty when implementing the checks.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 731e8704-7301-4db7-8010-e1d007967f3f

📥 Commits

Reviewing files that changed from the base of the PR and between b5da895 and fc18387.

📒 Files selected for processing (4)
  • frontend/src/App.tsx
  • frontend/src/components/CareerUnsavedGuard.test.tsx
  • frontend/src/components/CareerUnsavedGuard.tsx
  • frontend/src/components/forms/CareerResumeForm.tsx
💤 Files with no reviewable changes (1)
  • frontend/src/components/forms/CareerResumeForm.tsx

Comment thread frontend/src/components/CareerUnsavedGuard.tsx Outdated
CareerUnsavedGuard が formCache の unknown 値を CareerFormState に
キャストしていた箇所を、isCareerFormState による実行時検証に置き換える。
useCareerDirty / buildClean は配列・文字列フィールドへネストアクセスするため、
不正・欠損値が入っていた場合のクラッシュを防ぎ、安全側(未保存なし)に倒す。

- isCareerFormState 型ガードを追加(先頭構造の検証)
- 不正値は form=空フォーム / baseline=null にフォールバック
- 壊れたキャッシュ値でクラッシュしないことの回帰テストを追加

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@yusuke0610
yusuke0610 merged commit de929c4 into main Jun 9, 2026
18 checks passed
@yusuke0610
yusuke0610 deleted the feat/career-unsaved-guard-global branch June 10, 2026 05:59
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