-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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(selection-list): tabIndex should respect disabled state #7039
fix(selection-list): tabIndex should respect disabled state #7039
Conversation
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.
Code-wise LGTM, but I'm not sure whether disabling focus on the list is the way to go since it's also a presentational component. Thoughts @jelbourn?
@crisbeto Not too sure about that. The selection list seems more intended to be an interactive control than for presentation. But I see what you mean. Maybe in that case it would make more sense to use a normal list. |
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, selection-list is specifically a form contorl (listbox), so this should be the right behavior
30a6efc
to
9589b3c
Compare
9589b3c
to
e199178
Compare
e199178
to
8a68ed4
Compare
16dd5ac
to
c3d9672
Compare
Needs rebase. |
c3d9672
to
86b3151
Compare
@devversion Rebase one more time? |
* Currently if the selection-list is disabled, the tabIndex may be still set to a valid value that allows tabbing to the element. The `mixinTabIndex` respects the disabled state of the selection list.
86b3151
to
e46a98d
Compare
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
mixinTabIndex
respects the disabled state of the selection list.