Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(menu): set appropriate origin when restoring focus #9303

Merged
merged 1 commit into from
Jan 23, 2018

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Jan 9, 2018

Sets the correct focus origin depending on the way a menu has been opened.

Fixes #9292.

@crisbeto crisbeto added pr: needs review Accessibility This issue is related to accessibility (a11y) labels Jan 9, 2018
@crisbeto crisbeto requested a review from kara as a code owner January 9, 2018 18:47
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jan 9, 2018
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

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Jan 10, 2018
@crisbeto crisbeto force-pushed the 9292/menu-focus-origin-restore branch from 30cced2 to b9b329d Compare January 10, 2018 17:40
@crisbeto crisbeto requested a review from devversion as a code owner January 10, 2018 17:40
@crisbeto
Copy link
Member Author

Included the patchElementFocus changes from #9257 to help with some flaky IE tests.

@crisbeto crisbeto added the target: patch This PR is targeted for the next patch release label Jan 10, 2018
@@ -208,8 +211,12 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
}

/** Focuses the menu trigger. */
focus() {
this._element.nativeElement.focus();
focus(origin: FocusOrigin = 'program') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a @param JsDoc

@@ -274,8 +281,10 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
// We should reset focus if the user is navigating using a keyboard or
// if we have a top-level trigger which might cause focus to be lost
// when clicking on the backdrop.
if (!this._openedByMouse || !this.triggersSubmenu()) {
if (!this._openedByMouse) {
this.focus();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment that focus style will show for both program and keyboard, so we don't have to specify which?

Sets the correct focus origin depending on the way a menu has been opened.

Fixes angular#9292.
@crisbeto crisbeto force-pushed the 9292/menu-focus-origin-restore branch from b9b329d to 612d98b Compare January 10, 2018 19:39
@jelbourn jelbourn merged commit 278e25a into angular:master Jan 23, 2018
jelbourn pushed a commit to jelbourn/components that referenced this pull request Jan 29, 2018
Sets the correct focus origin depending on the way a menu has been opened.

Fixes angular#9292.
@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 8, 2019
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Accessibility This issue is related to accessibility (a11y) action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mat-menu trigger button is highlighted after closing menu
3 participants