Native gutenberg templating and styles

[kellymears]

The ideal version of this PR replaces Sage's _wp-classes.scss but the reality is a bit more complicated. Note that this PR brings the total build:production up to 229kb. If you compare that with the Gutenberg default styles, I believe it is a net savings.

I tried to adhere to Bootstrap standards as much as possible but it has been a couple years since I have used this framework. I imagine that there are more bootstrap-y ways to do many things within this PR, and I think it's probably best if someone who is already opinionated about usage of Bootstrap to incorporate them. I have tried to structure my work in a way that will make that process approachable.

Note that this PR also includes an additional composer dependency: querypath/querypath. It is utilized in filters.php to wrap .alignwide and .alignfull blocks using a div with the class .gutenberg-wrap applied. This makes it much easier to deal with these elements.

This analysis / study, along with many, many hours of frustrating experimentation, have led me to believe this is the correct approach. Also note the adoption of this method by Gutenberg contributor Chris Van Patten. To me, this component of the PR serves as a reasonably lightweight fix for an issue in a very unpredictable upstream.

This PR also restructures the starter template so that Gutenberg can have the full viewport. While it is possible to have Gutenberg output restricted to a container with a width less than 100vw there are issues that crop up when dealing with .alignwide and .alignfull elements. The main solution I've seen has been to utilize something like this:

.entry-content .alignfull,
.entry-content .alignwide {
    margin-left: calc( -100vw / 2 + 100% / 2 );
    margin-right: calc( -100vw / 2 + 100% / 2 );
    max-width: 100vw;
}

This works pretty well but it is not 💯. As detailed in the codepen.io study above, this approach causes issues with wp-block-cover that have a fixed background on various mobile devices. This is why I believe the approach in this PR is something closer to a best practice and am utilizing it in my own projects.

There are still some missing pieces of functionality, but this PR is already rather broad and I do not want to FUBAR it with complexity.

[MWDelaney]

Hot damn. I would be happy to look at the bootstrappiness of it some time this coming week. This was a lot of work. I’m not the one who makes the decision to merge it, but thank you for this effort.

[darrenjacoby]

I'm going to play devils advocate here.

Does it make sense to write these classes using Bootstrap? (and even existing CSS in Sage) If the user is to select a custom framework from the Roots installer, then this CSS will be cleared. My guess would be quite a few Sage users don't use Bootstrap these days.

The above code could get more use being written without a framework dependency. It shouldn't be hard to re-write this without a framework. I would be happy to help looking into the blocks CSS to convert.

[darrenjacoby]

@extend can also work in ways that not all users writing CSS expect, so I would avoid using it. @include would be safer, but just plain CSS is even better. @extend text-center is no more reusable than text-align: center. Same with @extend d-none, etc. Those classes can make sense as class utilities, but not in CSS itself imho.

[kellymears]

@darrenjacoby my original PR was framework agnostic

[darrenjacoby]

I feel like that's the better route.

[kellymears]

That said, I’ve seen those css skills .. please feel free to take a pass by all means !

[MWDelaney]

My understanding is that Sage is supposed to work with Bootstrap by default without the installer. Alternative versions of this code would need to be written for the other frameworks and added to the installer, but this is still an appropriate addition to Sage.

Re, @extend, Bootstrap doesn’t have @include mixins for most of these. I see the criticism of .text-center but I think margin and padding classes, which will use the Bootstrap spacer variable, are still a good choice here.

[darrenjacoby]

Yea, it may be a broader question. That is a fair amount of CSS to maintain for each framework, especially if more frameworks are added in the future. CSS frameworks often overhaul a lot on each major release too. Changing the default Sage CSS to be framework agnostic would be the most robust option imho. Just my opinion though, others may feel different.

[MWDelaney]

I'm happy with whatever the solution is for this, and I'm happy to believe that I went overboard on the @extends. Generally I support replacing the default WordPress block styles with Sage styles to make them more extensible/replaceable.

[kellymears]

I could see this decision going either way and be happy with the outcome.

