fix: close connect modal after social login completes#599
Merged
Agilulfo1820 merged 2 commits intomainfrom Mar 10, 2026
Merged
Conversation
Move auto-close logic from MainContent to ConnectModal so it runs regardless of which content view is displayed. Previously, when a social login button was clicked, the modal switched to LoadingContent which unmounted MainContent and its auto-close effect, causing the modal to stay open even after successful authentication. https://claude.ai/code/session_013M6uC6KCxFt8kV7ccu9EEZ
📝 WalkthroughWalkthroughMoved wallet-connection monitoring and auto-close behavior from MainContent into the parent ConnectModal; MainContent no longer accepts Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CM as ConnectModal
participant MC as MainContent
participant W as WalletHook
rect rgba(135,206,250,0.5)
U->>CM: open modal
CM->>MC: render MainContent
end
rect rgba(144,238,144,0.5)
W-->>CM: connection.isConnected = true
CM->>CM: useEffect detects connection
CM->>U: onClose() (modal closes)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vechain-kit/src/components/ConnectModal/Contents/MainContent.tsx (1)
20-27:⚠️ Potential issue | 🟠 MajorRemove the dead
onCloseprop fromMainContent.
onCloseis no longer used after moving the auto-close effect intoConnectModal, and ESLint is already flagging this at Line 27. Drop it fromPropsand from theConnectModalcall sites so this refactor doesn't leave a lint-blocking error behind.✂️ Proposed cleanup
type Props = { setCurrentContent: React.Dispatch< React.SetStateAction<ConnectModalContentsTypes> >; - onClose: () => void; }; -export const MainContent = ({ setCurrentContent, onClose }: Props) => { +export const MainContent = ({ setCurrentContent }: Props) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vechain-kit/src/components/ConnectModal/Contents/MainContent.tsx` around lines 20 - 27, Remove the unused onClose prop from the MainContent component: update the Props type to drop onClose and change the MainContent signature to only accept { setCurrentContent }: Props; then remove onClose from every ConnectModal -> MainContent call site so callers no longer pass onClose when rendering MainContent (search for MainContent and its JSX usages) and run lint to ensure no remaining references remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/vechain-kit/src/components/ConnectModal/Contents/MainContent.tsx`:
- Around line 20-27: Remove the unused onClose prop from the MainContent
component: update the Props type to drop onClose and change the MainContent
signature to only accept { setCurrentContent }: Props; then remove onClose from
every ConnectModal -> MainContent call site so callers no longer pass onClose
when rendering MainContent (search for MainContent and its JSX usages) and run
lint to ensure no remaining references remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f54ab0b3-725b-4c11-b8f8-4be43a429b0a
📒 Files selected for processing (2)
packages/vechain-kit/src/components/ConnectModal/ConnectModal.tsxpackages/vechain-kit/src/components/ConnectModal/Contents/MainContent.tsx
|
Size Change: -511 B (-0.01%) Total Size: 7.6 MB
ℹ️ View Unchanged
|
Clean up the unused onClose prop that remained after moving auto-close logic to ConnectModal. Bump package version to 2.6.4. https://claude.ai/code/session_013M6uC6KCxFt8kV7ccu9EEZ
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/vechain-kit/src/components/ConnectModal/Contents/MainContent.tsx (1)
20-26: Add a regression test for the loading-content handoff.This fix depends on the modal still observing connection state after
MainContentunmounts. A component test that switches into the social-login loading view and then asserts the modal closes on successful auth would help keep this from regressing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vechain-kit/src/components/ConnectModal/Contents/MainContent.tsx` around lines 20 - 26, Add a regression test that mounts the ConnectModal and verifies the modal still observes connection state after MainContent unmounts: simulate user interaction that calls MainContent's setCurrentContent to switch into the social-login loading view, then mock a successful auth/connection event and assert the ConnectModal closes; the test should specifically target the handoff from MainContent to the loading view (referencing MainContent and setCurrentContent) and confirm the modal reacts to connection state changes even after MainContent is unmounted to prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/vechain-kit/src/components/ConnectModal/Contents/MainContent.tsx`:
- Around line 20-26: Add a regression test that mounts the ConnectModal and
verifies the modal still observes connection state after MainContent unmounts:
simulate user interaction that calls MainContent's setCurrentContent to switch
into the social-login loading view, then mock a successful auth/connection event
and assert the ConnectModal closes; the test should specifically target the
handoff from MainContent to the loading view (referencing MainContent and
setCurrentContent) and confirm the modal reacts to connection state changes even
after MainContent is unmounted to prevent regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 863c06b5-da00-4b90-87b2-187b6bfd92aa
📒 Files selected for processing (3)
packages/vechain-kit/package.jsonpackages/vechain-kit/src/components/ConnectModal/ConnectModal.tsxpackages/vechain-kit/src/components/ConnectModal/Contents/MainContent.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/vechain-kit/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/vechain-kit/src/components/ConnectModal/ConnectModal.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Move auto-close logic from MainContent to ConnectModal so it runs
regardless of which content view is displayed. Previously, when a
social login button was clicked, the modal switched to LoadingContent
which unmounted MainContent and its auto-close effect, causing the
modal to stay open even after successful authentication.
https://claude.ai/code/session_013M6uC6KCxFt8kV7ccu9EEZ
Summary by CodeRabbit