Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

fix(menu): focus first non disabled item #9228

Merged
merged 1 commit into from
Sep 21, 2016

Conversation

clshortfuse
Copy link
Contributor

Iterate through menu items until it finds a non disabled item

Previously, menu would attempt to focus first item. If the item
was disabled, no attempts were made to seek another focusable item.

Fixes #9165

@topherfangio
Copy link
Contributor

Is it possible to add some tests for this? It looks like we already have some autofocus-specific ones.

https://github.com/angular/material/blob/master/src/components/menu/menu.spec.js#L155

@clshortfuse clshortfuse added the needs: unit tests This PR needs unit tests to cover the changes being proposed label Aug 4, 2016
@ThomasBurleson
Copy link
Contributor

@topherfangio - please pair with Carlos for unit tests.

@ThomasBurleson ThomasBurleson added this to the 1.1.2 milestone Aug 22, 2016
@ThomasBurleson ThomasBurleson added the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Sep 7, 2016
@clshortfuse clshortfuse force-pushed the menu-focusFirstEnabled branch 3 times, most recently from d640c60 to d2d02d3 Compare September 11, 2016 19:23
@clshortfuse clshortfuse added needs: review This PR is waiting on review from the team and removed in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs needs: unit tests This PR needs unit tests to cover the changes being proposed labels Sep 11, 2016
@clshortfuse
Copy link
Contributor Author

@topherfangio please review :)

Previously, menu would attempt to focus first item. If the item
was disabled, no attempts were made to seek another focusable item.

* Iterate through menu items until it finds a non-disabled item
* Add more menu items to autofocus spec tests

Fixes angular#9165
@clshortfuse clshortfuse force-pushed the menu-focusFirstEnabled branch from d2d02d3 to f496ff5 Compare September 11, 2016 19:33
@topherfangio topherfangio added needs: presubmit and removed needs: review This PR is waiting on review from the team labels Sep 12, 2016
@topherfangio topherfangio removed their assignment Sep 12, 2016
@topherfangio
Copy link
Contributor

Manually tested and checked code. LGTM 👍

@jelbourn jelbourn merged commit 1f32ccb into angular:master Sep 21, 2016
Frank3K pushed a commit to Frank3K/material that referenced this pull request Oct 8, 2016
Previously, menu would attempt to focus first item. If the item
was disabled, no attempts were made to seek another focusable item.

* Iterate through menu items until it finds a non-disabled item
* Add more menu items to autofocus spec tests

Fixes angular#9165
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants