Skip to content

feat(ui): enhance LeaveWithoutSaving component with popstate navigation handling #15074

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sohamingle wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(ui): enhance LeaveWithoutSaving component with popstate navigation handling #15074

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/ui/src/elements/LeaveWithoutSaving/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,14 @@ export const LeaveWithoutSaving: React.FC<LeaveWithoutSavingProps> = ({

const handleAccept = useCallback(() => {
closeModal(modalSlug)
setHasAccepted(false)
}, [closeModal, modalSlug])

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

const onCancel: OnCancel = useCallback(() => {
closeModal(modalSlug)
setHasAccepted(false)
Comment on lines +46 to +53
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

@DanRibbens DanRibbens Jan 13, 2026

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
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

@DanRibbens DanRibbens Jan 13, 2026

Choose a reason for hiding this comment

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

Same as above.

}, [closeModal, modalSlug])

const handleConfirm = useCallback(async () => {
Expand Down
56 changes: 54 additions & 2 deletions packages/ui/src/elements/LeaveWithoutSaving/usePreventLeave.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ export const usePreventLeave = ({

const router = useRouter()
const cancelledURL = useRef<string>('')
// Track if we're handling a popstate (back/forward) navigation
const isPopstateNavigation = useRef<boolean>(false)

// check when page is about to be changed
useEffect(() => {
Expand Down Expand Up @@ -128,6 +130,7 @@ export const usePreventLeave = ({
if (isPageLeaving && prevent && (!onPrevent ? !window.confirm(message) : true)) {
// Keep a reference of the href
cancelledURL.current = newUrl
isPopstateNavigation.current = false

// Cancel the route change
event.preventDefault()
Expand All @@ -153,14 +156,63 @@ export const usePreventLeave = ({
document.removeEventListener('click', handleClick, true)
}
}, [onPrevent, prevent, message])
// Handle browser back/forward button navigation (popstate events)
useEffect(() => {
if (!prevent) return

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

if (!hasBackHistory) return

Comment on lines +163 to +167
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

@DanRibbens DanRibbens Jan 13, 2026

Choose a reason for hiding this comment

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

Can you confirm @sohamingle?

window.history.pushState(null, '', window.location.href)

function handlePopstate() {

window.history.pushState(null, '', window.location.href)

isPopstateNavigation.current = true

if (!onPrevent) {
if (window.confirm(message)) {
if (onAccept) {
onAccept()
}
// When the user clicks back, they pop this entry instead of leaving, allowing us to show a confirmation. If they confirm, we use history.go(-2) to skip both fake entries.
window.history.go(-2)
}
return
}

onPrevent()
}

// Add the global popstate event listener
window.addEventListener('popstate', handlePopstate)

return () => {
// Remove the global popstate event listener
window.removeEventListener('popstate', handlePopstate)
}
}, [prevent, onPrevent, onAccept, message])

useEffect(() => {
if (hasAccepted && cancelledURL.current) {
if (hasAccepted) {
if (onAccept) {
onAccept()
}

startRouteTransition(() => router.push(cancelledURL.current))
if (isPopstateNavigation.current) {
isPopstateNavigation.current = false
// For back button navigation, go back in history
window.history.go(-2)
} else if (cancelledURL.current) {
const url = cancelledURL.current
cancelledURL.current = ''

// For click navigation, push to the cancelled URL
startRouteTransition(() => router.push(url))
}
}
}, [hasAccepted, onAccept, router, startRouteTransition])
}
Loading