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

Change location of block support styles in <head> #39164

Merged

Conversation

ndiego
Copy link
Member

@ndiego ndiego commented Mar 2, 2022

Description

#38750 moved to block support styles to <head>, which was great. However these styles were loaded before block/global inline styles. This causes block support styles (which includes user styles set in the Editor) to be potentially overwritten by block/global inline styles.

In this PR, instead of using wp_enqueue_scripts, we use wp_head instead. This ensures that block support styles are loaded after the block/global inline styles in <head>.

There may be a better option than wp_head, but I have so far not been able to find one that works. Any insight would be greatly appreciated!

Screenshots

Here are some screenshots looking at the Site Title block. In theme.json, there is a link color applied. Before this change, this color overrides the custom orange color set by the user in the Editor. After the change, the user color takes precedent.

Before:
image

After:
image

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).
  • I've updated related schemas if appropriate.

@ndiego ndiego added the [Type] Bug An existing feature does not function as intended label Mar 2, 2022
@ndiego ndiego self-assigned this Mar 2, 2022
@cbravobernal cbravobernal added this to the Gutenberg 12.7 milestone Mar 2, 2022
@cbravobernal cbravobernal added the CSS Styling Related to editor and front end styles, CSS-specific issues. label Mar 2, 2022
@cbravobernal cbravobernal removed this from the Gutenberg 12.7 milestone Mar 2, 2022
@ndiego
Copy link
Member Author

ndiego commented Mar 2, 2022

Just a quick note for those reviewing. While this PR places the styles in the correct spot in <head> and I do think we need to get this updated asap, in testing, we stumbled upon other specificity issues related to global styles. I will continue investigating this and add a separate issue(s) as needed.

@ndiego
Copy link
Member Author

ndiego commented Mar 2, 2022

@c4rl0sbr4v0 I was just doing some more testing and discovered other issues that this PR fixes. Without this fix, block styles override blockGap, which should not happen. Here is an example using the Image block.

image

This can be tested using Twenty Twenty-Two:

  1. Add a new page with some text
  2. Add an Image block
  3. Notice that blockGap is not getting applies to the image on the Frontend.

Note that it still looks ok in the Editor because the styles are still loaded in the footer.

Copy link
Contributor

@alexstine alexstine left a comment

Choose a reason for hiding this comment

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

@ndiego Is there no option to maybe use $deps?

https://developer.wordpress.org/reference/functions/wp_enqueue_script/
https://developer.wordpress.org/reference/functions/wp_enqueue_style/

E.g. Must have XYZ included? I am not really understanding what the issue is so I am probably not the right person to be reviewing this.

@ndiego
Copy link
Member Author

ndiego commented Mar 2, 2022

Thanks for taking a look @alexstine. The issue is not about what is be loaded, more about where. All of these styles need to get added, just after all the inline block/global styles. In Gutenberg 12.6, the block support styles were loaded in the footer. #38750 fixed this by placing them in the <header>, but at the top. This PR just moves them down which mocks the Gutenberg 12.6 implementation.

Copy link
Contributor

@alexstine alexstine left a comment

Choose a reason for hiding this comment

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

Left another idea after checking the code.

