fix(dialogs): move open-reset effects into onOpenChange handlers#1268
Conversation
|
Someone is attempting to deploy a commit to the Rohan Verma's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on 2b2453e..2d4adce
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (3)
• surfsense_web/components/documents/CreateFolderDialog.tsx
• surfsense_web/components/documents/FolderPickerDialog.tsx
• surfsense_web/components/free-chat/free-model-selector.tsx
Fixes #1251
Problem
Three components used
useEffect(() => { if (open) {...} }, [open])to reset form state and focus inputs when dialogs/popovers open. Per React's "you might not need an effect" guidance, logic triggered by a user action belongs in the event handler, not auseEffect.Changes
Replaced the
useEffectblock in each file with ahandleOpenChange = useCallback(...)wrapper that resets state whennext === true, then forwards to the parent'sonOpenChange/setOpen:CreateFolderDialog.tsx— clears folder name input + focuses itFolderPickerDialog.tsx— resets selected folder + collapsed statefree-model-selector.tsx— clears search query + resets focus index + focuses search inputRemoved
useEffectfrom the React import in the two files that no longer use it. The model-fetchuseEffectinfree-model-selector.tsxis preserved.What is NOT changed
FolderPickerDialogis intentionally left as-is (out of scope)How to verify
pnpm build— passes with no TypeScript errorsHigh-level PR Summary
This PR refactors three dialog components to follow React best practices by moving state reset and focus logic from
useEffecthooks intoonOpenChangeevent handlers. When dialogs open, the same initialization behavior (clearing input fields, resetting selection state, and focusing inputs) now happens directly in response to the user action rather than as a side effect of theopenstate changing.⏱️ Estimated Review Time: 5-15 minutes
💡 Review Order Suggestion
surfsense_web/components/documents/CreateFolderDialog.tsxsurfsense_web/components/documents/FolderPickerDialog.tsxsurfsense_web/components/free-chat/free-model-selector.tsx