From b8df6d8ec55c0029e5373660e040164bad2a7167 Mon Sep 17 00:00:00 2001 From: AmarTrebinjac Date: Thu, 5 Feb 2026 14:35:54 +0000 Subject: [PATCH 1/3] fix: await async save before closing dirty form modal The DirtyFormModal was closing immediately after triggering the onSave callback, without waiting for async mutations to complete. This caused a race condition where navigation could conflict with the mutation's onSuccess navigation handler. Changes: - Make DirtyFormModal's handleSave async and await onSave callback - Add loading state to Save button to prevent double-clicks - Keep modal open if save fails (error handled by mutation's onError) - Update useDirtyForm's onSave type to allow Promise return - Use mutateAsync in useUserExperienceForm for proper Promise handling Closes ENG-616 Co-Authored-By: Claude Opus 4.5 --- .../src/components/modals/DirtyFormModal.tsx | 26 ++++++++++++++----- packages/shared/src/hooks/useDirtyForm.ts | 2 +- .../shared/src/hooks/useUserExperienceForm.ts | 6 ++--- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/packages/shared/src/components/modals/DirtyFormModal.tsx b/packages/shared/src/components/modals/DirtyFormModal.tsx index 88e9111e30..36db5c7d52 100644 --- a/packages/shared/src/components/modals/DirtyFormModal.tsx +++ b/packages/shared/src/components/modals/DirtyFormModal.tsx @@ -1,5 +1,5 @@ import type { ReactElement } from 'react'; -import React from 'react'; +import React, { useState } from 'react'; import type { LazyModalCommonProps } from './common/Modal'; import { Modal } from './common/Modal'; import { Button, ButtonSize, ButtonVariant } from '../buttons/Button'; @@ -12,7 +12,7 @@ import { useLazyModal } from '../../hooks/useLazyModal'; interface DirtyFormModalProps extends LazyModalCommonProps { onDiscard: () => void; - onSave: () => void; + onSave: () => void | Promise; } export default function DirtyFormModal({ @@ -22,12 +22,23 @@ export default function DirtyFormModal({ onSave, }: DirtyFormModalProps): ReactElement { const { closeModal } = useLazyModal(); + const [isSaving, setIsSaving] = useState(false); - const handleSave = () => { - if (onSave) { - onSave(); + const handleSave = async () => { + if (!onSave) { + closeModal(); + return; + } + + setIsSaving(true); + try { + await onSave(); + closeModal(); + } catch { + // Error handled by caller's onError - keep modal open + } finally { + setIsSaving(false); } - closeModal(); }; const handleDiscard = () => { @@ -67,6 +78,7 @@ export default function DirtyFormModal({ variant={ButtonVariant.Secondary} size={ButtonSize.Medium} onClick={handleDiscard} + disabled={isSaving} > Discard @@ -75,6 +87,8 @@ export default function DirtyFormModal({ variant={ButtonVariant.Primary} size={ButtonSize.Medium} onClick={handleSave} + loading={isSaving} + disabled={isSaving} > Save changes diff --git a/packages/shared/src/hooks/useDirtyForm.ts b/packages/shared/src/hooks/useDirtyForm.ts index a7a3822632..26544dad9a 100644 --- a/packages/shared/src/hooks/useDirtyForm.ts +++ b/packages/shared/src/hooks/useDirtyForm.ts @@ -4,7 +4,7 @@ import { useLazyModal } from './useLazyModal'; import { LazyModal } from '../components/modals/common/types'; export interface UseDirtyFormOptions { - onSave: () => void; + onSave: () => void | Promise; onDiscard?: () => void; } diff --git a/packages/shared/src/hooks/useUserExperienceForm.ts b/packages/shared/src/hooks/useUserExperienceForm.ts index 038c07ad0b..7e7854befb 100644 --- a/packages/shared/src/hooks/useUserExperienceForm.ts +++ b/packages/shared/src/hooks/useUserExperienceForm.ts @@ -127,7 +127,7 @@ const useUserExperienceForm = ({ { condition: isNewExperience }, ); - const { mutate, isPending } = useMutation({ + const { mutateAsync, isPending } = useMutation({ mutationFn: (data: UserExperience | UserExperienceWork) => type === UserExperienceType.Work ? upsertUserWorkExperience(data as UserExperienceWork, id) @@ -166,9 +166,9 @@ const useUserExperienceForm = ({ }, }); const dirtyForm = useDirtyForm(methods.formState.isDirty, { - onSave: () => { + onSave: async () => { const formData = methods.getValues(); - mutate(formData); + await mutateAsync(formData); }, onDiscard: () => { methods.reset(); From 1a56bd0603ea57f7fb25fb4d4cd1fca3ed712eac Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Thu, 5 Feb 2026 14:56:43 +0000 Subject: [PATCH 2/3] fix: properly handle navigation after saving from dirty form modal - Wrap onSave in useDirtyForm to handle navigation after save completes - Navigate to pending URL after successful save - Prevent double navigation by checking for pending navigation in useUserExperienceForm - Fixes issue where back button bypassed the modal after save Co-authored-by: Amar Trebinjac --- packages/shared/src/hooks/useDirtyForm.ts | 17 ++++++++++++++--- .../shared/src/hooks/useUserExperienceForm.ts | 6 +++++- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/shared/src/hooks/useDirtyForm.ts b/packages/shared/src/hooks/useDirtyForm.ts index 26544dad9a..b601512802 100644 --- a/packages/shared/src/hooks/useDirtyForm.ts +++ b/packages/shared/src/hooks/useDirtyForm.ts @@ -30,6 +30,17 @@ export const useDirtyForm = ( } }, [onDiscard, router]); + const handleSave = useCallback(async () => { + await onSave(); + + allowNavigationRef.current = true; + + if (pendingUrlRef.current) { + router.push(pendingUrlRef.current); + pendingUrlRef.current = null; + } + }, [onSave, router]); + useEffect(() => { const handleRouteChangeStart = (url: string) => { if (allowNavigationRef.current || !isDirty || url === router.asPath) { @@ -43,7 +54,7 @@ export const useDirtyForm = ( type: LazyModal.DirtyForm, props: { onDiscard: handleDiscard, - onSave, + onSave: handleSave, }, }); @@ -58,7 +69,7 @@ export const useDirtyForm = ( return () => { router.events.off('routeChangeStart', handleRouteChangeStart); }; - }, [isDirty, router, openModal, handleDiscard, onSave]); + }, [isDirty, router, openModal, handleDiscard, handleSave]); useEffect(() => { const handleBeforeUnload = (e: BeforeUnloadEvent) => { @@ -89,6 +100,6 @@ export const useDirtyForm = ( }, hasPendingNavigation: () => pendingUrlRef.current !== null, navigateToPending, - save: onSave, + save: handleSave, }; }; diff --git a/packages/shared/src/hooks/useUserExperienceForm.ts b/packages/shared/src/hooks/useUserExperienceForm.ts index 7e7854befb..3f5f72754b 100644 --- a/packages/shared/src/hooks/useUserExperienceForm.ts +++ b/packages/shared/src/hooks/useUserExperienceForm.ts @@ -147,7 +147,11 @@ const useUserExperienceForm = ({ queryKey: generateQueryKey(RequestKey.UserExperience, user, 'profile'), exact: false, }); - router.push(`${webappUrl}settings/profile/experience/${type}`); + + // Only navigate to default location if there's no pending navigation from dirty form + if (!dirtyFormRef.current?.hasPendingNavigation()) { + router.push(`${webappUrl}settings/profile/experience/${type}`); + } }, onError: (error: ApiErrorResult) => { if ( From 3a53a77863dad5e882e56dcca8d6b47a97fc4e08 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Thu, 5 Feb 2026 15:08:11 +0000 Subject: [PATCH 3/3] fix: intercept browser back button with beforePopState The previous implementation only listened to routeChangeStart event, which doesn't fire for browser back/forward button clicks. Added beforePopState handler to properly intercept browser history navigation and show the dirty form modal. Co-authored-by: Amar Trebinjac --- packages/shared/src/hooks/useDirtyForm.ts | 29 +++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/shared/src/hooks/useDirtyForm.ts b/packages/shared/src/hooks/useDirtyForm.ts index b601512802..974caae3e6 100644 --- a/packages/shared/src/hooks/useDirtyForm.ts +++ b/packages/shared/src/hooks/useDirtyForm.ts @@ -71,6 +71,35 @@ export const useDirtyForm = ( }; }, [isDirty, router, openModal, handleDiscard, handleSave]); + useEffect(() => { + const handleBeforePopState = ({ url }: { url: string }) => { + if (allowNavigationRef.current || !isDirty) { + allowNavigationRef.current = false; + return true; + } + + // Store the URL we're trying to navigate to + pendingUrlRef.current = url; + + openModal({ + type: LazyModal.DirtyForm, + props: { + onDiscard: handleDiscard, + onSave: handleSave, + }, + }); + + // Prevent the navigation + return false; + }; + + router.beforePopState(handleBeforePopState); + + return () => { + router.beforePopState(() => true); + }; + }, [isDirty, router, openModal, handleDiscard, handleSave]); + useEffect(() => { const handleBeforeUnload = (e: BeforeUnloadEvent) => { if (isDirty) {