Skip to content

Discard Changes: Check before unload#23107

Open
nielslyngsoe wants to merge 4 commits into
v17/devfrom
v17/feature/discardchanges-beforeunload-listening
Open

Discard Changes: Check before unload#23107
nielslyngsoe wants to merge 4 commits into
v17/devfrom
v17/feature/discardchanges-beforeunload-listening

Conversation

@nielslyngsoe

@nielslyngsoe nielslyngsoe commented Jun 12, 2026

Copy link
Copy Markdown
Member

Checking for discard changes before unloading the page, aka. closing the tab/browser.

Cause the browser to ask if the user is sure about closing the tab when having unsaved changes

@nielslyngsoe nielslyngsoe changed the base branch from main to v17/dev June 12, 2026 09:58
@nielslyngsoe nielslyngsoe marked this pull request as ready for review June 29, 2026 08:40
Copilot AI review requested due to automatic review settings June 29, 2026 08:40
@claude

claude Bot commented Jun 29, 2026

Copy link
Copy Markdown

Claude finished @nielslyngsoe's task in 2m 19s —— View job


PR Review

Target: origin/v17/dev · Based on commit: 36c2dcfe · Skipped: 0 noise files out of 1 total

Adds a beforeunload listener to UmbEntityDetailWorkspaceContextBase so the browser's native "Leave page?" confirmation appears when the user attempts to close a tab or browser window with unsaved changes.

  • Affected implementations (outside this PR): All workspace contexts that extend UmbEntityDetailWorkspaceContextBase — directly: content-detail-workspace-base.ts, content-type-workspace-context-base.ts, entity-named-detail-workspace-base.ts. These all gain the new behavior automatically.

Suggestions

  • entity-detail-workspace-base.ts:489–493: e.preventDefault() triggers the native dialog in Chrome 119+ (Oct 2023), Firefox 54+, and Safari 16.1+. Enterprise/backoffice environments sometimes run slightly behind; adding e.returnValue = '' as a fallback costs nothing and covers the gap:

    #onBeforeUnload = (e: BeforeUnloadEvent) => {
        if (this.getHasUnpersistedChanges()) {
            e.preventDefault();
            e.returnValue = ''; // legacy fallback for Chrome < 119 / Safari < 16.1
        }
    };

    This is a nice-to-have; e.preventDefault() alone is fine for any browser released in the last 2+ years.


Approved with Suggestions for improvement

Good to go, but please carefully consider the importance of the suggestions.

@nielslyngsoe nielslyngsoe changed the title Check for discard changes before unload Discard Changes: Check before unload Jun 29, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds an unload-time safeguard to entity detail workspaces so users are prompted before closing the tab/window when there are unpersisted changes.

Changes:

  • Register a beforeunload handler to block page unload when getHasUnpersistedChanges() is true.
  • Unregister the beforeunload handler in destroy() to avoid leaking listeners.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@nielslyngsoe nielslyngsoe enabled auto-merge (squash) June 29, 2026 08:42
@nielslyngsoe nielslyngsoe added the category/ux User experience label Jun 29, 2026
@sonarqubecloud

sonarqubecloud Bot commented Jul 9, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

💡 Need a hand with PR review? Try Gitar by Sonar!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants