Try stabilising layout and its associated APIs #51434
Conversation
tellthemachines
requested review from
spacedmonkey,
ellatrix,
geriux,
ajitbohra,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
tellthemachines
requested review from
youknowriad,
andrewserong,
ntsekouras and
ramonjd
lib/block-supports/layout.php
Outdated
| @@ -402,7 +402,7 @@ function gutenberg_render_layout_support_flag( $block_content, $block ) { | |||
|
|
|||
| $global_settings = gutenberg_get_global_settings(); | |||
| $global_layout_settings = _wp_array_get( $global_settings, array( 'layout' ), null ); | |||
| $used_layout = isset( $block['attrs']['layout'] ) ? $block['attrs']['layout'] : _wp_array_get( $block_type->supports, array( '__experimentalLayout', 'default' ), array() ); | |||
| $used_layout = isset( $block['attrs']['layout'] ) ? $block['attrs']['layout'] : _wp_array_get( $block_type->supports, array( 'layout', 'default' ), array() ); | |||
There was a problem hiding this comment.
The above two changes fall back to __experimentalLayout if layout isn't available. Should we do that for this one, too?
There was a problem hiding this comment.
I didn't change it because it seems to be accessing the correct value anyway, although yeah it might be best to be on the safe side.
| @@ -1263,7 +1263,7 @@ protected function get_layout_styles( $block_metadata ) { | |||
|
|
|||
| if ( isset( $block_metadata['name'] ) ) { | |||
| $block_type = WP_Block_Type_Registry::get_instance()->get_registered( $block_metadata['name'] ); | |||
| if ( ! block_has_support( $block_type, array( '__experimentalLayout' ), false ) ) { | |||
| if ( ! block_has_support( $block_type, array( 'layout' ), false ) ) { | |||
There was a problem hiding this comment.
Should this one also fall back for __experimentalLayout if layout isn't available?
There was a problem hiding this comment.
Might as well add it in!
|
Size Change: +19 B (0%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
|
Thanks so much for opening this one! 🎉 Just added a couple of tiny drive-by comments, but will hopefully give this a more detailed look tomorrow.
Good question — since the layout panel changes are primarily UI changes, I suppose my first thought is that it might not really affect whether or not we consider the layout block support as being stable. For example, the padding and margin spacing controls have been stable for a long time, but still regularly go through design iterations for the UI such as when spacing presets were introduced. In terms of stability, I imagine it's more important that the CSS and classname output from the layout block support is stable / backwards compatibility is maintained, along with the API structure? |
|
Flaky tests detected in c1ff0a2. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5275435392
|
packages/block-editor/README.md
Outdated
|
|
||
| _Returns_ | ||
|
|
||
| - `string`: CSS rule. |
There was a problem hiding this comment.
For me LayoutStyle, useLayoutStyles and useLayoutClass should be both private APIs based on these. Most blocks shouldn't have to use them to get the layouts working properly.
There was a problem hiding this comment.
That would be ideal, but given that they have been publicly exposed as __experimental already, is it still viable to make them private now? A quick search in wp directory shows a few hits for each, though they seem to be minified build files so I'm guessing they're just from included packages.
There was a problem hiding this comment.
I just went ahead and did it anyway 😄 it's unlikely those are actually in use anywhere. In any case, we can revert if there are objections!
There was a problem hiding this comment.
If it's found that they're used, we could deprecate the public ones for some time, that said, I also think it's very unlikely that any plugin would use these.
tellthemachines
added
[Feature] Layout
There was a problem hiding this comment.
Nice work! Just gave this a re-test:
✅ Blocks and layout support is working correctly in post editor, site editor, and on the site frontend, I didn't run into any issues with any blocks
✅ Code changes all look good, and the fallback logic means that any existing blocks using __experimentalLayout should continue to work
I agree with the decision to move the three experimental exports (LayoutStyle, useLayoutStyles and useLayoutClasses) to be private. From the sounds of it, the risk of removing the experimental exports is minimal, and removal sounds preferable to either stabilising or keeping them as experimental indefinitely. I'm not too across the current decision making process over when we need to deprecate experimental exports prior to removal, so happy to defer to other people on that one if anyone feels strongly about it.
This LGTM! ✨
| @@ -513,7 +513,7 @@ In addition to styles at the individual block level and in global styles, there | |||
|
|
|||
| The layout block support is an experimental approach for outputting common layout styles that are shared between blocks that are used for creating layouts. Layout styles are useful for providing common styling for any block that is a container for other blocks. Examples of blocks that depend on these layout styles include Group, Row, Columns, Buttons, and Social Icons. | |||
|
|
|||
| While the feature is part of WordPress core, it is still flagged as experimental in the sense that the features and output are still undergoing active development. It is therefore not yet a stable feature from the perspective of 3rd party blocks, as the API is likely to change. The feature is enabled in core blocks via the `__experimentalLayout` setting under `supports` in a block's `block.json` file. | |||
| While the feature is part of WordPress core, it is still flagged as experimental in the sense that the features and output are still undergoing active development. It is therefore not yet a stable feature from the perspective of 3rd party blocks, as the API is likely to change. The feature is enabled in core blocks via the `layout` setting under `supports` in a block's `block.json` file. | |||
There was a problem hiding this comment.
Not for now, but once 6.3 is released, let's remember to come back to this document to update the wording to reflect that it's no longer experimental.
andrewserong
added
the
[Type] Code Quality
youknowriad
added
the
Needs Dev Note
|
Smoke tested layout blocks and LGTM! I fell short of testing native... couldn't get the thing running in time. Happy to try again tomorrow. |
tellthemachines
force-pushed
the
try/stabilise-layout
branch
from
fab487d
to
c1ff0a2
Compare
|
Thanks for the feedback and testing folks! I'm going to go ahead and merge this as is. I'm fairly confident from the wp directory results that those now-private APIs aren't being used anywhere, and we can always add a deprecation in later if really needed. |
Dev noteLayout support is now stableBlock support for layout has now graduated from experimental to stable. Everything works the same; the only difference is that when adding block support for layout in Code example (in |
|
@tellthemachines I was trying to look for any documentation of the |
There is this doc already; we can update it now that layout is no longer experimental and link to it in the dev note. I'll get onto that next week! |
I hadn't seen that doc yet. That is already good to know. What I'm looking specifically though is the documentation for the sub options of the So more of an API documentation than documentation of the layout system itself. |
|
I see, that makes sense! Would that doc be the best place to add the API documentation or is there somewhere better? |
|
@tellthemachines the Info about the supports settings would best fit here: https://github.com/WordPress/gutenberg/blob/trunk/docs/reference-guides/block-api/block-supports.md |
* Try stabilising layout and its associated APIs * Check for experimental layout everywhere * Mark internal layout APIs private
What?
Marks
__experimentalLayout,__experimentalLayoutStyle,__experimentalUseLayoutClassesand__experimentalUseLayoutStylesstable.In view of the likely change of the whole layout panel UI in #49459, we could maybe leave
__experimentalLayoutStyleas experimental. Thoughts?Testing Instructions
__experimentalLayouton some of its blocks. I tried Material Design, but there are more that can be found by searching the wp directory for__experimentalLayout.Testing Instructions for Keyboard
Screenshots or screencast