Components: A button ripple effect appears again after a dialog is closed

Created on 13 Nov 2017  路  31Comments  路  Source: angular/components

Bug, feature request, or proposal:

A bug.

What is the expected behavior?

The ripple darkening effect shouldn't appear on a button after a dialog is closed.

What is the current behavior?

A ripple / CSS effect appears on the 'invoking' button again after the dialog is closed. Should be only one one animation cycle when the button is pressed.

What are the steps to reproduce?

From:
https://material.angular.io/components/dialog/overview

On:
Firefox 56.0.2
Chrome 61.0.3163.100

Pressing the "Pick one" button, the ripple effect is played and the dialog is opened, then closing the dialog in any way return the effect on the button.

https://i.imgur.com/oOmbAmS.gif

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

It's visually 'bugged'

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

It appears in both local build
Angular 5
Material 5.0.0-rc0

And with a refreshed documentation / example page

Is there anything else we should know?

G P3 materiadialog has pr

Most helpful comment

You can add restoreFocus: false to the popup configuration, when you're creating it.

  • Whether the dialog should restore focus to the
  • previously-focused element, after it's closed.

All 31 comments

What you're seeing is the focus state for the button. It is normally supposed to only show when focus came from a keyboard interaction, but here it is happening when the triggering action is a mouse event.

@jelbourn showing it both for keyboard and programmatic focus was intentional, otherwise restoring focus from dialogs or sidenavs won't be visible. IMO program and keyboard focus should behave the same, otherwise users won't be able to see where focus is if we move it for them. It's different for mouse since people would already be following the cursor when they moved focus.

@jelbourn I think there are two solutions here:

  • Focus properly from the dialog, drawer using the FocusMonitor#focusVia. This involves a way of detecting the origin type that opened the dialog, drawer (hard to do right now)

  • Only show the focus styles for keyboard users (like we do with the checkbox, radio, etc. right now).

I chatted with @crisbeto about this and we think that solution 2) can be problematic for users with keyboard. The dialog, drawer will just restore focus using program and there won't be any focus indicator for keyboard users at all.

A _better solution_ would be just keeping it as it is and prioritizing accessibility over the UX here. If we go that way I'll update the checkbox, radio, slide-toggle to also show the focus indicators for program.

What about adding an optional openedVia argument to the open method on these components. If its not specified we assume it was keyboard, if it is specified we just use whatever they gave us. I like this better because:
1) It doesn't affect other situations where the components may be program-focused
2) Users can just use the FocusMonitor and pass the needed info along if its important to them

I was thinking we could show the focus indicator whenever the dialog open action _or_ the dialog closing action came from the keyboard.

We can also do it like @mmalerba described. This way we still give developers the flexibility to control whether the "original" element should have a focus indicator or not, but accessibility is prioritized by default.

For the dialog close action, I don't think there is an easy way to detect whether it came from the keyboard or not. In theory we could add a new option to the close() method and also register the FocusMonitor on the matDialogClose directive. But I'm not sure about that, it feels a bit too complicated for the developers in the end.

A user-facing API for this seems more cumbersome than anything. We can easily categorize events from mat-dialog-close directive, escape key events, and backdrop clicks. If the developer just calls the close method, we treat is as programmatic.

Yeah that sounds good (escape keys, backdrop clicks, dialog-close directive). I'm just a bit concerned about always treating the close() method as programmatic.

I just think, a lot of developers won't really think that the matDialogClose directive or close() method have a different result in the focus restoring (or accessibility).

Also, I think under the hood the matDialogClose or backdrop click observable would still somehow need to call the close method of the ref, so I think it would be worth having a method like _closeWithOrigin

https://github.com/angular/material2/blob/c21636bcee7ce71059f1acd9de95859825bdd6db/src/lib/dialog/dialog.ts#L230-L234

Yeah, we would need a new internal API at the very least. That can be a first step if we still want to think about a public api after after that.

I noticed an issue when adding the FocusMonitor to the matDialogClose buttons. Since those buttons are likely going to be auto-focused upon dialog open, the focus origin will be program, and another click on the button (using mouse), to close the dialog, won't trigger a new FocusOrigin (mouse)

This means that it will be incorrectly set to program. Other than that, the backdrop click, escape press should work fine.

For anyone searching, working around it is pretty simple: Just force mouse focus:

    matDialogRef.afterClosed().subscribe(() => {
      this.dialogOpenButton.nativeElement.classList.remove('cdk-program-focused');
      this.dialogOpenButton.nativeElement.classList.add('cdk-mouse-focused');
    });

You get a reference to the button using @ViewChild and the MatDialogRef is returned from dialog.open

Of course, my workaround will _always_ use mouse focus, which might not be desired, but 9/10 (if not more) is going to use the mouse to open the dialog, so to me it less confusing to most than always showing it (the escape or enter key is often used to close the dialog, but that is pretty irrelevant, IMHO. The important distinction is how it was opened)

Found a solution thanks to @LosD, a bit hacky but it works.

@ViewChild("button") private button: ElementRef;
//
ngAfterViewInit() {
  this.sidenav.openedChange.subscribe(() => {
    this.fixButtonRippleEffectBug(this.button);
  });
}

