Post editor: fix meta boxes accessibility #65466
Conversation
stokesman
added
[Type] Bug
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +630 B (+0.04%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
noisysocks
added
the
Backport to WP 6.7 Beta/RC
|
Thanks for the PR! I'd like to test this PR in detail later, but one thing I noticed is that when the resize handle is focused, pressing the up and down keys may lose focus. I checked out #65325 (07c6a1f), which implemented a custom resize handle, and tested this problem, and I was able to reproduce the problem in my environment. And, although the behavior is slightly different, this problem also occurs in this PR. We might have to investigate and fix this issue first. Below is a video that reproduces the problem in trunk: 688adb4e968556f21270dfa730aa77e0.mp4By the way, I submitted a PR to the Even if that PR is merged, backporting it to WP 6.7 may have a big impact. Therefore, we might consider shipping this PR at the time of WP 6.7 and updating the library in WP 6.8. |
It looks like this problem can be solved just by adding gutenberg/packages/edit-post/src/components/layout/index.js Lines 280 to 281 in e691b04 Update:
It turns out that this issue is specific to the Windows OS and Chrome browser. When text cursor mode is enabled, pressing the up and down keys takes focus away from the drag handle. In any case, it seems that |
|
bokuweb/re-resizable#827 has been merged and released as version 6.10.0. Another way to solve this issue is to update |
@t-hamano I see you're already working on it in this branch, but if you need any help, let us know. |
I was able to reproduce on Mac OS with Firefox or Chrome as they both support caret browsing. I added the |
|
I think we have two paths to take. Which one is better?
|
| className: clsx( className, 'is-toggle-only', isOpen && 'is-open' ), | ||
| }; | ||
| paneButtonProps = { | ||
| 'aria-label': __( 'Expand or collapse meta boxes pane' ), |
There was a problem hiding this comment.
This button should have a managed aria-expanded attribute, which allows the text to be shortened to just 'Meta boxes pane'. The behavior will be conveyed by the value of the aria-expanded attribute, and the extra terms are unnecessary.
There was a problem hiding this comment.
Thanks for reviewing! b7f087e adds aria-expanded but since the button itself already had "Meta boxes" as its text I just removed aria-label. Does that seem good?
There was a problem hiding this comment.
Regarding the tab order issue, I thought it might be solved by updating the re-resizable library (#65637). However, some behaviors have changed in the latest version, so I couldn't simply update it. Considering the impact on developers, it might be better to proceed with this PR after all.
After testing this PR, I noticed two things so far:
- In Windows high contrast mode, I can't see the resize handles and the chevron icon:
The following style should help:
Diff
diff --git a/packages/edit-post/src/components/layout/style.scss b/packages/edit-post/src/components/layout/style.scss
index 4e2e72b721..3d4d9af30f 100644
--- a/packages/edit-post/src/components/layout/style.scss
+++ b/packages/edit-post/src/components/layout/style.scss
@@ -5,6 +5,8 @@
display: flex;
flex-direction: column;
overflow: hidden;
+ // Windows High Contrast mode will show this outline, but not the box-shadow.
+ outline: 2px solid transparent;
}
.edit-post-meta-boxes-main__presenter {
@@ -35,6 +37,10 @@
inset: var(--wp-admin-border-width-focus);
@include button-style__focus();
}
+
+ svg {
+ fill: currentColor;
+ }
}
.is-resizable > & {
@@ -60,6 +66,8 @@
border-radius: $radius-small;
transition: width 0.3s ease-out;
@include reduce-motion("transition");
+ // Windows High Contrast mode will show this outline, but not the box-shadow.
+ outline: 2px solid transparent;
}
}- I can't resize it on touch devices. My guess is that this is because
useDraggingonly targets mouse events. I haven't found a good solution to this yet...
| } | ||
|
|
||
| return ( | ||
| <aside aria-label={ paneLabel } { ...paneProps }> |
There was a problem hiding this comment.
Is the aside element appropriate here? I'd like to hear from people with more knowledge.
|
I'm not sure if this is possible, but I wanted to share one idea I had.
|
|
Flaky tests detected in 753edcf. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11045799667
|
|
@afercia thanks for weighing in. I’d wanted to try a labeled section for the meta boxes for a couple of reasons. @joedolson’s #65406 (comment) got me thinking that maybe it improves a11y for the section itself to be locatable so I wanted to apply a label. Using That same comment also had me wondering if perhaps with a labeled landmark as the container then it would actually be okay to accessibly hide the meta boxes. That would remove my current concern that the container can be collapsed but there’s still a bunch of tab stops within. It seems like we have to uncollapse the section in case focus enters it. I don’t think that’s too challenging but hadn’t done it yet in case the simpler option could be considered. |
Here's what I tried: Navigale regions in action:
Edit: |
|
@afercia, thanks for clarifying! I like the idea of using
If so, I would guess it’s because |
I don't think that applies in this case. A region shouldn't be conditionally rendered on the fly, but whether or not to show metaboxes is a persistent conditional. Once set, the region is always present. This is different from examples like the Document overview, which is only present when toggled, and persistent across different editor views. |
Thanks for elaborating. I thought the same and you explained the reasoning more clearly. I’ve pushed a couple commits. The first one to create the navigable region and the second to (fully) hide the contents if collapsed. I did so to force the question of whether with a labeled navigable region if that makes it acceptable to hide its contents when not expanded. If not, then don’t we need to add logic to expand the area when focus enters its contents so to avoid invisible tab stops? |
|
Test results using NVDA: ba32c898568419fa43ad6e92b072164c.mp4 |
|
I think we're ready to ship this PR, but what do you think? I'd be grateful for additional accessibility feedback. |
I see
An entirely empty navigable region (which is an ARIA landmark) wouldn't be ideal. This is a problem for other regions in teh editor. However, to my understanding, the
|
Yes, that makes sure resizing added by
When collapsed it contains only the button to expand its contents. It is like the Publish region except it’s visible even when not focused. |
github-actions
bot
removed
the
Backport to WP 6.7 Beta/RC
* Replace details element with custom toggle * Consolidate details of updating height into function * Reduce conditionals for toggle vs. resizable * Fix style in RTL * Remove unneeded overflow style * Maintain focus even with caret browsing on * Use `ResizableBox` again * Add aria-expanded to toggle and remove aria-label * Fix styles for Windows high contrast mode * Add missing "border" in Windows high contrast mode * Move `flex-shrink` rule to toggle only * Fix rule preserved for non-iframe canvas * Make meta boxes container a navigable region * Try hiding meta box region contents when not expanded * Fix hidden conditional * Rename a couple functions Co-authored-by: DaniGuardiola <daniguardiola@git.wordpress.org> Co-authored-by: joedolson <joedolson@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: afercia <afercia@git.wordpress.org>
github-actions
bot
added
the
Backported to WP Core
|
I just cherry-picked this PR to the wp/6.7 branch to get it included in the next release: 93af574 |
* Replace details element with custom toggle * Consolidate details of updating height into function * Reduce conditionals for toggle vs. resizable * Fix style in RTL * Remove unneeded overflow style * Maintain focus even with caret browsing on * Use `ResizableBox` again * Add aria-expanded to toggle and remove aria-label * Fix styles for Windows high contrast mode * Add missing "border" in Windows high contrast mode * Move `flex-shrink` rule to toggle only * Fix rule preserved for non-iframe canvas * Make meta boxes container a navigable region * Try hiding meta box region contents when not expanded * Fix hidden conditional * Rename a couple functions Co-authored-by: DaniGuardiola <daniguardiola@git.wordpress.org> Co-authored-by: joedolson <joedolson@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: afercia <afercia@git.wordpress.org>
What?
A fix for meta box headings being absent from accessibility tree when their container is collapsed.
Why?
Meta boxes are expected to be accessible even if their container is collapsed.
To fix #65406.
How?
detailselement with a custom expandable (usingaria-expanded).Testing Instructions for Keyboard
Have a plugin activated that creates meta boxes or have the custom fields preference on and open a post or page in the Post editor.
Toggle (short viewport)