A button ripple effect appears again after a dialog is closed

[Eddygn]

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?

[jelbourn]

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.

[crisbeto]

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

[devversion]

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

[mmalerba]

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

[jelbourn]

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

[devversion]

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.

[jelbourn]

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.

[devversion]

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

[jelbourn]

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.

[devversion]

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.

[LosD]

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)

[VladislavLobakh]

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.

[andreElrico]

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

[smnbbrv]

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

[VladislavLobakh]

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

[smnbbrv]

It’s not working, that’s the problem

[pablomario]

@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; }

[smnbbrv]

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

[Crateros]

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

[botika21]

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

[michele6000]

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

[harsh177]

best solution

[lano-vargas]

I have a similar case, but I have mat radio in a popup modal when the model is triggered the first radion always has the ripples. I have yes\no and set no as default even if isn't set it always the first one.

Any ideas?