Thankfully all of the block css lives in one directory, so the process of swapping out a bespoke set of block styles written for a different framework should be as easy as overwriting a single folder. That’s what I’ll be doing, at any rate.

Regardless, having a path forward to customize the base styles is a good thing to have now. Letting the default one load and then trying to overwrite it is a huge pain with regards to selector specificity — no matter if you’re using bourbon or UIKit. It’s also non negligible in terms of network and rendering performance.

The WordPress of tomorrow handles pretty much every element of the site through the editor. At that point it’s not just a “good thing to have”, it’s essential to the task of theming.

[mmirus]

Definitely agree with Michael - huge props to you for putting this together, @kellymears!

Also agree with @darrenjacoby that it makes the most sense to implement this in a framework-agnostic way unless there's a strong reason for going with something that's BS-specific.

CC @retlehs - Ben probably has the best perspective on this!

[MWDelaney]

@mmirus I presumed the BS specificity because Sage is already BS-specific by default. Granted the default styles aren't nearly as extensive as these, but they're there.

[mmirus]

TLDR - I'm not sure what the benefit is of doing this in a BS-specific way, and it would require a lot of additional overhead compared to a vanilla SASS implementation.

...

@MWDelaney That's fair, but it's also intended (I believe) to be quite decoupled from any given framework, including BS. From the user's end, currently, if you select any of the frameworks, you have a similar starting point. Even if you select no framework, you're not losing much. From our end, the current overhead of maintaining the default styles for each framework is also pretty minimal, since they just give a really bare bones starting point.

If we build this in a BS-specific way and don't reproduce it for other frameworks, we're coupling ourselves much more closely to BS than is currently the case. I don't think that's a good option.

