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

Revert "Enqueue registered assets once." #4498

Closed
wants to merge 2 commits into from
Closed

Revert "Enqueue registered assets once." #4498

wants to merge 2 commits into from

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented May 24, 2023

Trac ticket https://core.trac.wordpress.org/ticket/58208
Reverts changeset https://core.trac.wordpress.org/changeset/55695

What

This PR reverts PR #4356

Why

It caused issues in the ordering of styles in the editor, which went unnoticed in the testing. The reason is that block styles (editor-only and front styles) would now be enqueued using the enqueue_block_assets which runs later than the enqueue_block_editor_assets hook.

The order of styles enqueued before the change was:

  1. front styles registered for blocks, via styles handle (block.json)
  2. editor styles registered for blocks, via editorStyles handle (block.json)
  3. editor styles enqueued via enqueue_block_editor_assets hook
  4. front styles enqueued via enqueue_block_assets hook

The order of styles enqueued after the change was:

  1. editor styles enqueued via enqueue_block_editor_assets hook
  2. front styles registered for blocks, via styles handle (block.json)
  3. editor styles registered for blocks, via editorStyles handle (block.json)
  4. front styles enqueued via enqueue_block_assets hook

How to test

  • Use this PR.
  • Install and activate the test plugin iframed-inline-styles.zip.
  • Go to the editor and add the block "iframed-inline-styles".
  • The expected result is that padding is 20px and border width is 2px:

image

SVN Commit Message

Loading assets for the editor: Revert [55695].

The changeset [55695] introduced a regression in the order of styles for the editor, causing the styles registered for the block (both editor and front) to be loaded after any other styles enqueued using the `enqueue_block_editor_assets` hook.

Since the original behavior was introduced in WordPress 5.0 changing it breaks the expectations of the ecosystem.

Props ellatrix.
Fixes #58208.

@oandregal oandregal requested a review from ellatrix May 24, 2023 07:56
@oandregal oandregal self-assigned this May 24, 2023
@oandregal oandregal added the bug label May 24, 2023
@oandregal
Copy link
Member Author

I considered an alternative: enqueue the block styles for the editor using the enqueue_block_editor_assets and the block styles for the front using the enqueue_block_assets. This would result in the block styles being loaded before the corresponding styles of the hook:

  1. editor styles registered for blocks, via editorStyles handle (block.json)
  2. editor styles enqueued via enqueue_block_editor_assets hook
  3. front styles registered for blocks, via styles handle (block.json)
  4. front styles enqueued via enqueue_block_assets hook

This may seem the correct behavior. However, the enqueue_block_editor_assets hook has been used by themes to override block styles for the editor (see hook in use and styles for TwentyTwenty, for example). Using this approach the styles of the theme would load 2nd, having to use greater specificity to override the front styles of the blocks that load 3rd.

Copy link
Member

@ellatrix ellatrix left a comment

Choose a reason for hiding this comment

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

Sounds good!

@oandregal
Copy link
Member Author

@oandregal oandregal closed this May 24, 2023
@oandregal oandregal deleted the revert/enqueue-registered-assets-once branch May 24, 2023 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants