Use onClick to make Site Editor Open Admin Sidebar Button keyboard accessible #50067
Conversation
On the site editor, when the Admin Sidebar is open, the button label and tooltip says "Open Admin Sidebar," but it is already open, so the button doesn't do anything when clicked again which makes it seem broken. I switched it into a toggle to open/close the Admin Sidebar as a starting point to at least fix the accessbility of it, as a non-functional button feels broken. I considered a couple other options but they had their own issues: 1. When the sidebar is open, have the button go back to the dashboard. This action would mean switching it from a `<button>` when the dashboard is closed, and then changing it to an `<a>` element afterwards to semantically describe the functionality. Switching the element's action like that feels confusing. 2. Disable focus on the button when the panel is open. When the panel is closed, clicking the Open Admin Sidebar button could open the sidebar and then place focus on the first item of the navigation menu. This could be a fine solution, but would require more work, so I didn't want to go through with that interaction unless others agreed it was desired. Switching to a toggle was the simplest way to address it in a sensible way.
|
Follow-up: |
|
Size Change: +11 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 6e809d6. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4798993907
|
|
Thanks, @jeryj! If this is a toggle, then the accessible name shouldn't include the word 'Open'; it should describe the object that is being activated; the word 'Toggle' would be better. The state of the toggle should be communicated using the button's state, via aria-expanded. Removing the word sidebar here is only viable if it can be replaced with something else more meaningful - just 'Admin' would just not be accurate. I'm not 100% certain what control we're looking at here; if I try to follow the instructions using the Gutenberg plugin, I'm seeing that the control says 'Open Navigation Sidebar' - is this changed recently, or am I looking at the wrong control? |
|
Thanks for opening this PR @jeryj. I'm a bit surprised #48886 went in, as it started with the intent of polishing the CSS animation and ended up with introducing functional changes that break accessibility and keyboard interaction. I'm not sure that's the best way to keep the quality of this project high. That said, a few notes:
|
|
Thanks for the follow up @jeryj. Let's fix the event handler oversight first and foremost #50094. For the broader navigation consideration, the situation before with swapping the element tag was clunky and had subtle bugs, so best to avoid. The issue here is the longer term plan is for the button to allow going up in levels quickly (like a "home" button) but that only works if the flow is controlled. When it functions as a link it's not very clear and the effect is disruptive (it's not easy to get back). So let's avoid the dashboard link. We could add the toggle behavior (I believe @richtabor also suggested that) but under the caveat that it'd be a temporary measure. I worry int hat case we'll train some muscle memory only to be changed at a later phase. I think it's safer from that perspective to keep it inert, but open to other suggestions. |
Agreed, I'm not thinking it should toggle any longer. Though we do need a method for entering the canvas (edit view) from via keyboard. |
To me, keeping a button that doesn't do anything is arguably a good UI. If this button was changed in preparation for a future feature (the 'go back' functionality) but that feature is not ready yet, I'd argue the change shouldn't have been merged in the first place. In its current state, the UI is not functional. #48886 should have been limited to its original scope: the CSS animation. Instead, it ended up introducing functional and breaking changes. One more thing that likely broke in #48886 is the focus style of the 'navigate regions': when navigating the main UI sections with Control + backtick (or Control + Option + N), a blue outline highlights the focused region. The sit eicon button now partially hides the outline, I guess because its wrapper has a dark background. Screenshot:
|
|
No, that visual bug is unrelated to that change. |
This sounds interesting, but potentially confusing - are you saying that the long term goal is to use this single button to perform different actions depending on context? In my view, that's already part of the problem with this control - that it doesn't perform a consistent action, so it's very difficult to predict. Will this long-term goal include any kind of breadcrumb-like affordance that would help users understand how it performs? |
|
@joedolson yes, the intent is to see if we can establish that control as a fixed anchor point that allows users to exit any full screen state (like the editor) and then also navigate back to the root of the dashboard quickly, very similar to a home button (exit app, press again to go to your Home Screen root). There's a small section on the home button here for more context. It's meant to also work alongside plugin specific sections and sub-sections, with a breadcrumb as part of the design next to the section title. This may or may not work in practice; as you mentioned, elements that perform contextual actions can be tricky, but also quite dependable if established well. Prototypes seem to suggest there's potential in there, but it's hard to fully grasp until more parts of the experience are developed. |
|
For now I see three things that need resolved for interacting with the Site Editor with a keyboard to make it functional for now: 1. Be able to enter the Site EditorAs @richtabor mentioned, we can't use a keyboard to enter the Site Editor. Would an acceptable flow for now be:
If this is OK, I can work on a PR for this this to see how it feels. 2. Rename the label on the control that opens the navigationI wouldn't say it should be the long-term one, but reverting to "Open Navigation" seems clearer for now until the sidebar is more of an admin panel. 3. Do something with the control when the navigation is open.When the Navigation is opened, we could disable the "Open Navigation," control and move focus to the "Back to Dashboard" button. I know these may not be perfect solutions, but they would be a point to iterate from. |
1.Indeed, I think being able to focus on the canvas frame would be great for the top level routes. Other sections, like "styles" or individual templates like "archive" have a dedicated edit button. Being able to jump into the editor at root levels is more of a shortcut to editing the homepage, essentially. 2."Open Navigation" can be confusing since there's a section called "Navigation" for managing nav menus already. But it'd be nice to have a straightforward name. |
|
Perhaps "editor navigation", to differentiate it from the navigation management tools. It might be good to name both tools unambiguously: "Site navigation" vs "Editor navigation". |
I'm not sure what the future of that Editor Sidebar panel is, but I do agree that in it's current state it does function as a Navigation. So, using the term "Navigation" in the button label most accurately describes its function. I'd be happy with "Open Editor Navigation," then once more things are added/that panel is changed to be more than a navigation, we can rename it to accurately describe its function. |
|
Definitely missing something obvious here but:
This way:
|
|
@draganescu that's what we had before — it's clunky because the tag change causes repaints and the flow breaks. |
|
@mtias does changing the role account to being a tag change? I am proposing it stays a button. |
|
Ah, I thought you meant at the component level |
|
Ooooh - nice idea @draganescu! The I've tested it out in #50296, and it does preserve the WordPress W animation effect. |
priethor
added
[Focus] Accessibility (a11y)
What?
The Open Admin Sidebar Button only works with a mouse click since it uses
onMouseDownto open the Admin Sidebar. This PR switches fromonMouseDowntoonClick. When the sidebar is in the open state, the button label says, "Open Admin Sidebar," but the sidebar is already open.To get the conversation started, I switched it to a toggle as the classNames within the file call it a
toggle. However, other options are possible:Alternative Proposals:
When the sidebar is open, have the button go back to the dashboard. This action would mean after clicking the Open Admin Dashboard
<button>, changing it to an<a>element to semantically describe the functionality and give it the properhref. Switching the element's action like that feels a little confusing from a screen reader perspective.Disable the button when the panel is open as it has no action. and send focus to the Back to Dashboard Link. This could be a fine solution, but would require more work as the Back to Dashboard link isn't within the same component, and would require more complex work to figure out the focus management code. I didn't want to go through with that interaction unless others agreed it was the right way to go.
Why?
The Site Editor Open Admin Sidebar button is not keyboard accessible.
How?
Switches the button from using
onMouseDowntoonClickto make it keyboard accessible, and switching the behavior of the button to a toggle.Testing Instructions
/wp-admin/site-editor.phpTesting Instructions for Keyboard
/wp-admin/site-editor.phpEnterto close the sidebar.Enterto close the sidebarScreenshots or screencast