FSE: Fix missing inserter #44357

sirreal · 2020-07-22T17:26:52Z

Changes proposed in this Pull Request

Add a MutationObserver to re-inject a React component on DOM changes if the component is not already present.

This is a workaround for #43784, which observed that the component is missing. The component is injected into a React tree, which is volatile. Injected components like this will disappear if a parent is re-rendered.

This also adds a class to fix the block inserter from being removed from the post editor. It should only be removed from the page editor.

Testing instructions

Use a dotcom-fse enabled site for testing.

Try to reproduce Missing Block Inserter in dotcom FSE w/ Gutenberg 8.4 #43784 on a Simple site without Gutenberg edge.
Try to reproduce Missing Block Inserter in dotcom FSE w/ Gutenberg 8.4 #43784 on a Simple site with Gutenberg edge.
Visit the post editor and ensure you can see the standard block inserter (Missing Block Inserter in dotcom FSE w/ Gutenberg 8.4 #43784 (comment)).

Fixes #43784

matticbot · 2020-07-22T17:26:54Z

Test live: https://calypso.live/?branch=fix/43784-fse-missing-inserter

matticbot · 2020-07-22T17:36:35Z

Caution: This PR affects files in the FSE Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D46774-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing in the FSE Plugin for more info: PCYsg-ly5-p2

matticbot · 2020-07-22T17:41:12Z

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

fullofcaffeine · 2020-07-22T17:53:12Z

apps/full-site-editing/full-site-editing-plugin/dotcom-fse/editor/block-inserter/index.js

+		// Re-inject the FSE inserter as needed in case React re-renders the header
+		const wpbody = document.getElementById( 'wpbody' );
+		if ( wpbody && typeof window.MutationObserver !== 'undefined' ) {
+			const observer = new window.MutationObserver( injectBlockInserter );


Clever workaround! I learned something new today, didn't know about the MutationObserver API.

Clever workaround!

Agreed! I didn't know about the MutationObserver either, this is very interesting and great to see. Nice work here. 😄

fullofcaffeine · 2020-07-22T17:58:37Z

This also adds a class to fix the block inserter from being removed from the post editor. It should only be removed from the page editor.

Why should it not be present on the page editor?

sirreal · 2020-07-22T18:42:23Z

Why should it not be present on the page editor?

It's hidden on the page editor so that it can be replaced with a custom inserter. That's part of the dotcom-fse functionality.

I don't know many details about why 🙂

fullofcaffeine · 2020-07-22T19:08:35Z

Works great ✔️

Tested from a fresh FSE-dotcom site mapped to a clear sandbox + the gutenberg-edge sticker applied (8.5.1). Could reproduce Missing Block Inserter in dotcom FSE w/ Gutenberg 8.4 #43784;
Checked out this branch and synced the files over to the sandbox using FSE's yarn dev --sync;
Verified that the block inserter button in the toolbar was visible and functional in both the FSE/page editor and the post editor.

fullofcaffeine · 2020-07-22T19:11:59Z

Why should it not be present on the page editor?

It's hidden on the page editor so that it can be replaced with a custom inserter. That's part of the dotcom-fse functionality.

I don't know many details about why slightly_smiling_face

Interesting. Not critical or anything, but I'd love to know why a custom inserter is needed there and what it does differently cc @noahtallen

noahtallen · 2020-07-22T19:19:44Z

I don't know many details about why

If I remember correctly:

It prevents blocks from being inserted outside of the post content block. In dotcom FSE, you are not able to edit the underlying template: it consists of just "header, post content, footer". You can edit each of those three blocks, but you are not able to modify the order or anything about that root template. This is one of the technical limitations for dotcom FSE which is resolved by core FSE. Basically, the "template" is hardcoded in PHP, which means that you should not be able to insert it it in the editor. Expanding further beyond that, we were basically hacking the normal page editor to render multiple entities, so the default behavior would be to edit anything on the page, which obviously doesn't work if parts of the page must always be hardcoded to the same thing.
It might also enable FSE blocks to show up only in the FSE editor.

Addison-Stavlo

Testing this on my FSE site and it looks good!

I didn't realize that the inserter was also being hidden in the post editor but only re-added in the page editor. 🤦‍♀️ So the inserter must have been missing from the post editor for quite some time?

Either way, with this PR the inserter is back in both editors and working well on both (edge and non-edge) Gutenberg versions.

Great job, and thank you for fixing this!

Although a separate issue, it's worth noting that the inserter is different between the post and page editors. Since the post editor is not re-implementing the inserter we have the standard sidebar style inserter with the patterns tab. While in the page editor our re-implemented version is the dropdown inserter for blocks only. This is expected with the current implementation and is in the prioritized backlog of the gardening board (#41654).

sirreal · 2020-07-23T07:52:47Z

Thanks, folks!

So the inserter must have been missing from the post editor for quite some time?

That seems to be the case. I discovered it when I started looking at #43784.

sirreal · 2020-07-23T07:55:41Z

Changes to /apps should have no impact on the Calypso e2e tests run against this branch. I'll merge in spite of those e2e test failures.

sirreal added 2 commits July 22, 2020 19:23

Remove inserter only on pages

c400a70

Observe mutations and re-inject the inserter

5950222

sirreal mentioned this pull request Jul 22, 2020

Missing Block Inserter in dotcom FSE w/ Gutenberg 8.4 #43784

Closed

sirreal added [Goal] Full Site Editing [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Type] Bug When a feature is broken and / or not performing as intended labels Jul 22, 2020

sirreal requested review from Addison-Stavlo and a team July 22, 2020 17:28

sirreal self-assigned this Jul 22, 2020

sirreal marked this pull request as ready for review July 22, 2020 17:37

sirreal requested a review from a team as a code owner July 22, 2020 17:37

sirreal requested a review from alshakero July 22, 2020 17:39

fullofcaffeine reviewed Jul 22, 2020

View reviewed changes

Addison-Stavlo approved these changes Jul 22, 2020

View reviewed changes

sirreal added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jul 23, 2020

sirreal merged commit 5307dcf into master Jul 23, 2020

sirreal deleted the fix/43784-fse-missing-inserter branch July 23, 2020 07:56

sirreal mentioned this pull request Jul 23, 2020

Update FSE plugin to version to 1.13 #44125

Merged

31 tasks

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 23, 2020

p-jackson mentioned this pull request Jul 23, 2020

Update FSE to version 1.14 #44415

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FSE: Fix missing inserter #44357

FSE: Fix missing inserter #44357

sirreal commented Jul 22, 2020 •

edited

Loading

matticbot commented Jul 22, 2020

matticbot commented Jul 22, 2020

matticbot commented Jul 22, 2020

fullofcaffeine Jul 22, 2020

Addison-Stavlo Jul 22, 2020

fullofcaffeine commented Jul 22, 2020

sirreal commented Jul 22, 2020

fullofcaffeine commented Jul 22, 2020

fullofcaffeine commented Jul 22, 2020 •

edited

Loading

noahtallen commented Jul 22, 2020

Addison-Stavlo left a comment •

edited

Loading

sirreal commented Jul 23, 2020

sirreal commented Jul 23, 2020

FSE: Fix missing inserter #44357

FSE: Fix missing inserter #44357

Conversation

sirreal commented Jul 22, 2020 • edited Loading

Changes proposed in this Pull Request

Testing instructions

matticbot commented Jul 22, 2020

matticbot commented Jul 22, 2020

matticbot commented Jul 22, 2020

fullofcaffeine Jul 22, 2020

Choose a reason for hiding this comment

Addison-Stavlo Jul 22, 2020

Choose a reason for hiding this comment

fullofcaffeine commented Jul 22, 2020

sirreal commented Jul 22, 2020

fullofcaffeine commented Jul 22, 2020

fullofcaffeine commented Jul 22, 2020 • edited Loading

noahtallen commented Jul 22, 2020

Addison-Stavlo left a comment • edited Loading

Choose a reason for hiding this comment

sirreal commented Jul 23, 2020

sirreal commented Jul 23, 2020

sirreal commented Jul 22, 2020 •

edited

Loading

fullofcaffeine commented Jul 22, 2020 •

edited

Loading

Addison-Stavlo left a comment •

edited

Loading