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

mat-menu trigger button is highlighted after closing menu #9292

Closed
LosD opened this issue Jan 8, 2018 · 6 comments · Fixed by #9303
Closed

mat-menu trigger button is highlighted after closing menu #9292

LosD opened this issue Jan 8, 2018 · 6 comments · Fixed by #9303
Assignees
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@LosD
Copy link

LosD commented Jan 8, 2018

Bug, feature request, or proposal:

Bug

What is the expected behavior?

Trigger button acts like other buttons: When action has completed, button returns to the neutral state.
image

... Actual keyboard focus stays, but the styling is back to neutral, when using the mouse to click (for keyboard navigation it stays lit... Which makes sense).

What is the current behavior?

Trigger button stay focused, and keeps the focused style.
image

What are the steps to reproduce?

  • Open https://stackblitz.com/edit/angular-material2-issue-tscawk
  • Click basic button.
  • Observe state of button after animation finishes.
  • Attach a menu trigger to a button.
  • Click button that is a mat-menu trigger.
  • Close menu using backdrop or buttons
  • Observe state of button, comparing with basic button.

What is the use-case or motivation for changing an existing behavior?

Consistency. Other buttons does not keep the focus styling, menu triggers shouldn't either.

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Seen on:
Angular 5.0.0, 5.1.3
Material 5.0.2, 5.0.3
Linux, Windows
TypeScript 2.5.3 + whatever TypeScript is running on StackBlitz
Internet Explorer 11, Chrome 63.0.3239.84, FireFox 57.0.3, EdgeHTML 16.16299

Is there anything else we should know?

May be related to fix in #8348 (don't really think so, though, I think I've seen this behavior for a long time. It used to have the same problem when expanding sidebars using a button)

@julianobrasil
Copy link
Contributor

This is not a bug, it's working as expected: #9252 (comment)

You can set the focus in other element on closed event.

@LosD
Copy link
Author

LosD commented Jan 9, 2018

Well, the actual issue isn't the keyboard focus itself, but that the button has the focus style after clicking, whereas other buttons does not, unless you use the keyboard (where it makes sense). That a menu is attached should be irrelevant to how the button is styled after clicking. I'll edit the issue to reflect that.

But thanks for the workaround (though I hope to remove the styling, not the actual keyboard focus)!

@LosD LosD changed the title mat-menu trigger button is highlighed/focused after closing menu mat-menu trigger button is highlighed after closing menu Jan 9, 2018
@LosD LosD changed the title mat-menu trigger button is highlighed after closing menu mat-menu trigger button is highlighted after closing menu Jan 9, 2018
@LosD
Copy link
Author

LosD commented Jan 9, 2018

It seems like the wrong class is applied when closing the menu, here's my workaround (attached to the close handler of the menu, not the trigger, as the trigger event does not specify the source):

  menuClosed(event: void | 'click' | 'keydown') {
    if (event !== 'keydown') { // void is clicking on the backdrop
      window.setTimeout(() => {
        this.favoriteMenuTrigger.nativeElement.classList.remove('cdk-program-focused');
        this.favoriteMenuTrigger.nativeElement.classList.add('cdk-mouse-focused');
      }, 0);
    }
  }

Not entirely sure why the timeout is needed, but without it, cdk-program-focused returns.

@julianobrasil
Copy link
Contributor

julianobrasil commented Jan 9, 2018

Nice to hear that you managed to do it, @LosD!

But what you did is exactly what I would do. Other approach (if you don't wanna rely on classes names and don't wanna keep focus), is create another focusable component out of the end user sight:

<input style="position: fixed; top: -2000; left; -2000" #elToFocus>

and, in code do something like (inside of your setTimeout):

this._elToFocus.nativeElement.focus();

The setTimeout is necessary because you want to assure your actions will be excecuted after all Material 2 actions take place, so you send it to the end of the line in the event loop queue (that's what setTimeout does when you use 0 as the second parameter).

@LosD
Copy link
Author

LosD commented Jan 9, 2018

@julianobrasil The problem with changing the actual focus is that it messes with the user's tab position, so going back to the keyboard can be confusing.

I understand what setTimeout(fn, 0) does, it just surprised me that the event is raised before Angular Material is done doing its thing, but there's probably good reasons for that.

@crisbeto crisbeto self-assigned this Jan 9, 2018
@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent has pr labels Jan 9, 2018
crisbeto added a commit to crisbeto/material2 that referenced this issue Jan 9, 2018
Sets the correct focus origin depending on the way a menu has been opened.

Fixes angular#9292.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jan 10, 2018
Sets the correct focus origin depending on the way a menu has been opened.

Fixes angular#9292.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jan 10, 2018
Sets the correct focus origin depending on the way a menu has been opened.

Fixes angular#9292.
jelbourn pushed a commit that referenced this issue Jan 21, 2018
Sets the correct focus origin depending on the way a menu has been opened.

Fixes #9292.
jelbourn pushed a commit that referenced this issue Jan 23, 2018
Sets the correct focus origin depending on the way a menu has been opened.

Fixes #9292.
jelbourn pushed a commit to jelbourn/components that referenced this issue 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
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants