Group block: remove innerprops from placeholder wrapper #49783
Merged
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
|
Size Change: 0 B Total Size: 1.37 MB ℹ️ View Unchanged
|
|
Flaky tests detected in 33823d2. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4684190021
|
andrewserong
approved these changes
Apr 13, 2023
There was a problem hiding this comment.
Nice one, this appears to do the trick!
The earlier fix just needed to output innerBlocksProps.children to ensure that the InnerBlocks component is rendered, which then updates the blockListSettings. There wasn't anything else we needed to add to the View wrapper component, so this looks like a safe change to me, and in testing, restores the correct focus behaviour as before 👍
Thanks for getting a fix up quickly!
andrewserong
added
the
[Type] Bug
andrewserong
changed the title
|
I appreciate the quick test @andrewserong 🙇 I'll beef up the description, do a sanity-check test and YOLO. |
That far exceeded my expectations! 😄 |
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?
Possibly, and to some extent, even most probably resolves #49758
To be sung to the tune of Captain Underpants, by Weird Al
When inserting Group block variants
You shouldn't need to take aperients
The block placeholder should have focus from the start, the start, the staaart
And now the icons can be tabbed right through
Find the arrow keys and smash! that! keyboard!
Check the tooltips as you move, they describe each variant.... Tra la la!
Fixing Group - the block's no longer fried!
Fixing Group - drag a paragraph inside!
Na, na, na, na, na
Group block variants, yeah yeah yeah.
2023-04-13.10.22.55.mp4
Why?
Many of the props were duplicated on the placeholder wrapper and keyboard navigation wasn't working as well as it could have. That's a euphemism.
How?
Removing
{ ...innerBlocksProps }from the<View />wrapper component. Yes! That's it.Testing Instructions / Testing Instructions for Keyboard
In the source, check that there is only one block wrapper that contains the Group block props and other attributes, e.g.,
aria-label="Block: Group"Also, for the Group block placeholder, make sure you can still drag child blocks via the list view. See issue: #49256
2023-04-13.11.57.59.mp4