On the other hand, if we build it in a BS-specific way and want to keep parity for other frameworks (maintaining Sage's current framework independence), then I share @darrenjacoby's concerns about making these styles BS-specific:

• they have to be reproduced and maintained for every framework we want to include in the installer
• we have to update them to keep up with BS and n other frameworks, not just core updates

On top of that, it likely at least marginally increases the complexity of the installer. All told, that's significant overhead.

Finally, it would put people who don't want to use any built-in framework in a worse spot relative to today, unless we also provide a vanilla SASS implementation--at which point, we might as well just use that for all of them.

For these reasons, I think implementing this in a framework-agnostic way is the best option.

On the other hand, I don't see any benefit to doing it in a BS-specific way--totally possible I'm missing something there!

[MWDelaney]

I think we're combining two very real, but very separate concerns here.

Sage is supposed to be Bootstrap by default by design, which is why BS styles are committed to the repository. The installer, if run, will replace those styles with framework-appropriate styles if another framework is chosen. These are kept up to date separately.

So yes, it's a lot of new Sass to maintain, and it has to be maintained across two repositories (this one, and sage-lib), but this PR is inline with Sage's design intent (Bootstrap by default)

[mmirus]

I guess the way my brain is getting to grips with this question is:

1. In philosophical or design intent terms: yes, Sage is BS-by-default, but only in a very minimal way. I don't see adding vanilla SASS to solve this problem as violating that, at least not in a way that has any negative consequences.
2. In practical terms, adhering strictly to that approach in this case does not have benefits (that I can see) but it does have significant negative consequences.

Definitely not trying to knock any of the work that was done to implement this in a BS-specific way, and hope I'm not coming across like that! I just don't think it's the best move.

If there are significant practical downsides to putting vanilla SASS in Sage or significant practical benefits to doing it in a BS-specific way that might outweigh the downsides, I'm more than open to going that route--just not seeing those from my perspective.

I've probably added as much noise here as is helpful at this point. :) I'll let wiser heads weigh in.

[MWDelaney]

Some benefits I can see:

• All Bootstrap variable overrides will affect these styles predictably. If the variable $font-weight-bold is overridden in your theme, you can be sure that bold text in blocks will follow suit. This could be done with variables instead of @extends here, but it would still be a very Bootstrap-heavy implementation. I think we if eschew the @extends, we'd need to use variable values from Bootstrap instead so that the styles behave predictably as variables are overridden in Sage.

• Container and breakpoint widths will match the rest of the Bootstrap widths on the site. It's always a huge pain when a page-builder or other tool (like Gutenberg) use a different set of breakpoints or container widths than your site does.

• Learning? There might be value in showing off the utility classes here?

It's a short list. And I get where you're coming from.

[kellymears]

Maybe it makes sense to build out the _variables.scss API more so that swapping out frameworks is less onerous and thereby less of a big deal? So, for instance: $gutenberg-primary: $primary, $gutenberg-weight-bold: $font-weight-bold, et al. This seems like it would be easier than having to change things out in 10+ places, per project.

I'm fine with taking this on, if it would be helpful.

[darrenjacoby]

This conversation ultimately leads to if Sage should remain BS specific. I feel like Sage has matured a lot, and it may be worth reviewing if it should remain BS specific. I always remove all CSS and JS from Sage and replace with my own.

When it comes to frontend frameworks, and CSS/JS in particular, users seem to be way more opinionated in their use. BS feels a bit like jQuery, both were obvious choices a few years ago, but that has since changed with more (and better) options available.

As far as I know, Ben included reporting for the Sage installer, having some stats may be able to assist with the correct route to take here.

However, I still back a framework agnostic approach for Sage, and a minimal CSS implementation overall. I feel like users should now be comfortable bringing in new dependencies on their own.

[retlehs]

for the sake of this PR, everything should be bootstrap by default. in the installer we can replace the missing bootstrap variables w/ some sensible defaults. the stats show that bootstrap is chosen 3x more than 'none', and the other frameworks are rarely chosen (tailwind slightly beating foundation, no one really picking bulma or tachyons).

i agree with @kellymears on adding more variables to sage to help account for things

the installer will also have a new option for opt-ing out of including gutenberg styles

if we are going to include color and font-size support for gutenberg in sage, i'd like to automate it rather than using colors and font-sizes that aren't from the theme variables

from my todo on #2118:

add add_theme_support('editor-color-palette') based on colors from theme (auto-save the theme CSS variables to JSON (ref))

[kellymears]

@ouun This is such an excellent and needed contribution! Thank you!

I think this is definitely the way to go and i'll be adding something along these lines when I get a moment.

evidently i need to catch up on what new block filters are available...

[MWDelaney]

I know this PR is getting bigger and bigger, but it seems to me that the one big missing feature is color pallets. @Log1x had a good idea for how to pull this off with Sage, making the Sage $theme-colors array available in js where Gutenberg needs it.

I really don’t know how much more work it would be, but it would make this effort really complete.

[ouun]

@kellymears is there something to do one can assist. Really looking forward to see with what you'll come up at the end and of cause to use a better approach as currently.

I am also wondering how this will be usable with Tailwind as it is currently very related to Bootstrap, isn't it?

[kellymears]

@kellymears is there something to do one can assist. Really looking forward to see with what you'll come up at the end and of cause to use a better approach as currently.

Hey @ouun thanks for the offer. I had a lot of work land and that pretty much totally wiped out the past two weeks for me. Good problem for me, but still, apologies that I haven't pushed to this. I'll have something as soon as I can :)

I am also wondering how this will be usable with Tailwind as it is currently very related to Bootstrap, isn't it?

In its current state this implementation does not require Bootstrap, but it does utilize Bootstrap if present. I'm doing this with an @mixin that checks for a sage $variable that the themer can use to indicate if they are using Bootstrap:

sage/resources/assets/styles/components/blocks/_utilities.scss

Line 26 in eb3caeb

