fix(dashboard): Email layout name saving issue fixes NV-7084#10045
fix(dashboard): Email layout name saving issue fixes NV-7084#10045
Conversation
…t name The settings drawer form (layout-settings) is rendered inside the main editor form (edit-layout) via React component tree. Even though the Sheet uses a Radix portal, React's synthetic submit events bubble through the component tree, causing both forms to fire on a single submit - one with the new name from the drawer, and another with the old name from the server, which overrides the update. Changes: - Add e.stopPropagation() to the settings drawer form's onSubmit to prevent the submit event from bubbling to the parent form - Remove the values prop from useForm which was causing the form to re-sync with server data during submission, leading to race conditions - Add useEffect to reset form values only when the drawer transitions from closed to open (not on every layout data change) - Reset form with response data on successful save to keep form state clean before the drawer closes Fixes NV-7084 Co-authored-by: Dima Grossman <dima@grossman.io>
|
Cursor Agent can help with this pull request. Just |
✅ Deploy preview added
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Fixes unreliable email layout name updates in the layout editor by preventing nested form submissions and stabilizing the settings drawer form state so it doesn’t re-sync mid-edit.
Changes:
- Stop React synthetic
submitevent bubbling from the settings drawer form to the parent editor form. - Remove
react-hook-formvaluesusage and reset the drawer form only when the drawer opens. - Reset the drawer form with mutation response data after a successful save.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const wasOpenRef = useRef(false); | ||
| useEffect(() => { | ||
| if (isOpen && !wasOpenRef.current && layout) { | ||
| form.reset({ | ||
| name: layout.name || '', | ||
| layoutId: layout.layoutId || '', | ||
| isTranslationEnabled: layout.isTranslationEnabled || false, | ||
| }); | ||
| } | ||
| wasOpenRef.current = isOpen; | ||
| }, [isOpen, layout, form]); |
There was a problem hiding this comment.
The form reset logic only runs on the closed→open transition. If the layout query is invalidated/refetched (e.g., after saving) and the user reopens the drawer before the refetch completes, the drawer can reset to stale layout data and then won’t resync when the fresh layout arrives while open (because wasOpenRef is already true). That can reintroduce the “old name overwrites new name” behavior if the user submits again. Consider also resetting when layout changes while the drawer is open and the form is still pristine (e.g., track last-synced layout.updatedAt/layout._id in a ref and call form.reset when it changes and !form.formState.isDirty).
scopsy
changed the title
|
Caution Review failedThe pull request is closed. WalkthroughThe layout editor settings drawer component is modified to improve form state management. The changes introduce useEffect and useRef imports, remove initial values from form initialization, and add logic to reset the form when the drawer opens with a layout present. The updateLayout success handler is updated to accept returned data and reset the form using specific properties. The onSubmit handler is refactored to stop event propagation and explicitly invoke form.handleSubmit with the event parameter. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What changed? Why was the change needed?
This PR resolves NV-7084, addressing the unreliable update of email layout names. Users reported that changes were often overridden, as the client sent duplicate API calls: one with the new name and another with the old name.
Root Cause:
layout-settings) is rendered within the React component tree of the main editor form (edit-layout). Despite the settings drawer using a Radix Sheet portal (separate DOM), React's syntheticsubmitevents bubble up the component tree. This caused both forms to submit simultaneously. The main form, always submitting thelayout?.name(the old server value), would override the new name sent by the settings drawer.useFormhook in the settings drawer was using thevaluesprop, making it a "controlled" form that continuously re-synced with externallayoutdata. This led to unexpected resets and race conditions during submission, potentially sending stale data.Changes:
e.stopPropagation()to the settings drawer form'sonSubmithandler to stop the submit event from reaching the parent main form.valuesprop fromuseFormin the settings drawer.useEffectwithwasOpenRefto reset the form only when the drawer transitions from closed to open, ensuring fresh data on open without interfering with mid-edit changes.Screenshots
N/A
Expand for optional sections
Related enterprise PR
Special notes for your reviewer
The core issue was the interaction between nested forms in the React component tree, where synthetic events bubble, even when using portals that separate DOM elements. The
e.stopPropagation()on the settings drawer's form submission is crucial to prevent the parent form from also submitting.Linear Issue: NV-7084