Apply the editor styles to the Site Editor page #20982

youknowriad · 2020-03-18T10:16:09Z

Refs #20791

This PR loads the editor styles in the Site Editor Page to match the frontend.

it extracts the EditorStyles application in the frontend to a reusable component
it copies the backend logic to prepare the editor styles to a reusable function.

Testing instructions

Try a "2019 blocks" from theme from the theme experiments repo https://github.com/WordPress/theme-experiments
The editor styles should load when you navigate to the site editor page.

github-actions · 2020-03-18T10:19:56Z

Size Change: -13 B (0%)

Total Size: 857 kB

Filename	Size	Change
`build/autop/index.js`	2.58 kB	+1 B
`build/block-directory/index.js`	6.02 kB	-1 B
`build/block-editor/index.js`	101 kB	+1.08 kB (1%)
`build/block-editor/style-rtl.css`	10.9 kB	+54 B (0%)
`build/block-editor/style.css`	10.9 kB	+56 B (0%)
`build/block-library/editor-rtl.css`	7.22 kB	-9 B (0%)
`build/block-library/editor.css`	7.23 kB	-8 B (0%)
`build/block-library/index.js`	110 kB	-1.26 kB (1%)
`build/block-library/style-rtl.css`	7.43 kB	+15 B (0%)
`build/block-library/style.css`	7.44 kB	+14 B (0%)
`build/block-serialization-default-parser/index.js`	1.65 kB	+1 B
`build/blocks/index.js`	57.5 kB	-23 B (0%)
`build/components/index.js`	191 kB	+81 B (0%)
`build/components/style-rtl.css`	15.8 kB	+9 B (0%)
`build/components/style.css`	15.7 kB	+9 B (0%)
`build/data/index.js`	8.25 kB	+55 B (0%)
`build/deprecated/index.js`	771 B	-1 B
`build/edit-post/index.js`	91.2 kB	-13 B (0%)
`build/edit-post/style-rtl.css`	8.43 kB	-89 B (1%)
`build/edit-post/style.css`	8.43 kB	-88 B (1%)
`build/edit-site/index.js`	6.73 kB	+1.67 kB (24%)	🚨
`build/edit-site/style-rtl.css`	2.91 kB	+380 B (13%)	⚠️
`build/edit-site/style.css`	2.9 kB	+376 B (12%)	⚠️
`build/edit-widgets/index.js`	4.43 kB	-1 B
`build/edit-widgets/style-rtl.css`	2.57 kB	-12 B (0%)
`build/edit-widgets/style.css`	2.57 kB	-12 B (0%)
`build/editor/editor-styles-rtl.css`	428 B	+47 B (10%)	⚠️
`build/editor/editor-styles.css`	431 B	+49 B (11%)	⚠️
`build/editor/index.js`	42.8 kB	-1.21 kB (2%)
`build/editor/style-rtl.css`	3.38 kB	-584 B (17%)	👏
`build/editor/style.css`	3.38 kB	-574 B (16%)	👏
`build/element/index.js`	4.44 kB	-11 B (0%)
`build/format-library/index.js`	6.95 kB	-143 B (2%)
`build/html-entities/index.js`	622 B	+1 B
`build/keycodes/index.js`	1.69 kB	+1 B
`build/list-reusable-blocks/index.js`	2.99 kB	-1 B
`build/media-utils/index.js`	4.84 kB	+3 B (0%)
`build/notices/index.js`	1.57 kB	-2 B (0%)
`build/nux/index.js`	3.01 kB	+1 B
`build/plugins/index.js`	2.54 kB	-1 B
`build/primitives/index.js`	1.5 kB	+2 B (0%)
`build/priority-queue/index.js`	781 B	+1 B
`build/redux-routine/index.js`	2.84 kB	-2 B (0%)
`build/rich-text/index.js`	14.5 kB	+130 B (0%)
`build/viewport/index.js`	1.61 kB	+1 B

ℹ️ View Unchanged

