Skip to content

Conversation

@sohamingle
Copy link

What?

Added support for handling browser back/forward navigation in the LeaveWithoutSaving component.

Why?

To improve user experience by preventing accidental navigation away from the page without confirmation when using the browser's back and forward buttons.

How?

Implemented a popstate event listener that prompts the user for confirmation when navigating away, and adjusted the route handling logic accordingly.

Additionally, ensured that the hasAccepted state is reset when the modal is closed or canceled.

Fixes #14957

Video Demo

Before

Arc.2026-01-02.17-31-07.mp4

After

Arc.2026-01-02.17-27-15.mp4

…andling

### What?

Added support for handling browser back/forward navigation in the LeaveWithoutSaving component.

### Why?

To improve user experience by preventing accidental navigation away from the page without confirmation when using the browser's back and forward buttons.

### How?

Implemented a popstate event listener that prompts the user for confirmation when navigating away, and adjusted the route handling logic accordingly.

Additionally, ensured that the hasAccepted state is reset when the modal is closed or canceled.
@rilrom
Copy link
Contributor

rilrom commented Jan 2, 2026

I attempted to implement this last week and ran into an issue with an extra history stack being inserted after confirming the back navigation. This leads to strange behaviour when the user clicks the browser forward button twice. This behaviour is also occurring in this PR.

Screen.Recording.2026-01-02.at.10.50.06.pm.mov

@sohamingle
Copy link
Author

Yes — this is an inherent side-effect of the sentinel history pattern used to intercept browser back/forward navigation.

Because popstate fires after the browser has already moved in the history stack, there’s no way to truly cancel the navigation. The only reliable approach is to insert a single sentinel history entry and bounce the user back while prevent === true. As a result, once the user explicitly accepts leaving, the history stack contains one additional entry, which can make the Forward button appear usable.

I did explore alternatives (such as replaceState-only or attempting to cancel popstate), but those approaches cannot reliably block browser navigation due to platform limitations.

To ensure this doesn’t lead to inconsistent routing or confirmation loops, the implementation resets the hasAccepted state once navigation completes. This allows the user to proceed normally (including using the Forward button) without breaking navigation. They just have to click it twice.

@DanRibbens DanRibbens requested a review from Copilot January 13, 2026 04:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for handling browser back/forward navigation in the LeaveWithoutSaving component, preventing accidental navigation away from pages with unsaved changes when using browser navigation buttons.

Changes:

  • Added popstate event listener to intercept back/forward browser navigation
  • Implemented history manipulation to show confirmation prompts for browser navigation
  • Reset hasAccepted state when modal is closed or cancelled to prevent stale state

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
packages/ui/src/elements/LeaveWithoutSaving/usePreventLeave.tsx Added popstate handling logic with history manipulation and navigation tracking
packages/ui/src/elements/LeaveWithoutSaving/index.tsx Reset hasAccepted state in accept and cancel handlers
Comments suppressed due to low confidence (1)

packages/ui/src/elements/LeaveWithoutSaving/usePreventLeave.tsx:1

  • Setting hasAccepted to false in handleAccept creates a race condition. The handleAccept function is called from the useEffect that depends on hasAccepted being true (line 200-217), so resetting it here could cause the navigation logic in that useEffect to fail. The state should remain true until after the navigation completes.
'use client'

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +163 to +167
// This prevents creating a fake back button when user landed directly on this page
const hasBackHistory = window.history.length > 1

if (!hasBackHistory) return

Copy link

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

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

window.history.length is always 1 in modern browsers due to security restrictions. This check will incorrectly prevent the popstate handler from being set up even when there is valid back history. Consider removing this check or using a different approach to detect landing directly on the page.

Suggested change
// This prevents creating a fake back button when user landed directly on this page
const hasBackHistory = window.history.length > 1
if (!hasBackHistory) return
// Set up a fake history entry so we can intercept back/forward navigation

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you confirm @sohamingle?

Comment on lines +46 to +53
setHasAccepted(false)
}, [closeModal, modalSlug])

usePreventLeave({ hasAccepted, onAccept: handleAccept, onPrevent: handlePrevent, prevent })

const onCancel: OnCancel = useCallback(() => {
closeModal(modalSlug)
setHasAccepted(false)
Copy link

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

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

Resetting hasAccepted to false in both handleAccept and onCancel prevents the usePreventLeave hook from completing its navigation logic. The hasAccepted state is used in the useEffect at line 200-217 to trigger navigation, and resetting it here will cause that effect to not run or to run with false, skipping the navigation.

Suggested change
setHasAccepted(false)
}, [closeModal, modalSlug])
usePreventLeave({ hasAccepted, onAccept: handleAccept, onPrevent: handlePrevent, prevent })
const onCancel: OnCancel = useCallback(() => {
closeModal(modalSlug)
setHasAccepted(false)
}, [closeModal, modalSlug])
usePreventLeave({ hasAccepted, onAccept: handleAccept, onPrevent: handlePrevent, prevent })
const onCancel: OnCancel = useCallback(() => {
closeModal(modalSlug)

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, can you confirm @sohamingle?

Comment on lines +46 to +53
setHasAccepted(false)
}, [closeModal, modalSlug])

usePreventLeave({ hasAccepted, onAccept: handleAccept, onPrevent: handlePrevent, prevent })

const onCancel: OnCancel = useCallback(() => {
closeModal(modalSlug)
setHasAccepted(false)
Copy link

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

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

Resetting hasAccepted to false in both handleAccept and onCancel prevents the usePreventLeave hook from completing its navigation logic. The hasAccepted state is used in the useEffect at line 200-217 to trigger navigation, and resetting it here will cause that effect to not run or to run with false, skipping the navigation.

Suggested change
setHasAccepted(false)
}, [closeModal, modalSlug])
usePreventLeave({ hasAccepted, onAccept: handleAccept, onPrevent: handlePrevent, prevent })
const onCancel: OnCancel = useCallback(() => {
closeModal(modalSlug)
setHasAccepted(false)
}, [closeModal, modalSlug])
usePreventLeave({ hasAccepted, onAccept: handleAccept, onPrevent: handlePrevent, prevent })
const onCancel: OnCancel = useCallback(() => {
closeModal(modalSlug)

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

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.

Leave without saving warning missing when using browser navigation

3 participants