-
Notifications
You must be signed in to change notification settings - Fork 0
fix: confirm before leaving dirty job forms #52
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,95 @@ | ||||||||||
| 'use client'; | ||||||||||
|
|
||||||||||
| import { InteractionContextType, useInteractionContext } from '@lib/contexts/interaction'; | ||||||||||
| import { useCallback, useEffect, useRef } from 'react'; | ||||||||||
|
|
||||||||||
| const DEFAULT_MESSAGE = 'You have unsaved changes. Are you sure you want to leave this page?'; | ||||||||||
|
|
||||||||||
| interface UseUnsavedChangesGuardOptions { | ||||||||||
| isDirty: boolean; | ||||||||||
| isSubmitting?: boolean; | ||||||||||
| message?: string; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| export function useUnsavedChangesGuard({ | ||||||||||
| isDirty, | ||||||||||
| isSubmitting = false, | ||||||||||
| message = DEFAULT_MESSAGE, | ||||||||||
| }: UseUnsavedChangesGuardOptions) { | ||||||||||
| const interaction = useInteractionContext() as InteractionContextType | null; | ||||||||||
| const isNavigationBlocked = isDirty && !isSubmitting; | ||||||||||
| const allowNavigationRef = useRef(false); | ||||||||||
| const isPromptOpenRef = useRef(false); | ||||||||||
|
|
||||||||||
| const requestConfirmation = useCallback(async () => { | ||||||||||
| if (!isNavigationBlocked || allowNavigationRef.current) { | ||||||||||
| return true; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if (isPromptOpenRef.current) { | ||||||||||
| return false; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| isPromptOpenRef.current = true; | ||||||||||
|
|
||||||||||
| try { | ||||||||||
| if (!interaction?.confirm) { | ||||||||||
| return window.confirm(message); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return await interaction.confirm(message, { | ||||||||||
| confirmButtonClassNames: 'bg-slate-900', | ||||||||||
| }); | ||||||||||
| } finally { | ||||||||||
| isPromptOpenRef.current = false; | ||||||||||
| } | ||||||||||
| }, [interaction, isNavigationBlocked, message]); | ||||||||||
|
|
||||||||||
| const confirmNavigation = useCallback( | ||||||||||
| async (action: () => void | Promise<void>) => { | ||||||||||
| const confirmed = await requestConfirmation(); | ||||||||||
|
|
||||||||||
| if (!confirmed) { | ||||||||||
| return false; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| allowNavigationRef.current = true; | ||||||||||
|
|
||||||||||
| try { | ||||||||||
| await action(); | ||||||||||
| return true; | ||||||||||
| } finally { | ||||||||||
| window.setTimeout(() => { | ||||||||||
| allowNavigationRef.current = false; | ||||||||||
| }, 500); | ||||||||||
|
Comment on lines
+62
to
+64
|
||||||||||
| window.setTimeout(() => { | |
| allowNavigationRef.current = false; | |
| }, 500); | |
| allowNavigationRef.current = false; |
Copilot
AI
Feb 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The window.setTimeout calls in the confirmNavigation callback and handlePopState handler are not cleaned up if the component unmounts before they fire. This could lead to memory leaks and attempts to update unmounted component state. Store the timeout IDs and clear them in a cleanup function or useEffect return.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The useEffect has an incomplete dependency array. The effect calls
getDefaultSchemaValues()which depends on various props and state (job, jobConfig, etc.), andsetTargetNodesCountLowerandsetAdditionalCostwhich are state setters. However, onlyformis listed in the dependency array. This will cause the effect to run on every render sinceformis recreated each time. Either add all necessary dependencies (which would cause the form to reset unexpectedly), or consider running this effect only once on mount if that's the intended behavior. If it should only run on mount, use an empty dependency array[]instead.