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

Adds an API for enqueuing assets to the theme or editor of gutenberg. #1420

Closed
wants to merge 1 commit into from

Conversation

BE-Webdesign
Copy link
Contributor

@BE-Webdesign BE-Webdesign commented Jun 24, 2017

4 top level functions can be used for adding extra assets.

gutenberg_add_block_editor_style
gutenberg_add_block_theme_style
gutenberg_add_block_editor_script
gutenberg_add_block_theme_script

Each of these will register the assets provided to the respective
location.

A quote block is registered server side so that we can register styles
to it.

mock implementation

In this PR I am registering the quote block server side so the API can be tested out.

Create a css file in your active theme like this:

quote.css:

.blocks-quote-style-1 {
    background-color: red;
    color: white;
}

.blocks-quote-style-2 {
    background-color: blue;
    color: orange;
}

Then somewhere in that themes functions php, preferably on the after_setup_theme hook, add this line:

gutenberg_add_block_theme_style( 'core/quote', array(
  'handle' => 'quote-colors',
  'src'    => get_template_directory_uri() . '/quote.css'
) );

This will register the special stylesheet to be loaded by the theme. Make sure you have added a quote block to your post, and on the front end you should now see a hideous red quote with white text. Click to block style two in the editor and you should see the blue bg with orange text. This should not apply in the editor.

To add these styles to the editor as well simply add this line right after:

gutenberg_add_block_editor_style( 'core/quote', array(
  'handle' => 'quote-colors',
  'src'    => get_template_directory_uri() . '/quote.css'
) );

** Testing Instructions **
Verify PHPUnit tests pass.

Verify that you can add custom assets to the theme and editor views.
NOTE Currently only registered blocks are supported, so you will need
to register styles to the quote block or latest posts block.

4 top level functions can be used for adding extra assets.

`gutenberg_add_block_editor_style`
`gutenberg_add_block_theme_style`
`gutenberg_add_block_editor_script`
`gutenberg_add_block_theme_script`

Each of these will register the assets provided to the respective
location.

A quote block is registered server side so that we can register styles
to it.

** Testing Instructions **
Verify PHPUnit tests pass.

Verify that you can add custom assets to the theme and editor views.
_NOTE_ Currently only registered blocks are supported, so you will need
to register styles to the quote block or latest posts block.
@BE-Webdesign
Copy link
Contributor Author

@joyously

I remember vaguely that you were looking for a way to register custom styles to Gutenberg, both in the editor and on the front end. Let me know if you feel this is an easy enough way to do it.

@joyously
Copy link

@BE-Webdesign
Yes, I think themes need to be styling the blocks, not the editor.
No, I do not like how this works at all. I see two problems.

  1. The whole point of a visual editor is to see the finished result in the editor as it is created, so the editor should match what is shown on the front end. You have divided that into two parts, so if done wrong, they won't match. The editor should be relying on the theme for styles, and not supplying any styles for the "special" blocks. So you should see straight browser defaults if the theme does not supply styles.

  2. Styles cascade, so having to register styles for each block is a terrible idea. I think you should plan on long-term having the existing function add_editor_style() be what it does today. For the short-term testing of the plugin, have a similarly named function for loading an entire stylesheet to affect the "special" blocks inside the editor. If it's all about the new classes, then it can even be the same stylesheet that is used on the front end. But if the structure is different inside the editor, then the selectors have to morph a little to match.

@nylen
Copy link
Member

nylen commented Jun 26, 2017

@joyously

Yes, I think themes need to be styling the blocks, not the editor.

We will need a combination of both. Core will bundle blocks which should come with their own default styles; plugins will also bundle blocks with styles. This will ensure that e.g. themes which know nothing about blocks will still look reasonable. Then, themes will be able to override/modify these styles if desired.

The editor should be relying on the theme for styles

This isn't really workable. Blocks must have different styles for editing (interacting with the controls inside of a block) than for viewing. We hope that many of the styles will be shared, but there will always be a need for editor-specific styling.

Styles cascade, so having to register styles for each block is a terrible idea.

I am not sure I follow your point here, but there is no way around registering styles for each block.

@nylen
Copy link
Member

