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

Fix: Conditionaly apply Editor Skeleton html element styles #20245


Copy link

jorgefilipecosta commented Feb 14, 2020

Previously editor skeleton applied styles to the HTML element unconditionally.
This PR makes sure the styles are only applied if the component is mounted.
Related: #20214 (comment)

It is important to get this in 5.4 because many blocks may have a bug where block editor styles are being enqueued without need and without this change we may be breaking the mobile experience for users with one of these blocks installed.

How has this been tested?

I verified the editor still works as expected on mobile.
I verified the styles for the new class are being loaded.

@@ -17,6 +32,7 @@ function EditorSkeleton( {
} ) {
useHTMLClass( 'block-editor-editor-skeleton-html-container' );

This comment has been minimized.

Copy link

youknowriad Feb 14, 2020


Maybe more something like block-editor-editor-skeleton__html-container

Copy link

youknowriad left a comment

I didn't test but this seems like a good change to me.

@jorgefilipecosta jorgefilipecosta force-pushed the fix/only-apply-html-styles-in-editor-sketelon-when-the-component-is-rendered branch from 6e19443 to b9bbeeb Feb 14, 2020
@jorgefilipecosta jorgefilipecosta merged commit ecd3b45 into master Feb 14, 2020
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
WordPress 5.4 Must Have automation moved this from Needs Review to Done Feb 14, 2020
@jorgefilipecosta jorgefilipecosta deleted the fix/only-apply-html-styles-in-editor-sketelon-when-the-component-is-rendered branch Feb 14, 2020
@github-actions github-actions bot added this to the Gutenberg 7.6 milestone Feb 14, 2020
function useHTMLClass( className ) {
useEffect( () => {
const element =
document && document.querySelector( `html:not(.${ className }` );

This comment has been minimized.

Copy link

aduth Feb 27, 2020



Suggested change
document && document.querySelector( `html:not(.${ className }` );
document && document.querySelector( `html:not(.${ className })` );

Causing: #20504

Edit: Fix: #20507

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.