fixButtonRippleEffectBug(button) {
  button._elementRef.nativeElement.classList.remove("cdk-program-focused");
  button._elementRef.nativeElement.classList.add("cdk-mouse-focused");
}

Edit: Original solution wasn't working, this one should should be fine

Hi,

Has this issue been resolved?

I see what there is a PR opened related to this issue but it failed.

https://github.com/angular/material2/pull/9257

Triggering "blur" also acts as a workaround.

  • onclick="this.blur()" in the template
    OR
  • button._elementRef.nativeElement.blur() in the component.ts

Found a solution thanks to @LosD & @azdanov.
It works for Angular 6

dialogRef.afterClosed().subscribe(result => {
  this.fixButtonRippleEffectBug();
});

private fixButtonRippleEffectBug() {
let buttonList = document.getElementsByClassName("btn-delete")
  let buttonArray = [].slice.call(buttonList);
  for(let currentButton of buttonArray) {
    currentButton.classList.remove('cdk-program-focused');
    currentButton.classList.add('cdk-mouse-focused');
  }
}

This does not instill confidence... I'm new to Ng and hitting this issue right off the bat for mat-icon-button in mat-sidenav straight out of angular material schematics. Looking forward to the real fix!

@vivatum your workaround fails when user goes to different tab in chrome then comes back to page (in my mat-sidenav case at least)... I think I also need to subscribe to window.onfocus as well (which seems to be another rabbit hole)...

@fizxmike Try using my solution with blur on click.

For me a workaround was to use ViewChild to grab the button and call blur() after closing the dialog.
<button mat-icon-button #removeAllBtn> <mat-icon>delete</mat-icon> </button>

@ViewChild('removeAllBtn') removeAllBtn: ElementRef<MatButton>;

dialogRef.afterClosed().subscribe(data => { (this.removeAllBtn as any)._getHostElement().blur() })

馃 I see google play music web app has the same problem (edge case with fix in place where you leave the page/tab and come back: button ripple color is stuck!).
image
... actually happens every other switch back (leave, come back: fine. leave, come back: ripple color stuck on button.) Cute.

CSS only solution

I'm not sure if the css class I'm about to mention didn't exist before, but here's a simple solution (note uise of ::ng-deep) :

/* Use the button type you're using here */
button.mat-stroked-button
{
    &.cdk-program-focused 
    {
        ::ng-deep .mat-button-focus-overlay
        {
            opacity: 0;  // hide overlay
        }
    }
}

This will only hide the overlay (which is what makes a button appear focused) if the focus was triggered programatically. My scenario is a two adjacent buttons that trigger two informational onlly dialogs, so when closing the dialog it looked super weird to have the button still highlighted.

@simeyla Unfortunately, that's only a temporary solution, as ng-deep is scheduled to be removed: https://angular.io/guide/component-styles#deprecated-deep--and-ng-deep

However, it might still be a good solution: Just put it in styles.(s)css instead of the components own CSS rules, then it shouldn't be needed.

You can add restoreFocus: false to the popup configuration, when you're creating it.

  • Whether the dialog should restore focus to the
  • previously-focused element, after it's closed.

@VladislavLobakh the problem is not the API naming, it's about a rock solid implementation that works in all imaginable border cases, does not have side-effects and does not create overhead.

I just came aware of the focus issue and this somehow should get fixed!

@VladislavLobakh

You can add restoreFocus: false to the popup configuration, when you're creating it.

after some experiments I cannot say I can trust it at all. It works in 50% of the cases and I cannot even figure out the pattern when it does and when does not work.

@andreElrico @smnbbrv MatDialogConfig has a propery "restoreFocus". It gives you the ability: Whether the dialog should restore focus to the previously-focused element, after it's closed. It's working and it's easy to understand.

It鈥檚 not working, that鈥檚 the problem

@smnbbrv

In my case, the real problem stay in button structure, 'material' build various sub components and last one is a 'div' with css class 'mat-button-focus-overlay':

My solution is simply, in 'style.css' file, add or sobrescribe the 'mat-button-focus-overlay'

.mat-button-focus-overlay { background-color: transparent!important; }

@pablomario thanks! this one works. However, this issue is not something that should be somehow fixed on the end developer side. This is just annoying. The dialogs are in 99% of the cases a reaction of a system on some action. That, of course, can be a navigation, but the most of dialogs are a result of clicking a button.

not sure why this seems to work effectively but:

dialogRef.afterClosed().subscribe(result => {
      setTimeout(() => {
        document.getElementById('<buttonID>').focus();
      }, 0);
      console.log('result: ', result);
    });

Using .blur() did not remove the overlay, but immediately invoking .focus() does...?

In my case, solved the problem this:
.cdk-program-focused { display: none; }

restoreFocus: false -----> Working for me, best solution

Was this page helpful?
0 / 5 - 0 ratings

Related issues

dzrust picture dzrust  路  3Comments

jelbourn picture jelbourn  路  3Comments

xtianus79 picture xtianus79  路  3Comments

Miiekeee picture Miiekeee  路  3Comments

vitaly-t picture vitaly-t  路  3Comments