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

[core/block-editor] Breaking Change introduced in getSettings (styles property removed) #32966

Closed
subhaze opened this issue Jun 24, 2021 · 11 comments
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability Good First Issue An issue that's suitable for someone looking to contribute for the first time

Comments

@subhaze
Copy link

subhaze commented Jun 24, 2021

Description

It appears that styles was removed from the core/block-editor settings, however, there was no deprecation warning or documentation update (docs and type definitions still state that settings should exist).

Line of code that appears to remove it. https://github.com/WordPress/gutenberg/blame/4834fad30e39e47d71a93e2e08d31b3b856711e1/packages/edit-post/src/editor.js#L112

Expected behaviour

When using useSelect() to select core/block-editor and retrieve styles from getSettings() enqueued styles should be there or an empty Array.

Actual behaviour

When using useSelect() to select core/block-editor the styles property has been completely removed.

Code snippet (optional)

const { getSettings } = useSelect(
	(select) => select("core/block-editor"),
	[]
);
const styles = getSettings().styles;
console.log(styles); // undefined

WordPress information

  • WordPress version: 5.7.2
  • Gutenberg version: using version released on WP 5.7.2
  • Are all plugins except Gutenberg deactivated? No
  • Are you using a default theme (e.g. Twenty Twenty-One)? No

Device information

  • Device: Desktop
  • Operating system: macOS
  • Browser: Microsoft Edge 91
@talldan
Copy link
Contributor

talldan commented Jun 28, 2021

@youknowriad @aristath — looks like this is from #30034.

@youknowriad
Copy link
Contributor

you can access that property by doing wp.data.select( 'core/editor' ).getEditorSettings().styles I think.
The PR you're mentioning is not the one that removed that styles, you can see that the "omit" was still there before the PR. I think this is a lot older.

@subhaze
Copy link
Author

subhaze commented Jun 28, 2021

you can access that property by doing wp.data.select( 'core/editor' ).getEditorSettings().styles I think.

I believe you're correct if not on WP 5.7, but that store appears to be effected by this as well; Even then, it should be added back to core/block-editor as this is still a breaking change that occurred in 5.7 considering it's part of the public API for that package https://developer.wordpress.org/block-editor/reference-guides/packages/packages-block-editor/#SETTINGS_DEFAULTS

The PR you're mentioning is not the one that removed that styles

Appologies, the link shared pointed to the wrong timeline with PR that removed it. Following https://github.com/WordPress/gutenberg/blame/33244eef0b7dfcda73883cb9225293aa7525616d/packages/edit-post/src/editor.js#L123 should lead you to the correct commit that began removing 'styles' for getSettings #27947.

I think this is a lot older.

While the commit that removed it is older than the initial commit linked, it's not by much, and appears to still point to it being introduced in the lastest release of WP (5.7.x)

@youknowriad
Copy link
Contributor

👍 Thanks for the exploration, that commit looks right yes.

I think the styles property doesn't make sense in the block-editor setting though because it's not something that apply to all block-editor instances by default. In other words, the block-editor package doesn't need it. That said, I think it's ok to bring it back for some times and mark it deprecated.

@youknowriad youknowriad added Good First Issue An issue that's suitable for someone looking to contribute for the first time Backwards Compatibility Issues or PRs that impact backwards compatability labels Jun 28, 2021
@subhaze
Copy link
Author

subhaze commented Jun 28, 2021

I'm curious why you feel it shouldn't belong there even if it's not always used; It could return as an empty array if not used for that instance?

I'm curious of this because if it's deprecated in block-editor getSettings, is the plan to also update (or introduce as new type) the settings object type that's returned? Since it appears this object type (EditorSettings) is used in a few different areas that would need styles, even if not always used; Such as edit-post when it gets passed an EditorSettings Obj with styles during initialization.

* @param {?Object} settings Editor settings object.

@youknowriad
Copy link
Contributor

Gutenberg is a module architecture, each package has a defined purpose:

  • block-editor is a package that allows developers to build block editors, these include the post editor, widget editor, navigation editor or any block editor you can think of, even outside wordpress.
  • edit-post is the implementation of the block editor to edit posts in WordPress.
  • "styles" setting is a WP specific setting used to apply editor styles, it's only needed in the WP context, that's why it doesn't seem right for the block-editor to have such setting. Potentially, it could make sense in the future if the generic block-editor itself consumed that config and for instance adds the styles to the block list canvas automatically, that's not how it work at the moment.

@subhaze
Copy link
Author

subhaze commented Jun 28, 2021

Thanks for additional context; And glad to see this being looked at as larger than WP.

So it seems like it would be more beneficial to adjust the Type Defs for EditorSettings and have the WP side of Gutenberg extend that, adding styles as a prop. Then potentially move styles over to the core/edit-post store since it does use it during initialization (from what I can tell while reading through code) and appears to be WP post exclusive?

@t-hamano
Copy link
Contributor

t-hamano commented Sep 20, 2021

Nice to meet you.
Related to this issue, the CSS added by add_editor_styles is not being applied to the Custom HTML Block when previewing.

The reason is that the styles property has been removed in getSettings and getEditorSettings, and empty styles are passed to the SandBox component.

https://github.com/WordPress/gutenberg/blob/wp/5.8/packages/block-library/src/html/edit.js#L39

Is this state in Custom HTML Block correct?
If not, I hope it will be fixed so that the set styles will be reflected in the preview, as it was before.

I considered submitting a pull request myself, but I apologize that I couldn't find a function to get styles.

@t-hamano
Copy link
Contributor

I confirmed that the styles were returned in #28165.
Please ignore my comments.

@ugljanin
Copy link
Contributor

@t-hamano I am facing the same issue as you described and my styles are not passing from https://github.com/WordPress/gutenberg/blob/wp/5.8/packages/block-library/src/html/edit.js#L39

Could you please explain how is this solved?

@ugljanin
Copy link
Contributor

Update: This is fixed with WP 5.9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability Good First Issue An issue that's suitable for someone looking to contribute for the first time
Projects
None yet
Development

No branches or pull requests

5 participants