Skip to content

Conversation

@devversion
Copy link
Member

  • 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.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Sep 13, 2017
Copy link
Member

@crisbeto crisbeto left a 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?

@devversion
Copy link
Member Author

devversion commented Sep 13, 2017

@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.

Copy link
Member

@jelbourn jelbourn left a 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

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Sep 13, 2017
@devversion devversion force-pushed the fix/selection-list-disabled-state-tabindex branch from 30a6efc to 9589b3c Compare September 16, 2017 13:11
@devversion devversion added the P2 The issue is important to a large percentage of users, with a workaround label Sep 18, 2017
@devversion devversion force-pushed the fix/selection-list-disabled-state-tabindex branch from 9589b3c to e199178 Compare September 21, 2017 15:32
@devversion devversion added P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful and removed P2 The issue is important to a large percentage of users, with a workaround labels Sep 21, 2017
@devversion devversion force-pushed the fix/selection-list-disabled-state-tabindex branch from e199178 to 8a68ed4 Compare September 22, 2017 14:54
@devversion devversion added P2 The issue is important to a large percentage of users, with a workaround and removed P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful labels Sep 27, 2017
@devversion devversion force-pushed the fix/selection-list-disabled-state-tabindex branch 2 times, most recently from 16dd5ac to c3d9672 Compare September 29, 2017 13:28
@andrewseguin
Copy link
Contributor

Needs rebase.

@andrewseguin andrewseguin added pr: needs rebase and removed action: merge The PR is ready for merge by the caretaker labels Sep 29, 2017
@devversion devversion force-pushed the fix/selection-list-disabled-state-tabindex branch from c3d9672 to 86b3151 Compare September 30, 2017 08:14
@devversion devversion added action: merge The PR is ready for merge by the caretaker and removed pr: needs rebase labels Sep 30, 2017
@kara
Copy link
Contributor

kara commented Oct 4, 2017

@devversion Rebase one more time?

@kara kara added pr: needs rebase and removed action: merge The PR is ready for merge by the caretaker labels Oct 4, 2017
* 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.
@devversion devversion force-pushed the fix/selection-list-disabled-state-tabindex branch from 86b3151 to e46a98d Compare October 5, 2017 15:13
@kara kara merged commit c2a9516 into angular:master Oct 5, 2017
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes PR author has agreed to Google's Contributor License Agreement P2 The issue is important to a large percentage of users, with a workaround

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants