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

Popovers with form controls for settings should use Cancel and Save buttons #63310

Open
afercia opened this issue Jul 9, 2024 · 2 comments
Open
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Editor /packages/editor [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Jul 9, 2024

Description

In the new Post summary panel, some settings previously placed within collapsible panels have been moved to Popovers.
This made more evident some pre-existing accessibility issues with this pattern, especially in terms of keyboard interaction.

There are some long established conventions when it comes to keyboard interaction and UI dialogs / popovers. To me, the two most important ones in this case are. As a user:

  • I'd expect that pressing the Escape key reverts any change I made and cancels the current operation.
  • Similarly, I'd expect that activating the X close button in a dialog or popover reverts any change I made and cancels the current operation.

Instead, when usign the keyboard in most of these popovers the only way to close the popover is to press the Escape key or the X close button but that doesn't reverts or cancel. The changed values are already saved on the onChange event. This isn;t what I would expect.

I'd like to collect some feedback from more accessibility specialists cc @joedolson. In my opinion, in this kind of popovers saving should always performed after an explicit user action. This is different from an input or other controls placed 'inline' in the UI, where it may make sense to save on change. The expectation in a popover or dialog is to explicitly Cancel or Save and I'd think the only way to improve the user experience and interaction is by adding Cancel and Save buttons.

A few screenshots of this design pattern in some of the editor popovers:

  • Post link (also known as 'permalink')
  • Post Excerpt
  • Author
  • Discussion

trunk

  • Status and visibility
  • Post publish date and time

status and visibility

Step-by-step reproduction instructions

  • Use the keyboard and interact with one of these popovers.
  • Observe the only ways to close these popovers are:
    • By pressing the Escape key
    • By moving focus to the X close button and pressing it
  • Expected behavior in both cases: any change to be discarded.
  • Actual behaviors: changes are saved

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@afercia afercia added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Editor /packages/editor labels Jul 9, 2024
@afercia
Copy link
Contributor Author

afercia commented Jul 9, 2024

Note: the Excerpt popover is a special case. When testing, you will notice that changes to the excerpt are saved only after tabbing away from the textarea (onblur). This is inconsistent with other popovers and is even more disorienting. When pressing Escape after a change and with the keyboard focus still in the textarea, the changes are not saved.

Noting that the Excerpt pattern is used also for editing other properties e.g. the Template description. Screenshot:

Screenshot 2024-07-12 at 10 55 51

@roygbyte
Copy link
Contributor

Would these dialogs benefit from a "revert" action?

  • Clicking revert would reset the contents of popover to its previous state (i.e.: what it contained before being revealed by the user).
  • Clicking X would persist the displayed state of the popover (so, just like it does now)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Editor /packages/editor [Type] Bug An existing feature does not function as intended
2 participants