Filename	Size	Change
`build/a11y/index.js`	998 B	0 B
`build/annotations/index.js`	3.43 kB	0 B
`build/api-fetch/index.js`	3.39 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/style-rtl.css`	760 B	0 B
`build/block-directory/style.css`	760 B	0 B
`build/block-library/theme-rtl.css`	669 B	0 B
`build/block-library/theme.css`	671 B	0 B
`build/block-serialization-spec-parser/index.js`	3.1 kB	0 B
`build/compose/index.js`	6.21 kB	0 B
`build/core-data/index.js`	10.6 kB	0 B
`build/data-controls/index.js`	1.04 kB	0 B
`build/date/index.js`	5.37 kB	0 B
`build/dom-ready/index.js`	568 B	0 B
`build/dom/index.js`	3.06 kB	0 B
`build/escape-html/index.js`	733 B	0 B
`build/format-library/style-rtl.css`	502 B	0 B
`build/format-library/style.css`	502 B	0 B
`build/hooks/index.js`	1.93 kB	0 B
`build/i18n/index.js`	3.49 kB	0 B
`build/is-shallow-equal/index.js`	710 B	0 B
`build/keyboard-shortcuts/index.js`	2.3 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	226 B	0 B
`build/list-reusable-blocks/style.css`	226 B	0 B
`build/nux/style-rtl.css`	616 B	0 B
`build/nux/style.css`	613 B	0 B
`build/server-side-render/index.js`	2.55 kB	0 B
`build/shortcode/index.js`	1.7 kB	0 B
`build/token-list/index.js`	1.27 kB	0 B
`build/url/index.js`	4.01 kB	0 B
`build/warning/index.js`	1.14 kB	0 B
`build/wordcount/index.js`	1.18 kB	0 B

_{compressed-size-action}

youknowriad · 2020-03-24T08:20:47Z

This could use some reviews @vindl @Addison-Stavlo @johnstonphilip

Addison-Stavlo

Testing with the experimental 2019 blocks theme:

Front End:

Editor (pre-change):

Editor (post-change):

So the site editor now does display the styles in a way that is much more representative of the front-end (although not yet completely 1-1).

It looks like the Site Title block encounters an error with this change, is that expected at this point?

johnstonphilip · 2020-03-24T20:07:21Z

@youknowriad They are loading properly for me! Code looks good to me as well.

@Addison-Stavlo The Site Title Block is working for me on this branch.

johnstonphilip · 2020-03-24T20:14:29Z

Testing with an actual style works for me as well

Addison-Stavlo · 2020-03-24T20:35:40Z

@Addison-Stavlo The Site Title Block is working for me on this branch.

Great! It works for me on master and as soon as i switch to using this branch I get warnings/errors stemming from SiteTitleEdit - React.createElement: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: undefined. 🤔 I wonder why that problem is branch specific on my end.

Either way, its good to hear these changes work on your end without causing any sort of issue like that.

johnstonphilip · 2020-03-24T21:10:15Z

@Addison-Stavlo That is super strange! The closest related issue I could find is this super old one so not sure if helpful:
#944

I wonder if it could even be that webpack had a hiccup or something?

youknowriad · 2020-03-25T08:57:58Z

React.createElement: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: undefined.

This mean we're trying to render undefined components. Looking at the SiteTitle block, it seems it was updated recently, so maybe just a branch switching workflow issue?

lib/edit-site-page.php

Co-Authored-By: Andrés <nosolosw@users.noreply.github.com>

oandregal

This works nicely on practice and I very much welcome it to unblock #20530 👏 However, I'd like to get some eyes on a failing test before approving.

I can repro locally by:

npm run test-unit:native packages/edit-post/src/test/editor.native.js

It looks like ReactNative doesn't welcome the addition of the EditorStyles component within the EditorProvider. The message is a bit confusing because it suggests it can't find it but I've traced the export/import statements and they seem fine to me.

Perhaps @gziolo knows more?

gziolo · 2020-03-25T14:59:05Z

Perhaps @gziolo knows more?

I know who could help to find someone who might know, @Tug or @hypest 😅

It should be fixed now.

oandregal

Ran the test locally and can confirm that it passes. The changes at 82c296c make sense now that I see that there's a separate export for native.

aduth · 2020-03-26T17:56:07Z

It's not immediately clear if or why it would be related, but I'm observing that every single commit to master since this (including this commit) have failed end-to-end tests; all but one with the following error:

FAIL packages/e2e-tests/specs/editor/various/sidebar.test.js (16.039s)
  Sidebar
  ● Sidebar › should reopen sidebar the sidebar when resizing from mobile to desktop if the sidebar was closed automatically
    expect(received).toHaveLength(expected)
    Expected length: 1
    Received length: 0
    Received array:  []
      89 | 
      90 | 		const sidebarsDesktop = await page.$$( SIDEBAR_SELECTOR );
    > 91 | 		expect( sidebarsDesktop ).toHaveLength( 1 );
         | 		                          ^
      92 | 	} );
      93 | 
      94 | 	it( 'should preserve tab order while changing active tab', async () => {
      at Object.toHaveLength (specs/editor/various/sidebar.test.js:91:29)
          at runMicrotasks (<anonymous>)

I've never seen this failure before, nor do any of the previous commits (including failures) have this error.

See: #21177

aduth · 2020-03-26T18:03:38Z

packages/block-editor/src/components/editor-styles/index.js

+ */
+import transformStyles from '../../utils/transform-styles';
+
+function EditorStyles( { styles } ) {


Was there any consideration to make this a hook, considering its own behaviors are hooks? And observing that the current implementation forced refactoring of EditorProvider to render a fragment to support the additional top-level component.

I thought about it, the main reason I didn't do it is because block-editor don't expose hooks but expose similar behavior components like "ObserveTyping", "WritingFlow". I guess even this behaviors could be hooks.

Now, that I think about it more, maybe a hook is better though.

Yeah, I was thinking about these "behavioral" components as well. They made sense at the time. Nowadays, I feel that hooks are a perfect fit for how and why we were implementing those.

There's still an argument that could be made of "consistency for consistency's sake", so I could be okay with either approach.

aduth · 2020-03-26T18:04:26Z

packages/editor/src/components/provider/index.js

-		if ( ! this.props.settings.styles ) {
-			return;
-		}


Where did this condition go in the new implementation? Was it not needed?

Not needed cause we map through the styles but it can be micro-optim

aduth · 2020-03-26T18:07:41Z

packages/block-editor/src/components/editor-styles/index.js

+		return () =>
+			nodes.forEach( ( node ) => document.body.removeChild( node ) );


Related to #20982 (comment), I observe this unmounting behavior is "new", though again not entirely clear what impact (if any) it would have.

One worry I might have is that since this unsubscribe behavior is called if any dependencies change, are we sure that styles will remain stable? Aside from my current debugging of test failures, seems like it might be a performance concern if the references change with any frequency.

are we sure that styles will remain stable

I think so since these come from the server. It would surprise if this is the reason for the failure.

The reason i added the unmount behavior is because it's the right thing to do if the styles changes, we need to recompute and reload them.

youknowriad · 2020-03-26T19:54:37Z

I'm not sure why this would fail these tests, but I'll try to debug tomorrow.

aduth · 2020-03-26T19:57:43Z

@youknowriad It's fixed as of #21180. It may still be worth debugging, but the pressing issue (failing builds) is sorted for now.

youknowriad · 2020-03-26T20:17:37Z

useEffect is async while componentDidMount is sync. that's the biggest suspicious thing for me. That said useEffect is recommended by the React team instead of the sync useLayoutEffect.

hypest · 2020-03-31T14:18:41Z

I know who could help to find someone who might know, @Tug or @hypest 😅

Sorry, too late to the party. Noticed that Riad had already fixed the failing mobile tests with 82c296c 🎉

Apply the editor styles to the Site Editor page

254e418

youknowriad requested review from ellatrix and TimothyBJacobs as code owners March 18, 2020 10:16

youknowriad self-assigned this Mar 18, 2020

youknowriad requested a review from mtias March 18, 2020 10:17

youknowriad added the [Feature] Full Site Editing label Mar 18, 2020

youknowriad mentioned this pull request Mar 18, 2020

Full Site Editing Breakdown #20791

Closed

53 tasks

youknowriad requested a review from vindl March 24, 2020 08:20

Addison-Stavlo reviewed Mar 24, 2020

View reviewed changes

oandregal reviewed Mar 25, 2020

View reviewed changes

lib/edit-site-page.php Outdated Show resolved Hide resolved

Fix tests

6d4ff60

Co-Authored-By: Andrés <nosolosw@users.noreply.github.com>

oandregal previously requested changes Mar 25, 2020

View reviewed changes

Fix mobile tests

82c296c

oandregal approved these changes Mar 26, 2020

View reviewed changes

youknowriad merged commit a061e7f into master Mar 26, 2020

youknowriad deleted the add/edit-site-editor-styles branch March 26, 2020 10:42

github-actions bot added this to the Gutenberg 7.9 milestone Mar 26, 2020

aduth mentioned this pull request Mar 26, 2020

E2E Tests: Intermittent failure in sidebar resizing test #21177

Closed

aduth reviewed Mar 26, 2020

View reviewed changes

ellatrix mentioned this pull request Jun 16, 2020

WP 5.5: merge all PHP changes #23210

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply the editor styles to the Site Editor page #20982

Apply the editor styles to the Site Editor page #20982

youknowriad commented Mar 18, 2020 •

edited

Loading

github-actions bot commented Mar 18, 2020 •

edited

Loading

youknowriad commented Mar 24, 2020

Addison-Stavlo left a comment •

edited

Loading

johnstonphilip commented Mar 24, 2020

johnstonphilip commented Mar 24, 2020

Addison-Stavlo commented Mar 24, 2020

johnstonphilip commented Mar 24, 2020

youknowriad commented Mar 25, 2020

oandregal left a comment •

edited

Loading

gziolo commented Mar 25, 2020

oandregal left a comment

aduth commented Mar 26, 2020 •

edited

Loading

aduth Mar 26, 2020

youknowriad Mar 26, 2020 •

edited

Loading

aduth Mar 26, 2020 •

edited

Loading

aduth Mar 26, 2020

youknowriad Mar 26, 2020

aduth Mar 26, 2020

youknowriad Mar 26, 2020

youknowriad commented Mar 26, 2020

aduth commented Mar 26, 2020

youknowriad commented Mar 26, 2020

hypest commented Mar 31, 2020

		return () =>
		nodes.forEach( ( node ) => document.body.removeChild( node ) );

Apply the editor styles to the Site Editor page #20982

Apply the editor styles to the Site Editor page #20982

Conversation

youknowriad commented Mar 18, 2020 • edited Loading

github-actions bot commented Mar 18, 2020 • edited Loading

youknowriad commented Mar 24, 2020

Addison-Stavlo left a comment • edited Loading

Choose a reason for hiding this comment

johnstonphilip commented Mar 24, 2020

johnstonphilip commented Mar 24, 2020

Addison-Stavlo commented Mar 24, 2020

johnstonphilip commented Mar 24, 2020

youknowriad commented Mar 25, 2020

oandregal left a comment • edited Loading

Choose a reason for hiding this comment

gziolo commented Mar 25, 2020

oandregal left a comment

Choose a reason for hiding this comment

aduth commented Mar 26, 2020 • edited Loading

aduth Mar 26, 2020

Choose a reason for hiding this comment

youknowriad Mar 26, 2020 • edited Loading

Choose a reason for hiding this comment

aduth Mar 26, 2020 • edited Loading

Choose a reason for hiding this comment

aduth Mar 26, 2020

Choose a reason for hiding this comment

youknowriad Mar 26, 2020

Choose a reason for hiding this comment

aduth Mar 26, 2020

Choose a reason for hiding this comment

youknowriad Mar 26, 2020

Choose a reason for hiding this comment

youknowriad commented Mar 26, 2020

aduth commented Mar 26, 2020

youknowriad commented Mar 26, 2020

hypest commented Mar 31, 2020

youknowriad commented Mar 18, 2020 •

edited

Loading

github-actions bot commented Mar 18, 2020 •

edited

Loading

Addison-Stavlo left a comment •

edited

Loading

oandregal left a comment •

edited

Loading

aduth commented Mar 26, 2020 •

edited

Loading

youknowriad Mar 26, 2020 •

edited

Loading

aduth Mar 26, 2020 •

edited

Loading