Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V8: Add auto-focus to the default action in overlay dialogs #5463

Merged

Conversation

kjac
Copy link
Contributor

@kjac kjac commented May 14, 2019

Prerequisites

  • I have added steps to test this contribution in the description below

Description

This one may be a bit controversial... but here goes 😄

There are a few issues with using keyboard input (tabbing and enter) around in overlay dialogs:

  1. The "unsaved changes" dialog completely breaks the backoffice if you hit enter in it 💥
    unsaved-changes-enter-errors
  2. If you hit enter in the save/publish/... dialogs for variant content, the default action is always fired when you hit enter, no matter if you have tabbed your way to the other actions.
    tab-publish-before
  3. Depending on the action that opens a dialog, you can't immediately tab your way to the buttons in the dialog, as the active HTML element is actually still somewhere underneath the overlay (!). This is very evident in the "unsaved changes" dialog, but also the save dialog for variant content suffers from this (first tab will activate the save and publish button underneath the overlay).
    tab-order-before

This PR introduces an option to set auto-focus on umb-button, and utilizes said option on the default action in the overlay dialogs. This means that the default action always has immediate focus, and thus will be invoked when you hit enter (unless you tab away).

The controversial bit of this PR is actually two fold:

  1. The default action button is highlighted (because it has focus). Personally I think it makes sense; now you can actually decode what's going to happen if you hit enter (even if the highlight color is a bit vague... fixing that is part of the accessibility initiative if I'm not mistaken).
    image

  2. Since the default action has focus, you need to shift+tab to access the other actions (as you need to go backwards in tab order). The upside is that you actually can use the other actions now.
    tab-publish-after

@MMasey
Copy link
Contributor

MMasey commented May 16, 2019

This is awesome dude! I'm literally in awe at how much you are getting done at the moment. I haven't had a chance to pull this down, but is there a keyboard trap for the modal? I think It's one of the things that @BatJan has been working on creating a directive for I think #4525. It's one of the recommended approaches to handling tabbing in modals and also crops a few times in the list of things to fix in #5277. It may be more suited to a separate PR relating to Jan's stuff, but figured i'd ask the question just in case.

@kjac
Copy link
Contributor Author

kjac commented May 16, 2019

@MMasey no keyboard trap just yet. I wanted to do one but I didn't have the time to look into it.... the auto-focus is a first (necessary) step on the way, though. And I'm all about incremental changes anyway 😆

@BatJan
Copy link
Contributor

BatJan commented May 16, 2019

@kjac Really nice work 👍- Regarding the keyboard track I suggest you wait until I have gotten some feedback on #4525 since it should be possible to reuse the concept for overlays once if/when it makes it into core 😃

@nul800sebastiaan nul800sebastiaan merged commit 040ebfe into umbraco:v8/dev Jun 27, 2019
@nul800sebastiaan
Copy link
Member

I think all of this makes perfect sense and works nicely, thanks very much @kjac !! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants