Post Editor: Support keyboard resizing of meta boxes pane #65325
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also computes maximum resize height taking into account notices
stokesman
added
[Type] Task
|
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: +1.38 kB (+0.08%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
t-hamano
reviewed
Sep 14, 2024
There was a problem hiding this comment.
Thanks for the PR! It's great that editor notices are taken into account.
So far, I've noticed two things:
aria-valuenowdoesn't seem to update properly in my environment:
523dbb1ce705ab0a6626304830051575.mp4
- I'm thinking about avoiding the use of
ref.currentas much as possible and making use of React state andResizableBoxprops. The following is a very simplified example, but at least the height of theResizableBoxcomponent and the value ofaria-valuenowmight be managed without referencing ref. Then, we might be able to deleteresizableBoxRefandseparatorRef.
Code example
function MetaBoxesMain( { isLegacy } ) {
const [ boxHeight, setBoxHeight ] = useState( openHeight );
const [ ariaValueNow, setAriaValueNow ] = useState( openHeight );
return (
<ResizableBox
defaultSize={ { height: boxHeight, width: 'auto' } }
size={ { height: boxHeight, width: 'auto' } }
onResize={ ( event, direction, elementRef, delta ) => {
setBoxHeight( elementRef.offsetHeight );
setAriaValueNow( getAriaValueNow( elementRef.offsetHeight, max ) );
} }
onResizeStop={ () => {
setBoxHeight( boxHeight );
setPreference(
'core/edit-post',
'metaBoxesMainOpenHeight',
boxHeight
);
} }
>
{ contents }
</ResizableBox>
);
}I haven't yet figured out all the logic of the code, so please let me know if I'm misunderstanding anything 🙏
Mine too 😱. I thought I had that working right at some point. I’ll get back on it.
It might simplify some things and could be worth trying to get a clearer picture of the tradeoffs. I was thinking to avoid dragging operations from re-rendering |
|
Okay, |
|
Flaky tests detected in 9fd8e14. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10866254493
|
t-hamano
reviewed
Sep 16, 2024
There was a problem hiding this comment.
Thanks for the update! I left a small suggestion at the end, but everything seems to work as expected.
- ✅ The
aria-valuenowattribute shows the correct percentage value - ✅ Screen readers announce drag handle operations properly.
- ✅ The resizable box's max height and
aria-valuenowattribute value are updated immediately when the browser height changes or notification components are added - ✅ Tested on iPhone. The resize handle is operable by touch.
whether it’s important to update during the drag anyway. Maybe it would suffice to only update it when the drag ends.
I don't have a strong opinion on this, but it seems that the site editor's resizable canvas updates the aria-valuenow attribute while dragging, so it might be fine the way it is now.
|
I’ve added 9559a2d that increases the resize handle height by one pixel. This is to avoid the separator button from having decimal top/bottom margins which can create minute off-centering. The old height was just taken from from what the resize handles used on blocks in the canvas are but they aren’t visibly that size anyway. Nobody’s complained about the rather chunky resize handle so far so probably one more pixel won’t push it over the edge. |
|
The main thing I don’t like about this is that the tab order doesn’t seem right. From the canvas, one has to tab through the entire meta box area before reaching the resize handle. meta-box-resize-tab-order.mp4It’s a limitation of re-resizable (the library our Anyway, I don’t think the tab order should block this because it’s considerable effort to fix and this at least adds the missing keyboard support. |
Yes, I was also concerned about this issue, and when I worked on #57475, I was not able to ideally solve the problem. It would be great if we could solve this issue without updating the library itself 👍 |
Yes, getting a patch merged upstream would be the proper solution. Switching to another library (or rewriting without one) is an alternative but it’d be complicated by preserving It happens that this use case is relatively simple so just not using |
This was referenced Sep 20, 2024
97 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
A follow up to #64351 adding keyboard support to resize the meta box pane.
Why?
a11y
How?
Implements a custom resize handle with
role="separator",aria-valuenow,aria-labelandaria-describedbyattributes.Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
keyboard-resize-meta-box.mp4