fix: confirm before leaving dirty job forms

[aledefra]

Summary

• add a reusable useUnsavedChangesGuard hook for dirty-form navigation protection
• apply the guard to create job form cancel/back flows
• apply the guard to edit job form cancel/back flows
• pass onCancel through the create job header so both header and footer exits are protected

Validation

• npm run lint ✅
• npm run build ⚠️ fails at prerender of /deeploys/create-project due existing env/config issue (deeployUrl undefined), unrelated to this change

[Copilot]

Pull request overview

This PR implements navigation guards to prevent users from accidentally leaving job forms with unsaved changes. It introduces a new useUnsavedChangesGuard hook that monitors form state and prompts users for confirmation before navigating away, then applies this protection to both the create and edit job form flows.

Changes:

• Adds a reusable useUnsavedChangesGuard hook that intercepts navigation attempts (browser close/refresh, anchor clicks, and browser back/forward)
• Integrates the hook into create job form (JobFormWrapper.tsx) and edit job form (JobEditFormWrapper.tsx) to prevent data loss
• Updates JobFormHeader component to accept an optional onCancel callback for consistent guard application across header and footer buttons

Reviewed changes

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

File	Description
src/lib/hooks/useUnsavedChangesGuard.ts	New hook implementing navigation guard with beforeunload, click, and popstate event handlers
src/components/edit-job/JobEditFormWrapper.tsx	Integrates unsaved changes guard and creates handleCancel wrapper for router.back()
src/components/create-job/JobFormWrapper.tsx	Integrates unsaved changes guard and creates handleCancel wrapper for form reset
src/components/create-job/JobFormHeader.tsx	Adds optional onCancel prop to support custom cancel handlers

---

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

[Copilot]

The popstate event handler implementation is problematic. The event fires after the navigation has already occurred, so attempting to prevent it by calling window.history.go(1) creates a race condition and unreliable behavior. Additionally, this handler uses window.confirm instead of the custom interaction.confirm used elsewhere, creating an inconsistent user experience. Consider either removing the popstate handler entirely (relying on beforeunload for browser navigation) or implementing a proper history-based solution that pushes a dummy state and listens for navigation attempts.

Suggested change

	useEffect(() => {
	if (!isNavigationBlocked) {
	return;
	}
	const handlePopState = () => {
	if (allowNavigationRef.current) {
	return;
	}
	const confirmed = window.confirm(message);
	if (confirmed) {
	allowNavigationRef.current = true;
	window.setTimeout(() => {
	allowNavigationRef.current = false;
	}, 500);
	return;
	}
	allowNavigationRef.current = true;
	window.history.go(1);
	window.setTimeout(() => {
	allowNavigationRef.current = false;
	}, 0);
	};
	window.addEventListener('popstate', handlePopState);
	return () => {
	window.removeEventListener('popstate', handlePopState);
	};
	}, [isNavigationBlocked, message]);
	return {
	isNavigationBlocked,
	confirmNavigation,
	};
	}
	return {
	isNavigationBlocked,
	confirmNavigation,
	};
	}

[Copilot]

Setting allowNavigationRef.current = true when the user cancels navigation (line 156) followed by attempting to go forward in history creates a window where navigation could be allowed unintentionally. If any other navigation event occurs during this brief period before the timeout resets the flag, it will be allowed to proceed without confirmation.

[Copilot]

The 500ms timeout for resetting allowNavigationRef after successful navigation seems arbitrary and could cause issues. If the navigation action completes quickly but the component unmounts or re-renders before the timeout fires, the flag will remain true. Consider resetting the flag immediately after navigation or using a cleanup function to handle unmounting scenarios.

Suggested change

	window.setTimeout(() => {
	allowNavigationRef.current = false;
	}, 500);
	allowNavigationRef.current = false;

[Copilot]

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.

[Copilot]

The click handler intercepts anchor tag clicks and uses window.location.assign for navigation, which causes a full page reload. This approach won't intercept Next.js Link component navigation or programmatic navigation via router.push(), which are common in Next.js applications. This means users can bypass the unsaved changes guard by clicking Next.js Link components or through programmatic navigation. Consider adding support for Next.js router events or document this limitation clearly.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/components/create-job/JobFormWrapper.tsx:243

• The useEffect dependency array is incomplete. It's missing projectName, draftJobsCount, and account. The effect calls setDefaultJobAlias which uses projectName and draftJobsCount, and getBaseSchemaDefaults uses account?.applicantType for setting jobTags. If any of these values change, the effect won't re-run and the form will have stale values. Add these missing dependencies to the array.

    useEffect(() => {
        if (!jobType) {
            previousJobTypeRef.current = undefined;
            previousRecoveredPrefillKeyRef.current = undefined;
            return;
        }

        const recoveredPrefill = getMatchingRecoveredPrefill();
        const recoveredPrefillKey = getRecoveredPrefillKey(recoveredPrefill);
        const hasSameJobType = previousJobTypeRef.current === jobType;
        const hasNewRecoveredPrefill =
            !!recoveredPrefill && previousRecoveredPrefillKeyRef.current !== recoveredPrefillKey;

        if (hasSameJobType && !hasNewRecoveredPrefill) {
            return;
        }

        previousJobTypeRef.current = jobType;
        previousRecoveredPrefillKeyRef.current = recoveredPrefillKey;

        const defaults = getDefaultSchemaValues();
        const mergedDefaults = mergeDefaults(defaults, recoveredPrefill?.formValues as Record<string, any>);

        form.reset(mergedDefaults);
        form.setValue('jobType', jobType);

        if (form && !recoveredPrefill?.formValues?.deployment?.jobAlias) {
            setDefaultJobAlias(jobType);
        }

        setBaselineValues(form.getValues());

        if (recoveredPrefill) {
            clearPendingRecoveredJobPrefill();
        }
    }, [jobType, form, projectHash, clearPendingRecoveredJobPrefill, pendingRecoveredJobPrefill]);

---

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

[Copilot]

The useEffect has an incomplete dependency array. The effect calls getDefaultSchemaValues() which depends on various props and state (job, jobConfig, etc.), and setTargetNodesCountLower and setAdditionalCost which are state setters. However, only form is listed in the dependency array. This will cause the effect to run on every render since form is 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.

Suggested change

	}, [form]);
	}, []);