WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.49k stars 4.19k forks source link

Site Editor: restore block-library editor.css outside canvas #66556

Closed ellatrix closed 2 days ago

ellatrix commented 3 days ago

What?

In #66544 @t-hamano observed that a bit too many styles have been removed in https://github.com/WordPress/gutenberg/pull/66432. Indeed, it should have kept loading editor.css files defined by blocks because they target both content and UI outside the canvas. The remaining styles should still be removed though.

One light problem is that all the styles we want to remove are added as dependencies of wp-edit-blocks. So restoring wp-edit-blocks will add back all the styles we don't want. To fix the issue we can register a separate handle wp-block-library-editor, which exclusively loads the editor styles.

Why?

Fix a regression from #66432.

How?

Testing Instructions

In content only mode, select an image and click the "alternative text" button in the toolbar.

Testing Instructions for Keyboard

Screenshots or screencast

github-actions[bot] commented 3 days ago

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

t-hamano commented 3 days ago

This PR works for me.

My only concern is that deleting wp-edit-blocks or changing the handle name would be a breaking change. For example, block developers might apply their own styles to block sidebars, etc. with code like this:

wp_enqueue_style(
    'my-block-common',
    'path/to/block-common.css'
    array( 'wp-edit-blocks' )
);

I searched the WP Directory and it seems that a lot of plugins enqueue styles that depend on the wp-edit-blocks handle.

https://wpdirectory.net/search/01JBC821K2ZBFBMPF7QQSP7W8K

ellatrix commented 3 days ago

@t-hamano

  1. Using wp-edit-blocks will still work. It's used in the canvas. So if you add dependencies to wp-edit-blocks, it should still work.
  2. Blocks should never add wp-edit-blocks as a dependency. It should be the opposite: wp-edit-blocks should contain all the blocks' style sheets as dependencies.
ellatrix commented 3 days ago

What I mean is: nothing about wp-edit-blocks changes. It's all still there.

t-hamano commented 3 days ago

Using wp-edit-blocks will still work. It's used in the canvas.

If developers are adding a wp-edit-blocks handle to a dependency to enqueue a style outside of the canvas, the style won't be loaded outside of the canvas (root document), right?

Relying on wp-edit-blocks to apply styles outside of the canvas may not be a recommended use case, but I thought some dev notes might be needed.

ellatrix commented 3 days ago

@t-hamano I'm not sure I understand. Do you mean:

Or:

t-hamano commented 3 days ago

Sorry, maybe I misunderstood. I don't think this PR is a breaking change. Let me explain a bit more.

Is my understanding correct?

Also, if we go ahead with this PR, do we also need a backport PR to core?

ellatrix commented 3 days ago

@t-hamano Ah, in that case, this PR will not make any difference? Because merely adding wp-edit-blocks as a dependency does not cause it to load wherever wp-edit-blocks is enqueued? In other words removing wp-edit-blocks outside the canvas does not change anything for this code.

Yes, I'll add it to https://github.com/WordPress/wordpress-develop/pull/7643 :)

ellatrix commented 3 days ago

In fact, it will actually undo the removal of this PR because the action is called outside the canvas: https://github.com/WordPress/wordpress-develop/blob/695476ea5e51d1c92f389ac236b138ceeaee8a99/src/wp-admin/site-editor.php#L177

github-actions[bot] commented 3 days ago

Flaky tests detected in 4c7d42ea3e08af2369d482a28fd46c1d0788fb9a. Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11578911527 📝 Reported issues:

t-hamano commented 2 days ago

BTW, should this PR be backported to Gutenberg 19.6? It looks like 19.6 RC1 is just about ready.

ellatrix commented 2 days ago

@t-hamano Yes, I'll update the core PR. Thank you for the review!

ellatrix commented 2 days ago

BTW, should this PR be backported to Gutenberg 19.6? It looks like 19.6 RC1 is just about ready.

I'll check with the person doing the release.

cbravobernal commented 2 days ago

I just cherry-picked this PR to the release/19.6 branch to get it included in the next release: 37368403cf

Mamaduka commented 2 days ago

I think this introduced the following warning in the editors.

Screenshot

CleanShot 2024-10-31 at 09 55 21

ellatrix commented 2 days ago

@Mamaduka Ah, will fix

ellatrix commented 2 days ago

@Mamaduka Fixed in #66628.