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

Gallery block: update the gallery gap to load styles in header for block themes #41015

Merged
merged 8 commits into from
May 30, 2022

Conversation

glendaviesnz
Copy link
Contributor

@glendaviesnz glendaviesnz commented May 12, 2022

What?

Moves the custom gallery gap styles to the header if the theme being used is a block theme.

Why?

Currently they are always loaded in the footer, which can cause a flash of unstyled content.

Fixes: #40936

How?

Update the gallery render to use the gutenberg_enqueue_block_support_styles method so styles get put in header for block themes

Testing Instructions

  • In a site with a block theme add a Gallery block and set the block gap
  • In the frontend make sure the .wp-block-gallery-{id} inline style is loaded near end of header and the gallery shows the correct gap
  • Switch the site to a non block theme and reload the same gallery in the frontend and check style is now near bottom of page body and that the gap is still applied.

Screenshots or screencast

Before:
gap-before

After:
Block theme
gap-after

Non block theme
gap-before

@glendaviesnz glendaviesnz added the [Block] Gallery Affects the Gallery Block - used to display groups of images label May 12, 2022
@glendaviesnz glendaviesnz self-assigned this May 12, 2022
@glendaviesnz glendaviesnz marked this pull request as draft May 12, 2022 05:18
@glendaviesnz
Copy link
Contributor Author

Just need to work out exactly where the method fits in terms of compat folders before this is ready for review

@ramonjd
Copy link
Member

ramonjd commented May 20, 2022

This is testing well for me so far, though I can't see a difference between trunk and this PR for block themes. Tried network throttling and a couple of browsers.

Here's me refreshing like it's 1999:

2022-05-20 10 23 12

Moves the custom gallery gap styles to the header if the theme being used is a block theme.

Can confirm that classic theme gap styles remain in the footer while block theme gap styles are in the header.

@glendaviesnz
Copy link
Contributor Author

This is testing well for me so far, though I can't see a difference between trunk and this PR for block themes.

Yeh, I wasn't actually able to replicate the FOUC, but figured it made better sense to have the styles in the header when possible regardless.

I have rebased it to pull in the changes to the row/column gap, so could do with another test when you have a moment please.

@ramonjd ramonjd force-pushed the update/gallery-gap-move-to-header branch from b005f27 to ae6b7cd Compare May 24, 2022 00:10
Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM.

@markhowellsmead
Copy link

In WordPress 6.0 without the plugin, the gap on a gallery block is a fixed size in the generated inline CSS: gap: 0.5em instead of a custom CSS property.

@glendaviesnz
Copy link
Contributor Author

In WordPress 6.0 without the plugin, the gap on a gallery block is a fixed size in the generated inline CSS: gap: 0.5em instead of a custom CSS property.

Thanks for reporting @markhowellsmead. There is a potential fix for this here which we can hopefully get into 6.0.1. In the meantime the following custom CSS should fix the issue:

.wp-block-gallery.has-nested-images {
	gap: var(--wp--style--gallery-gap-default)
}

@glendaviesnz
Copy link
Contributor Author

This shouldnt be merged until after #41423

@markhowellsmead
Copy link

Thanks @glendaviesnz, that works fine. The same applies to the columns block; does that need a separate ticket?

@glendaviesnz glendaviesnz force-pushed the update/gallery-gap-move-to-header branch from 9ecb837 to 5d66c81 Compare May 30, 2022 21:43
lib/load.php Show resolved Hide resolved
@glendaviesnz
Copy link
Contributor Author

glendaviesnz commented May 30, 2022

Thanks @glendaviesnz, that works fine. The same applies to the columns block; does that need a separate ticket?

I see you added an issue for this here #41431 - currently the columns block does not have the option to set a default gap via a css var like the gallery does, so a matter of the functionality not being there, rather than a css load error. There is an open PR here that adds something similar for the sake of fixing the mobile stacking options. Have linked your issue to that so it can be considered as part of that work.

@ramonjd
Copy link
Member

ramonjd commented May 30, 2022

Thanks @glendaviesnz

I ran through the test instructions, using the site editor and block editor with multiple galleries.
The block gap value is applied as expected in block themes, and then for classic themes I tested (2020, 2014, 2019)

@glendaviesnz glendaviesnz merged commit fbfff91 into trunk May 30, 2022
@glendaviesnz glendaviesnz deleted the update/gallery-gap-move-to-header branch May 30, 2022 23:43
@github-actions github-actions bot added this to the Gutenberg 13.4 milestone May 30, 2022
ramonjd added a commit that referenced this pull request May 31, 2022
youknowriad pushed a commit that referenced this pull request Jun 3, 2022
…ock themes (#41015)

Co-authored-by: Glen Davies <glen.davies@a8c.com>
Co-authored-by: ramonjd <ramonjd@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Gallery Affects the Gallery Block - used to display groups of images
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gallery block: the CSS generated for the block spacing is printed at the end of the page unlike other blocks
3 participants