-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix tab panel initial tab selection #47100
Conversation
|
||
// If the currently selected tab becomes disabled, select the first enabled tab. | ||
// (if there is one). | ||
if ( firstEnabledTab ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous code didn't do so, but the component should probably consider a case where all tabs are disabled.
It might be handled elsewhere in the component though, I haven't really checked.
30ad7a3
to
9103fe2
Compare
Flaky tests detected in 9103fe2. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/3899933660
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thank you both for flagging the issue and working on a fix so quickly 🙏 |
What?
Fixes #47079
Why?
From what I can tell there are two causes of the bug:
tabs
array on later renders.tabs
array.I did try looking to see if this could be fixed by always rendering the tab navigation block, but it's less than straightforward. The tab is added somewhat dynamically when there are fills in a slot. I struggled to see where the issue was coming from.
How?
This fixes the issue. If
initalTabName
is declared, but there's no matching tab in thetabs
array, the effect no longer makes a selection until it is in the tabs array.This does leave a situation where
initalTabName
could be declared and the matching tab is never present, but I'd consider this implementor error (and the same issue could happen before #46471).I have refactored the code a bit here too. I personally found the code a little hard to understand with the added logic, so it's now a little more verbose, but hopefully easier to follow.
Testing Instructions
Expected - the List View tab should be selected again on the navigation block.
In trunk - the Appearance tab is selected.
Screenshots or screencast
Before
Kapture.2023-01-12.at.14.18.34.mp4
After
Kapture.2023-01-12.at.14.16.22.mp4