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.

Sign up for GitHub

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

Jump to bottom

Dropdowns should close on Esc press #11568

Closed
Reinmar opened this issue Apr 5, 2022 · 5 comments · Fixed by #12003
Closed

Dropdowns should close on Esc press #11568

Reinmar opened this issue Apr 5, 2022 · 5 comments · Fixed by #12003
Labels
domain:accessibility This issue reports an accessibility problem. domain:ui/ux This issue reports a problem related to UI or UX. squad:features Issue to be handled by the Features team. type:improvement This issue reports a possible enhancement of an existing feature.
Milestone
iteration 55 (v35...

Comments

@Reinmar
Copy link
Member

Reinmar commented Apr 5, 2022

Scenario:

  1. Open a drop-down (e.g. headings)
  2. Press Esc

Expected: the dropdown is closed. The focus moved back to the editing.

Actual: nothing happened.

If you'd like to see this improvement implemented, add a 👍 reaction to this post.
@Reinmar Reinmar added type:improvement This issue reports a possible enhancement of an existing feature. domain:accessibility This issue reports an accessibility problem. domain:ui/ux This issue reports a problem related to UI or UX. squad:core Issue to be handled by the Core team. labels Apr 5, 2022
@Reinmar
Copy link
Member Author

Reinmar commented Apr 5, 2022

Media embed's panel closes. All other stay open.

@oleq
Copy link
Member

oleq commented Apr 6, 2022

It never worked for headings if opened using mouse, I checked builds as old as v12.0.0. 

It only works when you're navigating using the keyboard because then the dropdown element (panel, items, whatever) is focused, and  the heading's keystroke handler listens to events from the dropdown element.

Media embed's panel closes. All other stay open.

Because there's an input in the panel that focuses as soon as you open the dropdown. Focused input == keystroke handler embedded in the media feature is able to listen to events from the media dropdown.

To address this issue, we'd need to:

  • either use editor's global keystroke handler, 
    • which would break encapsulation of UI views: heading dropdown view has no access to the global keystroke handler of the editor instance
  • or figure out another global system for keystrokes. It would need to aggregate different features, though:
    • to avoid 50 listeners to the same DOM event,
    • to avoid collisions, 
      • what if nested components should respond to Esc? For instance, a dropdown in a balloon: dropdown should close on first Esc, the balloon on the next. 
        • The current implementation handles this pretty well because each component has its own keystroke handler which works according to the current focus (no collision).
  • (probably the best idea) or we could start focusing dropdowns (also, buttons, etc.) when clicked. Embedded keystroke handlers will begin to work and do their job. a11y will also improve. Example: bootstrap dropdowns.

@Reinmar
Copy link
Member Author

Reinmar commented Apr 6, 2022

  • (probably the best idea) or we could start focusing dropdowns (also, buttons, etc.) when clicked. Embedded keystroke handlers will begin to work and do their job. a11y will also improve. Example: bootstrap dropdowns.

My thoughts too 👍

@wimleers wimleers mentioned this issue May 31, 2022
Closed
@mlewand
Copy link
Contributor

mlewand commented Jun 9, 2022

So this issue should be fixed by #11838 let's keep it open to make sure that indeed it works for all dropdowns as expected.

@mlewand mlewand added squad:features Issue to be handled by the Features team. and removed squad:core Issue to be handled by the Core team. labels Jun 9, 2022
@Comandeer
Copy link
Member

Expected: the dropdown is closed. The focus moved back to the editing.

Actually, the focus should probably move back to the element that triggered the dropdown (ARIA Practices recommends that). Moving back to editing would be really disturbing to the keyboard-only user, e.g. they opened some dropdown by accident and they're forced to refocus the toolbar and move through all buttons once more just to find the right dropdown.

@mlewand mlewand mentioned this issue Jul 1, 2022
Merged
This was referenced Jul 4, 2022
Open
Closed
oleq added a commit that referenced this issue Jul 13, 2022
@oleq
Merge pull request #12003 from ckeditor/ck/11838-placement-of-focus-a…
a95ad5c 
…fter-opening-dropdowns

Fix (ui): Opening a dropdown should focus the first item (or the first active item) for easier navigation and accessibility. Closes #11838. Closes #2000. Closes #11568. Closes #3215. Closes #5859.

MINOR BREAKING CHANGE (ui): Until now, `preventDefault()` was called on the `mousedown` event to prevent the [`ButtonView`](https://ckeditor.com/docs/ckeditor5/latest/api/module_ui_button_buttonview-ButtonView.html) from "stealing" the focus from the editing root. Starting from this editor version, to improve the accessibility of the editor, all instances of the [`ButtonView`](https://ckeditor.com/docs/ckeditor5/latest/api/module_ui_button_buttonview-ButtonView.html) class (and child classes) will automatically focus in DOM when clicked. It is recommended that features that use buttons to manage the content call `editor.editing.view.focus()` after the button was [executed](https://ckeditor.com/docs/ckeditor5/latest/api/module_ui_button_buttonview-ButtonView.html#event-execute) in order to bring the DOM focus back to the editing root to allow users type right away. 
@Reinmar Reinmar added this to the iteration 55 milestone Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
domain:accessibility This issue reports an accessibility problem. domain:ui/ux This issue reports a problem related to UI or UX. squad:features Issue to be handled by the Features team. type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Milestone
iteration 55 (v35.0.0)
Development

Successfully merging a pull request may close this issue.

Implemented focusing first item after opening a dropdown ckeditor/ckeditor5
4 participants
@Reinmar @Comandeer @oleq @mlewand