Return focus after dialogs close

[steverep]

Proposed change

Adds an event handler to return focus, where possible, to the button or control that opened the dialog. This is best practice for good accessibility. Here are some details on the implementation:

• Finds and stores the active element and possibly dialog generically right before new dialog is shown
• Data is stored in the module in tag dictionary same as created elements, and typing was added for better code quality
• Handles the common special case where dialog is opened from a popup menu and returns focus to the trigger
• Handles the replace situation for more info / entity settings
• When dialog is opened from another dialog, focus method for mwc-dialog becomes the fallback in case of failure

Limitations

1. Assumes the dialog fires the dialog-closed event. The majority of the 70+ dialogs do, but about 20 or so need to be updated in future changes.
2. Assumes the element that opened the dialog is focusable, which is not always the case, but should be for keyboard accessibility (e.g. entity settings dialogs from configuration page and some more info dialogs from certain cards)

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New feature (thank you!)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Example configuration

Additional information

• This PR fixes or closes issue: Modal dialogs need keyboard focus management #10754
• This PR is related to issue or discussion:
• Link to documentation pull request:

Checklist

• The code change is tested and works locally.
• There is no commented out code in this PR.
• Tests have been added to verify that the new code works.

[steverep]

Approvals is slower than usual... something I did? 😕 😄

[zsarnett]

Bram is on Holiday 😄 I haven't been able to take a look yet.

[steverep]

Ah I was wondering where he disappeared to. Okay thanks for the heads up.

[steverep]

Any chance this can get a review in time for April's beta?

[bramkragten]

I wonder if we need this change, as mwc-dialog should handle this (it does here I think? https://material-components.github.io/material-web/demos/dialog/)

I think the issue is the location in the DOM where we add the dialogs probably? They are in a different place in the DOM then the element that triggers it...

Would have to look in to that

[steverep]

There's no code in mwc-dialog to handle returning focus. What OS/browser are you using on those demos? Focus is not put back on the trigger for me in Chrome or Firefox on Windows. It's just letting the browser handle it, which varies with browser. For example, Firefox always throws focus back to the top of the document, whereas Chrome tends to throw focus further down the page.

You could make the argument that mwc-dialog should have a default to return to the last focused element, but that will only ever handle a subset of cases. For example, mwc-dialog is never going to handle cases of replacing dialogs, trigger buttons becoming hidden, dialogs changing the DOM on close, etc. That logic needs to be specific to the app.

[bramkragten]

I'm using Chrome on MacOS

[steverep]

I'm using Chrome on MacOS

I don't have a Mac to reproduce that, but I'd be shocked if webkit was actively trying to return focus. It doesn't do that on iOS.

In any case,leaving it up to the browser isn't a good or even consistent approach as I said, and it's definitely not mwc-dialog code. All the component does is use PolymerLabs/blocking-elements to set the inert and aria-hidden="true" attributes outside the dialog when it's open, and removes them when it's closed. The only code regarding focus is to set it when the dialog is open, but only if the dialogInitialFocus attribute is set.

[steverep]

PS - At least on that demo page, Chrome/blink on Windows seems to send focus to the next button rather than the one that opened the dialog, which is not the desired behavior.

[steverep]

@bramkragten so what's the review verdict here? Have I convinced you this code is needed independent of mwc-dialog in order to handle HA situations?

[bramkragten]

Yeah, I'm seriously considering it, want to make it as easy as possible to maintain... Will get back to you this week.

[steverep]

@bramkragten I don't understand the maintainability concerns. This is not particularly complicated code. Of course I'm open to review suggestions.

@zsarnett can you add your review cents here?

[steverep]

@balloob can you intercede here? I've commented in the past that HA does not prioritize accessibility and this PR lingering on without a review is an example. This implements simple focus management for dialogs - a fundamental piece of good keyboard accessibility.

[zsarnett]

Please stop pinging developers. Bram advised he will be reviewing.

This is not about limiting Accessibility so don't try to pull on that string. This is about maintainability.

Normally your PRs go through without much question. This one adds a lot of code we have to maintain. Be patient

[steverep]

Please stop pinging developers. Bram advised he will be reviewing.

That was weeks ago and much work has come and merged since. I don't see any problem with trying to call attention and start a discussion with the project's lead on prioritizing accessibility. It was not meant to offend anyone.

This is not about limiting Accessibility so don't try to pull on that string. This is about maintainability.

I didn't accuse the project of limiting accessibility. I accused the project of not prioritizing it. There's a huge difference. If there is a way to make it more maintainable, then review and I'd be happy to implement it. No one has done that.

Normally your PRs go through without much question. This one adds a lot of code we have to maintain. Be patient

Please don't pretend like this is an exorbitant amount of code. Again, many more lines have come and merged since. And, just as food for thought, do you have any idea how often people with disabilities are told to be patient when trying to resolve accessibility or accommodation issues? It's not about patience - it's just about treating those issues with the priority they deserve.

[steverep]

@bramkragten this is ready for another look. I like this version better and I think it settles all your issues.

[bramkragten]

Looks a lot better 👍