nylen commented Jun 26, 2017

What is the benefit of registering a block's assets outside of the register_block_type call? I had imagined something more like this:

register_block_type( 'core/quote', array(
    // ...
    'stylesheets' => array(
        'main' => ... // or 'block'?
        'editor' => ...
    ),
) );

I am also not sure if we would need all the traditional arguments for these stylesheets (handle, deps, ver, in_footer). Maybe we could infer or require specific values for some of these?

@BE-Webdesign
Copy link
Contributor Author

BE-Webdesign commented Jun 26, 2017

I am also not sure if we would need all the traditional arguments for these stylesheets (handle, deps, ver, in_footer). Maybe we could infer or require specific values for some of these?

Right now we are merging in defaults to the array. Maybe we should't do that at all and purely rely on wp_enqueue_script to do the default handling.

What is the benefit of registering a block's assets outside of the register_block_type call? I had imagined something more like this:

This is for people who want to add special styles. You can register styles with register_block_type still like this:

register_block_type( 'core/quote', array(
    // ...
    'assets'          => array(
        'editor'      => array(
            'styles'  => array(
               // Style data.
            ),
        ),
        'theme'       => array(
            'scripts' => array(
              // Script data.
            ),
        ),
    ),
) );

Then a developer could extend those styles via:

gutenberg_add_block_editor_style( 'core/quote', array(
    'handle' => 'my-cool-styles',
    'src'    => 'url-to-my-cool-styles',
) )

@joyously
Copy link

Yes, I think themes need to be styling the blocks, not the editor.

We will need a combination of both. Core will bundle blocks which should come with their own default styles; plugins will also bundle blocks with styles. This will ensure that e.g. themes which know nothing about blocks will still look reasonable. Then, themes will be able to override/modify these styles if desired.

If you look at the existing editor, you will see it has a skin that includes some styles for WordPress default classes such as alignleft and alignright, which makes these look correct in Visual mode. The theme can have those styles in their editor-style.css also, which should override. However, Core does not supply anything for these classes on the front end. It is all up to the theme. The content in the editor is just HTML, and it will look like that HTML looks if the theme does not style it. Core should not be outputting any styles on the front end. Unless the blocks have inline styles, which I hope they don't as this is not very theme friendly, how the front end looks should be totally up to the theme. And if you want the Visual mode to look like the front end, the theme should be involved in that.

The editor should be relying on the theme for styles

This isn't really workable. Blocks must have different styles for editing (interacting with the controls inside of a block) than for viewing. We hope that many of the styles will be shared, but there will always be a need for editor-specific styling.

Yes, it is workable. TinyMCE does it just fine. I know the internal structure can be different, but using class names should allow styling regardless of the slight differences in structure. That's why the theme supplies a different style sheet for the editor than the front end.

Styles cascade, so having to register styles for each block is a terrible idea.

I am not sure I follow your point here, but there is no way around registering styles for each block.

Just look at the current editor and add_editor_style(). You just load the style sheet that the theme gives you, and you don't have to have a lot of code knowing about blocks. The style sheet would have all the specifics about the blocks handled, and no code is needed from the theme. Plugins creating their own blocks would of course have code and styles and load them on the front end also, as always.

@samikeijonen
Copy link
Contributor

So Core will handle basic styles for all blocks in the front-end? I'm ok with that direction as long as there is easy way to unhook (remove) them. And in that case I would be responsible to write my own CSS for the blocks in a theme.

@mtias
Copy link
Member

mtias commented Jun 27, 2017

@samikeijonen yes, core will provide structural CSS for all the native blocks (gallery columns, quote citations, image captions, block alignments). This should both be easy to extend (low specificity and reliable classes for once) and easy to dequeue if you so desire.

@obenland
Copy link
Member

I wonder if this could be simplified by letting themes register scripts/styles and let Gutenberg enqueue them as needed. That way the API could be limited to just passing on script/style handles.

Additionally we could think about letting themes register support for certain (or all) blocks and refrain from enqueueing the structural CSS that @mtias mentioned above, something along the lines of _custom_logo_header_styles().

@mrwweb
Copy link

