Fixed exit button on Quiet Hobbies tab to clear parent#54
Fixed exit button on Quiet Hobbies tab to clear parent#54adityasengarr wants to merge 1 commit intomainfrom
Conversation
WalkthroughA callback invocation is added to the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
rise-dc-app/src/scheduling_components/pages/QuietHobbiesParticipant.tsx (2)
36-50: Consider callback ordering consistency.In
handleChooseActivity(line 47), the parent callback is invoked beforesetOpen(false), but inhandleCloseModal(lines 38-40), the local state is updated before callingonClose?.(). For consistency and to ensure the parent receives callbacks while the child's state is still "open," consider movingonClose?.()beforesetOpen(false).Apply this diff for consistency:
const handleCloseModal = () => { console.log("Closing modal, current open state:", open); - setOpen(false); // Properly calls the parent's onClose() which resets everything and removes all overlays. onClose?.(); + setOpen(false); };
37-37: Consider removing debug logging.The
console.logstatement appears to be debugging code. If this logging isn't needed for production troubleshooting, consider removing it.Apply this diff to remove the console log:
const handleCloseModal = () => { - console.log("Closing modal, current open state:", open); // Properly calls the parent's onClose() which resets everything and removes all overlays. onClose?.(); setOpen(false); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rise-dc-app/src/scheduling_components/pages/QuietHobbiesParticipant.tsx(1 hunks)
🔇 Additional comments (2)
rise-dc-app/src/scheduling_components/pages/QuietHobbiesParticipant.tsx (2)
39-40: LGTM! Parent notification properly added.The addition of
onClose?.()correctly notifies the parentEventSelectionModalto close when the user exits, solving the overlay issue described in the PR. The optional chaining ensures safety ifonCloseis undefined.
27-31: Verify error handling behavior with parent modal.In the error catch block,
setOpen(false)is called butonClose?.()is not. This differs fromhandleCloseModalwhere both are invoked. Confirm whether the parentEventSelectionModalshould remain open when hobby loading fails (allowing the user to try again or go back), or if it should also close. The same question applies to the retry error path at line 69.
Before only set open to false, which closed the QuietHobbyModal and removed its overlay. But it never told the parent EventSelectionModal to close, so added onClose?.() to call the parent's close function, resets and removes all overlays.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.