fix(Authenticator): clicking on current tab switches tabs #5912
Conversation
🦋 Changeset detectedLatest commit: 21e1e12 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
hbuchel
changed the title
| onValueChange={() => (route === 'signIn' ? toSignUp() : toSignIn())} | ||
| onValueChange={(prevRoute) => { | ||
| if (prevRoute !== route) { | ||
| route === 'signIn' ? toSignUp() : toSignIn(); |
There was a problem hiding this comment.
I don't know why, but this is hurting my brain. Is the route the tab I clicked on or the route I'm currently at? So if I click any tab then it will essentially toggle to the other one? Why is comparing the prevRoute to the current route necessary?
There was a problem hiding this comment.
I think these variable names are switched. It looks like route is the active tab and prevRoute is the clicked tab.
So the old logic was just "if Sign In is the active tab go to Sign Up, otherwise go to Sign In", which results in always going to the other tab no matter what is clicked.
And the new logic is just adding the check of "if the clicked tab isn't the active tab...", which prevents changing tabs if you click on the already active tab, but is still just setting the tab without using the value from what was clicked. I think that logic still makes sense for the component, but imo prevRoute should just be changed to something like nextRoute, newRoute, etc.
There was a problem hiding this comment.
newRoute might be the more consistent name, since TabContainer's setActiveTab() is using the name newValue for the argument passed to onValueChange()
There was a problem hiding this comment.
Updated in latest to use newRoute instead, that makes more sense for the actual logic happening.
Description of changes
Fixes an issue where clicking on the currently open Authenticator tab switches to the other tab. i.e. clicking on the "Sign in" tab while it is already active will no longer cause the "Create account" tab to open.
before
CleanShot.2024-10-17.at.14.25.53.mp4
after
CleanShot.2024-10-17.at.14.26.22.mp4
Issue #, if available
Fixes: #5911
Description of how you validated changes
Verified in local docs examples
Checklist
yarn testpasses and tests are updated/addeddocs,e2e,examples, or other private packages.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.