mrwweb commented Jun 28, 2017

Before getting into how a specific block styles can be registered, there are some basic editor styles that I still am unclear on. I could also imagine that any more advanced solution would be an outgrowth of that.

Primarily, how are the basic typography styles registered. Things like:

  • Body font
  • Heading sizes
  • Link colors
  • etc.

I think this is maybe getting at what @joyously talks about with "cascading." By scoping CSS to a specific block, it's not sustainable to register a stylesheet with font definitions for every single block (including those used for plugins).

@joyously
Copy link

Yes, thank you @mrwweb. Why have lots of code when all you need is one style sheet loaded?
I imagine a plugin could register a stylesheet for the editor only, such as http://ffoodd.github.io/a11y.css/
without adding any blocks. You might want to test with that one, actually, as it gives you a visible message if it's not accessible markup.

@BE-Webdesign
Copy link
Contributor Author

There are two camps here and both have valid points for why they should work, and I think we should support both. I think using add_editor_style is a great idea, but that shouldn't preclude registering assets for blocks individually and vice versa.

Why have lots of code when all you need is one style sheet loaded?

There is a certain duality to this idea, because one could say: Why have all of this CSS if I just need the text block styles? Editor styles built as one stylesheet tend to add to the CSS bloat problems WordPress already struggles with.

Styles cascade, so having to register styles for each block is a terrible idea

Terrible is a strong word and doesn't lend itself to good communication. CSS cascades and you can use that to your advantage when registering styles for each block. Blocks do not need to know what their typeface, font size, color, etc. is, unless you want to specifically override that. So if you define typeface, color, etc. at a higher level, your styles will inherit everything. This way you can reuse the block specific styles on either the front end or the back end and the theme or editor could have their own "global" set of styles that will apply and cascade over each block.

@joyously
Copy link

This way you can reuse the block specific styles on either the front end or the back end and the theme or editor could have their own "global" set of styles that will apply and cascade over each block.

Yes, that's what we're talking about -- getting the editor to look like the front end. So there should be a way to add a style sheet unrelated to a block, just like exists today. And, given that, doing it by block bloats the code, and so for themes the better way is one style sheet to handle it all. (because neither the editor nor the theme knows what block the user will choose to add, so they all have to be there)

Terrible is a strong word and doesn't lend itself to good communication.

It served its purpose well for conveying my strong aversion to having to write code to load many styles instead of just one.

@nylen
Copy link
Member

nylen commented Jul 5, 2017

After investigating some more, I'm inclined to agree that each plugin which uses blocks should probably enqueue a single stylesheet and a single script. This is at least the simplest way to start, until we can see what else we need.

I feel a bit awkward about changing my mind here, but upon seeing the (relative) complexity of this PR, I think we should go with something more like #514 to start, and then add the minimum amount necessary of this API once we see what else is needed.

This week, our team is meeting up in person and working on one or more example plugins that will use this approach. I'll resurrect and implement something like #514 so that we can get started.

@BE-Webdesign
Copy link
Contributor Author

It served its purpose well for conveying my strong aversion to having to write code to load many styles instead of just one.

Fair enough 😄.

This week, our team is meeting up in person and working on one or more example plugins that will use this approach.

Should I try to refine this PR or will it most likely not be used at some point in the future?

@nylen
Copy link
Member

nylen commented Jul 25, 2017

Upon seeing this implemented I became convinced that we should go with something much simpler. I think the approach we took in #1717 will meet our needs pretty well, though I would like to hear about any cases where it doesn't work so well.

@gziolo
Copy link
Member

gziolo commented Sep 22, 2017

@BE-Webdesign, it seems like we came up with a similar approach with #1717. There was no action in this thread for 2 months now. I will close it, but feel free to reopen if you feel that we still should improve the way we handle assets. Thanks for your time spent on this PR 👍

@gziolo gziolo closed this Sep 22, 2017
@gziolo gziolo deleted the add/block-style-management-api branch September 22, 2017 14:34
youknowriad pushed a commit that referenced this pull request Jan 17, 2020
* Add ref to gutenberg repo

* Update ref

* Update ref

* Revert changes in gitmodules and update ref
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants