Skip to content
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

Cover Block: Opt in to the experimental Custom Spacing control. #2277

Closed
iamtakashi opened this issue Jul 21, 2020 · 12 comments
Closed

Cover Block: Opt in to the experimental Custom Spacing control. #2277

iamtakashi opened this issue Jul 21, 2020 · 12 comments

Comments

@iamtakashi
Copy link
Contributor

iamtakashi commented Jul 21, 2020

Varia Most of the themes don't seem to have opted in the spacing option for the Cover block. It seems easy to implement, and the customer would be able to use it as soon as we add the support to the themes.

Edit: This will require theme.json support, see below. (@kwight)

Ref: WordPress/gutenberg#21492

This feature is opt-in via add_theme_support. In order to show the padding controls, the following needs to be added to a theme's function.php
add_theme_support( 'experimental-custom-spacing' );

cc @ianstewart @alaczek

@iamtakashi
Copy link
Contributor Author

Same as #2449, I'll change this to all themes (that specified in #2449) issue. cc @jeffikus

@iamtakashi iamtakashi changed the title Varia themes: Opt in Custom Spacing Control for Cover Block Opt in Custom Spacing Control for Cover Block Sep 18, 2020
@jordesign jordesign self-assigned this Jun 28, 2021
@jordesign
Copy link
Contributor

Happy to handle this - starting with Varia and Seedlet?

@jordesign
Copy link
Contributor

I've issued a PR - which seems to add the control for Varia. Wanted to get it checked before I move on to Seedlet.
#4160

@jordesign
Copy link
Contributor

I'll confess I haven't been able to get this working.

add_theme_support( 'experimental-custom-spacing' ); seems to have no effect - and add_theme_support( 'custom-spacing' ); still uses the original style.

@iamtakashi does this padding option/property seem like something that is still possible?

@kwight
Copy link
Contributor

kwight commented Jul 12, 2021

@jordesign It looks like it should be add_theme_support('custom-spacing'): WordPress/gutenberg#27045

What do you mean by "still uses the original style"?

@jordesign
Copy link
Contributor

add_theme_support('custom-spacing') is already in place in Varia at the moment - and the result is this padding control:

Screen Shot 2021-07-13 at 11 47 24 am

But I haven't been able to enable the controls shown in WordPress/gutenberg#21492

@kwight
Copy link
Contributor

kwight commented Jul 14, 2021

@jordesign Hm, this may have been removed since it was merged; for example, gutenberg_extend_settings_custom_spacing is a function introduced in WordPress/gutenberg#21492, which is no longer present in the client-assets.php file. (I also confirmed it's not in the plugin version we have running on dotcom). If you like, you can try tracing back from the current version of the file to see when it was removed.

Maybe @jeffikus or @scruffian have more insight?

@jordesign
Copy link
Contributor

It looks like it may have been removed in WordPress/gutenberg#25141 in order to have support added as part of theme.json.

@jordesign
Copy link
Contributor

Alrighty - @MaggieCabrera was super helpful with some advice here:

I think last time we tried to do this we found that we had to add a theme.json file to enable it. By doing that we are opting in to the new layout controls and that requires a whole other degree of refactoring as to not break the alignments of the theme. I’m not sure what we gain from adding the controls right now is worth the amount of work from refactoring the alignments we would have to do.

I'd tend to agree with that - but happy for @ianstewart and @kwight to weigh in?

@kwight
Copy link
Contributor

kwight commented Jul 15, 2021

I'd tend to agree with that - but happy for @ianstewart and @kwight to weigh in?

Sure, it sounds like the Theme Team has been down this road before, and our pod is in no better position to start tackling broad theme.json support at this point (now that I think about it, this sounds vaguely familiar, Ben may have described that to me somewhere). We'll leave this open as it's still valid, but let's move on to other things @jordesign – thanks for digging in!

@kwight kwight changed the title Opt in Custom Spacing Control for Cover Block Cover Block: Opt in to the experimental Custom Spacing control. Jul 15, 2021
@jeffikus
Copy link
Contributor

@jordesign is this still an issue? If not let's close this out.

@jordesign
Copy link
Contributor

Thanks for checking @jeffikus - confirming this is no longer an issue - I'll close it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants