Add spacing presets to the spacer block #44002
Conversation
|
Size Change: +394 B (0%) Total Size: 1.35 MB
ℹ️ View Unchanged
|
|
(replying to your comment in #39371 here to keep things a bit more organized)
The grab handle snapping between the presets was my immediate thought, but I think just removing it when custom values are disabled would be fine as an MVP if that means it can ship in 6.0. Personally, having responsive clamp-driven sizing on it in any capacity is greatly preferable to waiting another 3-4 months just so it will work with the grab handle. |
|
Currently a new spacer block defaults to 100px, do we instead want to default it to |
562a0a5
to
8e3da3f
Compare
|
I have hit a potential blocker with this in that the Mobile app currently allows editing of the Spacer block height and width, and if we start adding values like @fluiddot - do you have any thoughts on how to best handle an update to the Spacer block that involves adding in the ability to select from presets as well as custom values? |
|
Hey @glendaviesnz. 👋🏻 Thanks for reaching out regarding the native editor experience. It looks like the native editor CI tests are failing. I noticed the following error occurs when running this branch in the native Demo editor, which may explain some of the failures. The error relates to the lack of exporting Native Editor Error
Yes, using a native app version that cannot parse the saved block state would likely display a invalid block error. A user attempting to recover or modify the block may lead to losing the preset setting for the block. In regards to avoiding this scenario, my understanding is that we have little ability to guarantee it will not occur. In the past, I believe we have looked to reduce the likelihood that it occurs by shipping a native mobile release with support for the new block structure a few weeks before the web release, which provides time for users to upgrade the native app. @geriux is it accurate to say this is the approach we took with List V2 and it is relevant here?
It looks like the presets are based upon CSS custom properties. In order to support those in the native editor, we may need to expand the categories of custom properties we parse within @geriux WDYT? Am I interpreting the global styles code correctly? Any thoughts on size of effort for this? |
This is correct, I've just tested the following snippet in the current production version of the app and it shows up
It looks like it, I believe we might need to have to go with an approach similar if not the same to Quote, List V2.
This is different because the value is within the block's attributes so we will need to parse these values in the block itself. If we want to support default style values for the Spacer block coming from the |
|
Hey @glendaviesnz. 👋🏻 The mobile team discussed this subject in chat further. We captured the apparent blocker for merging this PR in #44234. We have a few competing priorities, but hope to spend time adding support for the native mobile editor next week. Once the mobile support merges, we would include it in the next release. We create a new release every two weeks, which is then included in the subsequent WordPress mobile app release. Ideally, we allow users a couple of weeks to update their installed app after the WordPress mobile app release is published to the stores before shipping the web change. Please let us know if you have any thoughts, questions, or concerns about this. Thank you! 🙇🏻 |
|
Thanks for the update @dcalhoun, we have missed the 6.1 deadline on this so no panic to get the change in, so the timeframes you mention are no problem. |
|
It looks like the blocker is cleared for this. Are we good to pick this back up? |
840188d
to
1e1401b
Compare
|
Thanks @richtabor I have rebased this if you want to give it a test |
|
Flaky tests detected in 5d19263. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4612262918
|
|
@geriux, @dcalhoun it looks like we should be all set to merge this from the mobile side now with this merged - is that the case? |
@glendaviesnz how quickly would this feature be included in a Gutenberg release if it were merged? I.e. when is the next web release? I believe the WordPress 21.9 mobile app release — which includes the mobile change — was delayed a bit. It may be one to two weeks before a majority of users receive the update from the app stores. @geriux does this align with your understanding as well? |
|
@dcalhoun 15.4 of the webclient is going out this week I think, so would be another two weeks after that. I don't believe there is any urgency around getting this in, so maybe the safest option is for you to ping me on this PR when you think the mobile client releases have been in the wild long enough to cover it and I will merge it at that point? |
|
@glendaviesnz I set a reminder for myself to follow up one week from today. 👍🏻 |
|
Works well, I'm just seeing the control a bit out of line:
|
The space between the label and the input (both Y sides of the label). It's different on trunk vs. this PR. 🙃 |
|
@richtabor - I think it is closer now:
the one on left is trunk. |
|
Looks good to me, thanks! |
|
@glendaviesnz I believe the install base of the WordPress 21.9 app is to a percentage where it is relatively safe to merge this work for a web release. Thank you for your collaboration with the mobile team. 🙇🏻 |
a5fb088
to
5d19263
Compare
|
Thanks for confirming |
What?
Adds spacing presets to the spacer block
Why?
This has been requested by theme authors to allow better control of spacing across a site
How?
Adds the new spacing presets control to the Spacer block
Testing Instructions
settings.spacing.spacingScale.stepsto 0 and make sure only the custom sizing option appearssettings.spacing.customSpacingSizeto false and check that custom sizing and resizable box do not show in Spacer blockScreenshots or screencast
spacer.mp4