Skip to content

feat: warning when navigating away from page with unsaved changes#18065

Merged
Jondyr merged 5 commits intomainfrom
feat/warning-when-navigating-away-from-about-app-page
Mar 9, 2026
Merged

feat: warning when navigating away from page with unsaved changes#18065
Jondyr merged 5 commits intomainfrom
feat/warning-when-navigating-away-from-about-app-page

Conversation

@Jondyr
Copy link
Copy Markdown
Member

@Jondyr Jondyr commented Mar 6, 2026

Description

Adds a warning when navigating away from 'om appen' with unsaved changes.
Should probably not be merged before #18063
PR adds a new hook in studio-hooks: useUnsavedChangesWarning since both traditional navigation and SPA/react-router navigation need different handling useBlocker/useBeforeUnload

Text

Added a text for the warning dialog:

"app_settings.about_tab_unsaved_changes_navigation_warning": "Du har ulagrede endringer.",

Screen.Recording.2026-03-06.at.09.18.34.mov

Verification

  • Related issues are connected (if applicable)
  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • Relevant automated test added (if you find this hard, leave it and we'll help out)

Summary by CodeRabbit

  • New Features

    • Added unsaved changes detection and warning system that alerts users when attempting to navigate away or close the application with unsaved modifications to configuration settings. Users will receive confirmation dialogs before losing any unsaved changes.
  • Tests

    • Added comprehensive test coverage for unsaved changes warning functionality across browser unload and in-app navigation scenarios.

@github-actions github-actions Bot added skip-releasenotes Issues that do not make sense to list in our release notes frontend solution/studio/designer labels Mar 6, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 6, 2026

📝 Walkthrough

Walkthrough

A new unsaved changes warning system has been implemented to alert users before navigating away from or closing a form with unsaved changes. This includes a new custom React hook, integration into the AppConfigForm component, Norwegian translation, test coverage, and test infrastructure updates.

Changes

Cohort / File(s) Summary
New Hook Implementation
src/Designer/frontend/libs/studio-hooks/src/hooks/useUnsavedChangesWarning.ts
Introduces useUnsavedChangesWarning hook that uses useBeforeUnload to prevent page unload and useBlocker to intercept in-app navigation, showing a confirmation dialogue when unsaved changes exist.
Hook Export
src/Designer/frontend/libs/studio-hooks/src/hooks/index.ts
Re-exports new useUnsavedChangesWarning hook to extend public API of the studio-hooks library.
Hook Testing
src/Designer/frontend/libs/studio-hooks/src/hooks/useUnsavedChangesWarning.test.ts
Comprehensive test suite validating beforeunload prevention, navigation blocker behaviour, and confirmation dialogue interactions with mocked react-router-dom hooks.
Component Integration
src/Designer/frontend/app-development/features/appSettings/components/.../AppConfigForm.tsx
Integrates useUnsavedChangesWarning hook to compute hasUnsavedChanges flag by comparing updated and default app config; updates ActionButtons disabled state based on this flag.
Translation Support
src/Designer/frontend/language/src/nb.json
Adds Norwegian translation key app_settings.about_tab_unsaved_changes_navigation_warning with message about unsaved changes.
Test Infrastructure
src/Designer/frontend/testing/setupTests.ts
Adds mock implementations for react-router-dom hooks (useBlocker, useBeforeUnload) and updates SignalR mock to include local mock extensions.

Sequence Diagram

sequenceDiagram
    participant User
    participant Component as AppConfigForm Component
    participant Hook as useUnsavedChangesWarning Hook
    participant Router as React Router
    participant Dialog as Confirmation Dialogue

    User->>Component: Make form changes
    Component->>Component: Update appConfig state
    Component->>Component: Compute hasUnsavedChanges
    Component->>Hook: Pass hasUnsavedChanges & message
    Hook->>Router: Register blocker with hasUnsavedChanges=true
    
    User->>Router: Attempt navigation
    Router->>Hook: Trigger blocker
    Hook->>Dialog: Show confirm(message)
    Dialog->>User: Display confirmation prompt
    
    alt User confirms
        User->>Dialog: Click OK
        Hook->>Router: Call blocker.proceed()
        Router->>Component: Navigate away
    else User cancels
        User->>Dialog: Click Cancel
        Hook->>Router: Call blocker.reset()
        Router->>Component: Remain on page
    end
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A rabbit hops through forms with care,
Now warning users: "Data's there!"
Before you flee or close the door,
We'll ask: "Are you sure? Save more?"
Unsaved changes, now beware! 🍕

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarises the main change: adding a warning when users navigate away from a page with unsaved changes.
Description check ✅ Passed The description covers the main changes and includes context about the new hook, but the Verification section is incomplete with most checkboxes unchecked or marked as not done.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/warning-when-navigating-away-from-about-app-page

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.

@Jondyr Jondyr force-pushed the feat/warning-when-navigating-away-from-about-app-page branch from 95f5f32 to 6214861 Compare March 6, 2026 08:26
@Jondyr Jondyr requested a review from Ildest March 6, 2026 08:27
@github-actions github-actions Bot added the quality/testing Tests that are missing, needs to be created or could be improved. label Mar 6, 2026
@Jondyr Jondyr moved this to 🔎 In review in Team Altinn Studio Mar 6, 2026
@Jondyr Jondyr added the squad/utforming Issues that belongs to the named squad. label Mar 6, 2026
@Jondyr Jondyr self-assigned this Mar 6, 2026
@Jondyr Jondyr force-pushed the feat/warning-when-navigating-away-from-about-app-page branch from 811a7b3 to 2d60ce2 Compare March 6, 2026 09:09
@Jondyr Jondyr marked this pull request as ready for review March 6, 2026 09:14
Copy link
Copy Markdown
Contributor

@Ildest Ildest left a comment

Choose a reason for hiding this comment

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

Se bra ut Jonas! Forslag til en liten omformulering, men ikke et must. Prøver å unngå adverb skapt med u, unntatt de som er veldig kjent, som ugjort.

Comment thread src/Designer/frontend/language/src/nb.json Outdated
Co-authored-by: Gørild Døhl <gorild.dohl@digdir.no>
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.20%. Comparing base (323683c) to head (ef395f6).
⚠️ Report is 142 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #18065      +/-   ##
==========================================
+ Coverage   95.16%   95.20%   +0.03%     
==========================================
  Files        2501     2506       +5     
  Lines       32446    32605     +159     
  Branches     3844     3872      +28     
==========================================
+ Hits        30877    31041     +164     
+ Misses       1221     1219       -2     
+ Partials      348      345       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Jondyr Jondyr removed their assignment Mar 6, 2026
@JamalAlabdullah JamalAlabdullah self-assigned this Mar 6, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

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 the current code and only fix it if needed.

Inline comments:
In
`@src/Designer/frontend/libs/studio-hooks/src/hooks/useUnsavedChangesWarning.ts`:
- Around line 14-24: useBlocker in useUnsavedChangesWarning relies on a data
router but both studio-root/index.tsx and dashboard/index.tsx currently use
BrowserRouter (legacy) so in-app navigation blocking fails; migrate those roots
to use a data router (e.g., createBrowserRouter) and render via RouterProvider
so useBlocker works: replace the BrowserRouter instantiation/import with
createBrowserRouter(routeDefinitions) and wrap the app in <RouterProvider
router={router}> (updating route definitions to the data-router shape), update
any navigation usages if needed, and keep useUnsavedChangesWarning (and its
useBlocker call) unchanged so it receives the correct router context.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 793c51c8-5cce-4a8f-8197-9df06ba8431a

📥 Commits

Reviewing files that changed from the base of the PR and between b30565d and ef395f6.

📒 Files selected for processing (6)
  • src/Designer/frontend/app-development/features/appSettings/components/TabsContent/Tabs/AboutTab/AppConfigForm/AppConfigForm.tsx
  • src/Designer/frontend/language/src/nb.json
  • src/Designer/frontend/libs/studio-hooks/src/hooks/index.ts
  • src/Designer/frontend/libs/studio-hooks/src/hooks/useUnsavedChangesWarning.test.ts
  • src/Designer/frontend/libs/studio-hooks/src/hooks/useUnsavedChangesWarning.ts
  • src/Designer/frontend/testing/setupTests.ts

Copy link
Copy Markdown
Contributor

@JamalAlabdullah JamalAlabdullah left a comment

Choose a reason for hiding this comment

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

Very good enhancement! I tested and most of them works fine, I just noticed two things you can see them in the video i added. I am not sure if these planned to be resolved in this issue ?

  • The warning message does not work with optional choices
  • changing in switch in the delegering card keep the global save button active.
Screen.Recording.2026-03-06.at.11.40.44.mov

@Jondyr
Copy link
Copy Markdown
Member Author

Jondyr commented Mar 9, 2026

@JamalAlabdullah
I think those are implemented with their own internal state, so i think a rework to use the state of the overall page would be necessary to support this. Maybe something better suited for another PR?

@Jondyr Jondyr assigned JamalAlabdullah and unassigned Jondyr Mar 9, 2026
Copy link
Copy Markdown
Contributor

@JamalAlabdullah JamalAlabdullah left a comment

Choose a reason for hiding this comment

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

Nice!

@Jondyr Jondyr merged commit 75eb03a into main Mar 9, 2026
11 checks passed
@Jondyr Jondyr deleted the feat/warning-when-navigating-away-from-about-app-page branch March 9, 2026 08:27
@github-project-automation github-project-automation Bot moved this from 🔎 In review to ✅ Done in Team Altinn Studio Mar 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend quality/testing Tests that are missing, needs to be created or could be improved. skip-releasenotes Issues that do not make sense to list in our release notes solution/studio/designer squad/utforming Issues that belongs to the named squad.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants