-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Block Library: Update Columns block to use Patterns API #18283
Conversation
582aaaf
to
3d385da
Compare
c659e2e
to
5744f19
Compare
5744f19
to
f6b2034
Compare
@youknowriad, I applied some of the changes which were included in #18343. In particular, I added the wrapper for
This is very close to the vision shared. I'd like to refactor one more block to identify some potential issues with the current proposal. At the moment the wrapper which handles the pattern selection is presented when there are no nested blocks added to the block. In the case of Columns block, it's perfectly fine as it always needs to have at least 2 nested blocks. However, this might not fit all use cases as some blocks might assume that there might be no blocks provided when the block gets created. In addition, it doesn't cover also blocks that don't have inner blocks where we want to use Patterns API to offer some initial setup. |
435f520
to
356703a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking very good in my testing
356703a
to
13b6cae
Compare
It looks like 13b6cae solves all the issues discussed. |
Nice, Travis likes the changes proposed. Thanks @youknowriad for the great tip 🥇 The only question remaining is whether we should remove |
Using Icon in the IconButton can cause some breakage. Notably I see that DropIt icon is broken in the more menu, that said I'm fine if we can consider this a plugin issue but we should check quickly how many issues this could introduce. |
I see why it can happen, it's the Inception issue 😄 Can we add the case in the |
It looks like the issue is different, there is function Icon({ size = "small", borderless = false, color = "unsplash" }) { |
yeah! I'm fine for this being a Dropit issue :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
13b6cae
to
e522171
Compare
Description
Part 2 of #16283.
Fixes #18738.
Continuation of #18270. This branch is based on the
add/patterns-api
master
branch.This PR refactors Columns block to use this new API making the placeholder open for customization.
Selector
This PR also introduces a new selector which lets to find the default pattern:
To ensure that icons have a shared size in the layout selection, this PR also adds a new prop to the
IconButton
component:size
to make it possible to enforce one value for icons passed in various formats.Components
__experimental BlockPatternPicker
I refactored an internal component
InnerBlocksTemplatePicker
from theInnerBlocks
implementation. It's now calledBlockPatternPicker
and is exposed as an experimental feature__experimental BlockPatternPicker
from@wordpress/block-editor
package.In addition, I slightly tweaked wordings in the UI to use the
pattern
keyword. It's also reflected in the name of props.InnerBlocksTemplatePicker
is no longer included inInnerBlocks
as discussed in this PR. I updated all the corresponding documentation. This API was experimental so we don't need any further actions.IconButton
I added a new
size
prop toIconButton
component to make it possible to scale all icons to the same size in the block pattern picker.How has this been tested?
npm run test-unit
– new unit tests were added.Columns block UI
Extensibility
To add a new pattern to the Columns block:
To remove a pattern from the Columns block:
When those two commands get executed, then the custom pattern becomes the default one. In effect, when you add a Column block and click the skip link, then 4 columns will be inserted.
IconButton
You can test in Storybook the
IconButton
and its newsize
prop. To do it runnpm run design-system:dev
and navigate to the proper page. You can use knobs to play with sizes:Checklist: