fix : correct modal order and prevent click-through on View Profile #185
Conversation
WalkthroughThis change fixes a modal stacking issue where the View Profile modal appeared behind the New Collaboration modal. The fix explicitly closes the collaboration modal before opening the profile view, preventing overlapping modals and unintended redirects. Minor formatting adjustments and structural improvements are also applied. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Modal as NewCollaborationModal
participant Profile as ViewProfileModal
User->>Modal: Click "View Profile"
rect rgb(200, 220, 240)
Note over Modal: Fix: Close collaboration modal first
Modal->>Modal: onClose()
end
Modal->>Profile: Open with creator data
User->>Profile: View creator information
Profile-->>User: Profile displayed in front
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Frontend/src/components/collaboration-hub/NewCollaborationModal.tsx(12 hunks)
🔇 Additional comments (2)
Frontend/src/components/collaboration-hub/NewCollaborationModal.tsx (2)
101-106: Good practice to reset state on close.The
onOpenChangehandler properly cleans up modal state when the Dialog is dismissed, which is good for ensuring a fresh start on the next open. However, this aggressive reset may cause issues with the View Profile flow (see comment on lines 177-179).
402-409: Verify focus management for accessibility.The PR objectives mention preventing "focus leakage," but there's no explicit focus management when transitioning between the collaboration modal and profile modal. While the Dialog component may handle focus internally, ensure that:
- Focus moves correctly to the ViewProfileModal when it opens
- Focus returns appropriately when the profile modal closes
- Keyboard navigation (Tab, Escape) works as expected
- Screen readers announce modal transitions properly
Test the modal transitions with keyboard-only navigation and screen readers to confirm accessibility requirements are met, especially since the fix changes the modal flow from overlapping to sequential.
| <Button | ||
| size="sm" | ||
| variant="outline" | ||
| onClick={() => { | ||
| onClose(); | ||
| setSelectedCreator(creator); | ||
| setShowProfile(true); | ||
| }} | ||
| > | ||
| View Profile | ||
| </Button> |
There was a problem hiding this comment.
Critical: State reset causes data loss and breaks onConnect callback.
Calling onClose() before opening the profile modal triggers handleResetModal() (via onOpenChange on line 103), which resets searchTerm to "" and selectedCreator to null. This creates two critical problems:
-
Broken onConnect callback: When the user clicks "Connect" in the ViewProfileModal (line 407),
searchResults[0]will beundefinedbecausesearchResultsis empty (sincesearchTermwas reset). This will cause incorrect behavior. -
User data loss: The user loses all search context, collaboration description, and proposal data when viewing a profile, forcing them to start over.
Recommended approach: Instead of closing the parent modal, use one of these solutions:
- Option 1 (Recommended): Pass the
creatorobject directly toViewProfileModalas a prop, and handle z-index/stacking with CSS (z-50,z-[60], etc.) or a Portal. - Option 2: Store
creatorin a ref or separate state that persists across modal close/open cycles. - Option 3: Only reset state after both modals are fully closed, not during the profile view flow.
Apply this diff for Option 1:
<Button
size="sm"
variant="outline"
onClick={() => {
- onClose();
setSelectedCreator(creator);
setShowProfile(true);
}}
>
View Profile
</Button>Then update ViewProfileModal to accept a creator prop and adjust its z-index styling to appear above the parent modal.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Button | |
| size="sm" | |
| variant="outline" | |
| onClick={() => { | |
| onClose(); | |
| setSelectedCreator(creator); | |
| setShowProfile(true); | |
| }} | |
| > | |
| View Profile | |
| </Button> | |
| <Button | |
| size="sm" | |
| variant="outline" | |
| onClick={() => { | |
| setSelectedCreator(creator); | |
| setShowProfile(true); | |
| }} | |
| > | |
| View Profile | |
| </Button> |
🤖 Prompt for AI Agents
In Frontend/src/components/collaboration-hub/NewCollaborationModal.tsx around
lines 173 to 183, calling onClose() before opening the profile modal resets the
parent modal state and clears search/results causing broken onConnect and data
loss; instead, stop closing the parent modal here and pass the clicked creator
object directly into ViewProfileModal as a new prop
(setSelectedCreator(creator); setShowProfile(true) only), update
ViewProfileModal to accept a creator prop and use it for display and connect
actions (do not rely on searchResults[0]), and ensure the profile modal renders
above the parent by adding appropriate z-index classes or rendering it in a
Portal so the parent modal state remains intact while viewing profiles.
| onConnect={() => { | ||
| setShowProfile(false); | ||
| setSelectedCreator(searchResults[0]); | ||
| }} |
There was a problem hiding this comment.
Critical: Accessing undefined array element.
After the parent modal closes (triggered by onClose() on line 177), searchTerm is reset to "", which makes searchResults an empty array. This callback accesses searchResults[0], which will be undefined, causing selectedCreator to be set to undefined.
This is a direct consequence of the state reset issue identified in lines 173-183.
Fix: Once the state reset issue is resolved (by not closing the parent modal or preserving the creator data), update this callback to use the preserved creator:
<ViewProfileModal
open={showProfile}
onClose={() => setShowProfile(false)}
onConnect={() => {
setShowProfile(false);
- setSelectedCreator(searchResults[0]);
+ // selectedCreator is already set when opening the profile
+ // or pass creator as prop to ViewProfileModal and use it here
}}
/>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Frontend/src/components/collaboration-hub/NewCollaborationModal.tsx around
lines 405 to 408, the onConnect handler directly reads searchResults[0] which
can be undefined after the parent modal closes and searchResults is cleared;
update the handler to avoid accessing searchResults[0] directly by instead using
the preserved creator data (passed via props or kept in a local preservedCreator
state) and fall back to a guarded check (only use searchResults[0] if
searchResults.length > 0) before calling setSelectedCreator; keep the
setShowProfile(false) call but ensure setSelectedCreator receives a valid
creator object (preservedCreator or guarded searchResults[0]) to prevent setting
undefined.
1 participant
###Fixes
#132 – Search result “View Profile” is hidden behind collaboration Dialog .
###Description
This PR fixes the issue where the "View Profile" modal was appearing behind the Collaboration modal and also allowed click-through, causing unexpected behaviour.
###This PR ensures
Parent modal closes before opening View Profile modal
Prevents click-through and focus leakage
Provides consistent modal behaviour across collaboration flow
Summary by CodeRabbit
Bug Fixes
Refactor