Tabs: Don't allow navigating to hidden tabs #1724
Conversation
Fixes #15013
|
@scottgonzalez, thanks for your PR! By analyzing the annotation information on this pull request, we identified @apsdehal, @arschmitz and @jzaefferer to be potential reviewers |
| return false; | ||
| } | ||
|
|
||
| if ( that.tabs.eq( index ).is( ":hidden" ) ) { |
There was a problem hiding this comment.
How about...
return that.tabs.eq( index ).is( ":visible" )
There was a problem hiding this comment.
When there are multiple failure cases, I generally like to list them out individually as failure cases, then return true at the end, but I'm fine making this change.
There was a problem hiding this comment.
Just a thought but the :visible and :hidden selectors are pretty slow and not 100% reliable. We don't document that hidden tabs are automatically not navigable. Would it perhaps be better to leave this change out and document that you should mark hidden tabs as disabled?
There was a problem hiding this comment.
Actually, for some reason switching it causes the tests to fail.
There was a problem hiding this comment.
We don't document that hidden tabs are automatically not navigable.
Why would anyone ever expect a hidden tab to be navigable?
There was a problem hiding this comment.
I don't think they would which is why i suggested documenting that hidden tabs should be marked disabled
There was a problem hiding this comment.
|
Besides that one thing, 👍 |
|
I'm going to close this because this will result in an odd state of tabs not being marked as disabled, but also not being able to be activated properly. Instead, we should implement the hidden detection inside |
Fixes #15013