@mixin bootstrap-else($bootstrap, $fallback) {

Then, in any of the .wp-block-* partials, I can describe the fallback to be used if bootstrap is not enabled, and pass those in to the mixin, along with the bootstrap classes that represent Sage default. I do this with %extend syntax so that only one or the other is compiled during build -- no extra weight. This doesn't really add that much overhead or maintenance burden to the SCSS and means that the block styles can be used or modified by anyone, regardless of framework, while also leveraging Bootstrap if it is present (it is the most popular Sage implementation by far).

Example usage, currently implemented, would be for the .wp-block-button class:

sage/resources/assets/styles/components/blocks/_wp-block-button.scss

Line 2 in eb3caeb

%wp-block-button__link {

As for Tailwind specifically -- I love it (I don't really use Bootstrap, actually, although I think it is fine)! And I have a repository where I'm trying to compile block styles for major frameworks if you would like to take a shot at doing something with Tailwind and contributing: https://github.com/kellymears/wp-block-styles-project. In the end, I think a proper Tailwind plugin would be the ultimate jam and I would love to do something like that.. but am obviously overcommitted at the moment.

Cheers!

[kellymears]

Added support for blacklisting blocks, unregistering style variants for blocks, and registering new style variants for blocks. All of this is configured through a single json file in the assets dir.

https://github.com/roots/sage/blob/57bddf9ca82a9f32551ffcda18f53f1c72b6e961/resources/assets/editor.json

This configuration file also generates the sass variable map used by the block styles themselves. That transform is handled with a Webpack plugin (based on an idea by @Log1x). This means that all colors, and font-sizes defined in this config file are automatically available to both the editor and public posts and only need to be maintained in one place.

Looking ahead towards Sage10 I have bundled said Webpack plugin as a laravel-mix extension (https://github.com/kellymears/laravel-mix-json-to-sass-map) and it works fine in that context as well. So, not a dead-end despite the upcoming changes in build tooling.

This also adds some new stylesheet rules for the core/cover block.

[greatislander]

FYI, I was trying this out and I ran into a weird Gutenberg bug with the Media & Text block and your use of the render_blocks filter. I opened a ticket, which was closed. It looks like the underlying issue will be addressed, but at the moment Media & Text blocks with the default (wide) alignment will not be given a wrapper as the default alignment is not parsed from the block markup.

[kellymears]

@greatislander nice catch 👍 . This might be approachable by utilizing the render_block_data filter, which is called prior to render_data and allows for parsing block data that is not reflected in the markup (I think that's what's happening here).

https://developer.wordpress.org/reference/hooks/render_block_data/

[greatislander]

@kellymears Interesting! I'll check that out.

[kellymears]

I figure a quick note is in order —

While this PR will probably not make it into Sage 9, and my attention has switched to Sage 10 for the most part, it seems to have helped a lot of people figure out how to get started. Thank you all for your kind messages both here and on the Roots discourse.

I’d suggest leaving this PR open for at least a while longer for anybody who uses it as a reference (people still ask me questions about it from time to time). It has issues but it’s really not a bad place at all to start from. However, if anybody was waiting for this to become canonical I’m flagging that this is not currently being worked on and probably will not ultimately be a part of any mainstream release.

My plan is to close this when Roots releases Sage 10.

[GerardKetuma]

Does sage 10.0.0-dev (and hence Sage 10) support Gutenberg?

[Log1x]

@GerardKetuma keep an eye on #2338 – @kellymears and I are brewing up something to add to it related to this :)

[dsturm]

Trailing commas in function calls require PHP 7.3. Is this intended?

[dsturm]

Could / should this be simplified to

if (in_array($block['attrs']['align'], ['wide', 'full'], true)) {
    return $block_content = '<div class="wp-block-wrap wp-block-' . $block['attrs']['align'] . '-wrap">'. $block_content .'</div>';
}

or alternatively should we use strict comparison?

[dsturm]

If the 'editor.json' is missing (or corrupt) this throws an error. Should we check for this?

[kellymears]

Hey @dsturm these are all great suggestions ✨, but this PR is pretty much not going anywhere and has been left up purely as a resource for others.

If you submit a PR to my branch I'd accept it but my last commit to this was in May of 2019.

I'm sure a lot of the CSS is super out-of-spec by now. The editor markup has changed a lot. Even the anticipated markup for full/wide align blocks, which you commented on, doesn't match up with blocks' actual markup anymore, I don't believe.