@@ -63,7 +63,7 @@ function gutenberg_enqueue_global_styles_assets() {
function gutenberg_enqueue_block_support_styles( $style ) {
$action_hook_name = 'wp_footer';
if ( wp_is_block_theme() ) {
$action_hook_name = 'wp_enqueue_scripts';
Copy link
Contributor

Choose a reason for hiding this comment

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

@ndiego Why not add:

$action_hook_priority = 999;

Then the block support styles can load after the global styles as default priority on hooks is 10. If you are looking to do the reverse, you can make the priority 9 or lower as it will load before the priority of 10. Does this make sense?

@ndiego
Copy link
Member Author

ndiego commented Mar 2, 2022

@MaggieCabrera pinging you here to see if this fix resolves the issues you were having in #39014. I believe it does but would love your review if you have time.

Copy link
Contributor

@cbravobernal cbravobernal left a comment

Choose a reason for hiding this comment

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

The PR brings back visual consistency between 12.6 and 12.7. I tested and everything seems fine. We will still need to work on styles cascade :-)

Looks good to me!

@cbravobernal cbravobernal added this to the Gutenberg 12.7 milestone Mar 2, 2022
@cbravobernal cbravobernal merged commit 8df8fb2 into WordPress:trunk Mar 2, 2022
cbravobernal pushed a commit that referenced this pull request Mar 2, 2022
* Use wp_head instead of wp_enqueue_scripts

* Updates documentation
@youknowriad
Copy link
Contributor

Did the original PR made it to the WP minor @oandregal @jorgefilipecosta ? wondering whether we should flag this for potential next minor.

@MaggieCabrera
Copy link
Contributor

Thanks for bringing this in! +1 to flagging this for next minor

@youknowriad youknowriad added the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Mar 3, 2022
@ndiego ndiego deleted the fix/add-block-styles-end-of-head branch March 3, 2022 10:33
@Mamaduka Mamaduka removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Mar 29, 2022
Mamaduka pushed a commit that referenced this pull request Mar 29, 2022
* Use wp_head instead of wp_enqueue_scripts

* Updates documentation
pento pushed a commit to WordPress/wordpress-develop that referenced this pull request Mar 29, 2022
Backports changes from WordPress/gutenberg#39164.

Props ndiego, alexstine, cbravobernal, mamaduka.
See #55474.


git-svn-id: https://develop.svn.wordpress.org/trunk@53015 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Mar 29, 2022
Backports changes from WordPress/gutenberg#39164.

Props ndiego, alexstine, cbravobernal, mamaduka.
See #55474.

Built from https://develop.svn.wordpress.org/trunk@53015


git-svn-id: http://core.svn.wordpress.org/trunk@52604 1a063a9b-81f0-0310-95a4-ce76da25c4cd
gMagicScott pushed a commit to gMagicScott/core.wordpress-mirror that referenced this pull request Mar 29, 2022
Backports changes from WordPress/gutenberg#39164.

Props ndiego, alexstine, cbravobernal, mamaduka.
See #55474.

Built from https://develop.svn.wordpress.org/trunk@53015


git-svn-id: https://core.svn.wordpress.org/trunk@52604 1a063a9b-81f0-0310-95a4-ce76da25c4cd
pento pushed a commit to WordPress/wordpress-develop that referenced this pull request Mar 29, 2022
Backports changes from WordPress/gutenberg#39164.

Props ndiego, alexstine, cbravobernal, mamaduka.
Merges [53015] to the 5.9 branch.
See #55474.


git-svn-id: https://develop.svn.wordpress.org/branches/5.9@53017 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Mar 29, 2022
Backports changes from WordPress/gutenberg#39164.

Props ndiego, alexstine, cbravobernal, mamaduka.
Merges [53015] to the 5.9 branch.
See #55474.

Built from https://develop.svn.wordpress.org/branches/5.9@53017


git-svn-id: http://core.svn.wordpress.org/branches/5.9@52606 1a063a9b-81f0-0310-95a4-ce76da25c4cd
gMagicScott pushed a commit to gMagicScott/core.wordpress-mirror that referenced this pull request Mar 29, 2022
Backports changes from WordPress/gutenberg#39164.

Props ndiego, alexstine, cbravobernal, mamaduka.
Merges [53015] to the 5.9 branch.
See #55474.

Built from https://develop.svn.wordpress.org/branches/5.9@53017


git-svn-id: https://core.svn.wordpress.org/branches/5.9@52606 1a063a9b-81f0-0310-95a4-ce76da25c4cd
whereiscodedude pushed a commit to whereiscodedude/wpss that referenced this pull request Sep 18, 2022
Backports changes from WordPress/gutenberg#39164.

Props ndiego, alexstine, cbravobernal, mamaduka.
Merges [53015] to the 5.9 branch.
See #55474.

Built from https://develop.svn.wordpress.org/branches/5.9@53017
VenusPR added a commit to VenusPR/Wordpress_Richard that referenced this pull request Mar 9, 2023
Backports changes from WordPress/gutenberg#39164.

Props ndiego, alexstine, cbravobernal, mamaduka.
Merges [53015] to the 5.9 branch.
See #55474.

Built from https://develop.svn.wordpress.org/branches/5.9@53017


git-svn-id: http://core.svn.wordpress.org/branches/5.9@52606 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants