Migrating BlockPatternSetup to use updated Composite implementation#55425
Merged
Conversation
- Removes `__unstableComposite` imports from `@wordpress/components` - Adds private `Composite*` exports from `@wordpress/components` - Refactors `BlockPatternSetup` and subcomponents to use updated `Composite` components - Additionally rewords UX to improve keyboard/AT experience
andrewhayward
added
[Type] Task
andrewhayward
requested review from
brookewp,
chad1008,
ciampo,
jorgefilipecosta,
noisysocks and
ntsekouras
andrewhayward
commented
Oct 17, 2023
andrewhayward
commented
Oct 17, 2023
andrewhayward
commented
Oct 17, 2023
Comment on lines
+268
to
+270
| setActiveSlide( ( active ) => | ||
| Math.max( active - 1, 0 ) | ||
| ); |
Contributor
Author
There was a problem hiding this comment.
Now that previous/next buttons are no longer disabled (see CarouselNavigation), this is necessary to keep the active slide within bounds.
andrewhayward
commented
Oct 17, 2023
Comment on lines
+263
to
+265
| setActiveSlide( ( active ) => | ||
| Math.min( active + 1, patterns.length - 1 ) | ||
| ); |
Contributor
Author
There was a problem hiding this comment.
Now that previous/next buttons are no longer disabled (see CarouselNavigation), this is necessary to keep the active slide within bounds.
andrewhayward
commented
Oct 17, 2023
| justify-content: space-between; | ||
| border-top: 1px solid $gray-300; | ||
| align-self: flex-end; | ||
| align-self: stretch; |
Contributor
Author
There was a problem hiding this comment.
This change seems to make the toolbar more stable, preventing it from jumping around as slide height varies.
6 tasks
|
Size Change: +133 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
chad1008
reviewed
Oct 19, 2023
Contributor
chad1008
left a comment
chad1008
left a comment
There was a problem hiding this comment.
👍 This looks good to me, but I'd like to leave the official approval to someone more familiar with block-editor and Composite
Contributor
|
Thanks for the PR @andrewhayward! I think it becomes really obvious here, where we also make more changes besides the |
Contributor
Author
|
I've reverted a lot of the original changes, to reduce the scope of the PR somewhat. So it's now much more focused on swapping out the old There are still a few additional updates, but nothing functional beyond allowing the carousel navigation buttons to retain focus when disabled. It is otherwise just minor semantic alterations. |
Contributor
Author
It's a good point @ntsekouras, and one I've been thinking about quite a lot recently. Would be great to see more documentation in general for contributors to Gutenberg, around where and how different parts of the codebase are used, etc. |
Contributor
Absolutely. Let's focus on finishing the migration to the new ariakit components first, and then provide some good storybook docs and examples for this new version |
ciampo
approved these changes
Nov 18, 2023
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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?
This PR updates
BlockPatternSetupin@wordpress/block-editor(and related sub-components) to use the updatedCompositeimplementation from #54225.Additionally, it reworks some of the UX to improve the keyboard/AT experience.
Why?
In #54225, an updated implementation of
Compositewas added to@wordpress/components. As per #55224, all consumers ofCompositeneed to migrate from the old version to the new version.How?
__unstableCompositeimports from@wordpress/componentsComposite*exports from@wordpress/componentsBlockPatternSetupand subcomponents to use updatedCompositecomponentsTesting Instructions
Unfortunately
BlockPatternSetupis no longer used in Gutenberg (see #52374), but because it is used by various plugins (despite being marked as 'experimental'!), we need to keep it around.With the JetPack plugin installed, add a JetPack forms block to a page/post, and open the "Form Patterns" dialog.
The (default) grid view should present as a single tab stop, with arrow keys moving up and down through the various options, as before. (This somewhat confusingly doesn't behave as a grid, but that's a problem outside of the scope of this PR!)
The carousel view should also behave as before, with the arrow buttons at the bottom navigating through the different patterns.