[WIP] feat: add switch account to oauth page#9838
Closed
rdmclin2 wants to merge 4 commits into
Closed
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR introduces a “Switch Account” feature on the OAuth consent login page by adding the necessary UI, state management, logout handling, and translation support. Sequence diagram for Switch Account interaction on OAuth consent pagesequenceDiagram
actor User
participant "OAuth Consent Page"
participant "UserStore (logout)"
User->>"OAuth Consent Page": Clicks 'Switch Account'
"OAuth Consent Page"->>"UserStore (logout)": Call logout()
"UserStore (logout)"-->>"OAuth Consent Page": Logout complete
"OAuth Consent Page"-->>User: Redirect to login page
Class diagram for updated LoginConfirmClient component with Switch Account featureclassDiagram
class LoginConfirmClient {
+uid
+clientMetadata
+isSwitching : boolean
+handleSwitchAccount()
+logout()
}
class UserStore {
+isUserStateInit
+userAvatar
+nickName
+logout()
}
LoginConfirmClient --> UserStore : uses
Class diagram for updated OAuth translation keysclassDiagram
class OAuthTranslations {
+button : string
+description : string
+switchAccount : string
+title : string
+userWelcome : string
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
TestGru AssignmentSummary
Tip You can |
Member
|
Thank you for raising your pull request and contributing to our Community |
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- After logout completes, make sure to redirect or refresh the page to bring the user back to the sign-in flow, since calling logout alone won’t update the UI.
- While switching accounts, disable or hide the primary “Confirm Login” button to prevent overlapping actions during the logout process.
- Add error handling in handleSwitchAccount to catch logout failures and surface an appropriate user-facing notification.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- After logout completes, make sure to redirect or refresh the page to bring the user back to the sign-in flow, since calling logout alone won’t update the UI.
- While switching accounts, disable or hide the primary “Confirm Login” button to prevent overlapping actions during the logout process.
- Add error handling in handleSwitchAccount to catch logout failures and surface an appropriate user-facing notification.
## Individual Comments
### Comment 1
<location> `src/app/[variants]/oauth/consent/[uid]/Login.tsx:132-134` </location>
<code_context>
{buttonText}
</Button>
</form>
+ <Button loading={isSwitching} onClick={handleSwitchAccount} type="text">
+ {switchButtonText}
+ </Button>
</code_context>
<issue_to_address>
**suggestion:** Review the use of type="text" for the Button component.
If the Button is not meant for form submission, use type="button" or omit the type for better semantics and accessibility.
```suggestion
<Button loading={isSwitching} onClick={handleSwitchAccount} type="button">
{switchButtonText}
</Button>
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+132
to
+134
| <Button loading={isSwitching} onClick={handleSwitchAccount} type="text"> | ||
| {switchButtonText} | ||
| </Button> |
Contributor
There was a problem hiding this comment.
suggestion: Review the use of type="text" for the Button component.
If the Button is not meant for form submission, use type="button" or omit the type for better semantics and accessibility.
Suggested change
| <Button loading={isSwitching} onClick={handleSwitchAccount} type="text"> | |
| {switchButtonText} | |
| </Button> | |
| <Button loading={isSwitching} onClick={handleSwitchAccount} type="button"> | |
| {switchButtonText} | |
| </Button> |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #9838 +/- ##
========================================
Coverage 84.55% 84.55%
========================================
Files 944 944
Lines 64537 64537
Branches 9585 9299 -286
========================================
Hits 54567 54567
Misses 9970 9970
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
5e1b82f to
15819a0
Compare
15819a0 to
146fc58
Compare
1 task
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.
💻 Change Type
🔀 Description of Change
📝 Additional Information
Summary by Sourcery
Enable users to log out and switch to a different account from the OAuth consent login page
New Features: