A bug.
The ripple darkening effect shouldn't appear on a button after a dialog is closed.
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.
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
It's visually 'bugged'
It appears in both local build
Angular 5
Material 5.0.0-rc0
And with a refreshed documentation / example page
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
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.
Triggering "blur" also acts as a workaround.
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!).
... 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
Most helpful comment
You can add
restoreFocus: false
to the popup configuration, when you're creating it.