Framework: Move components from the blocks to the editor module #6521

youknowriad · 2018-05-01T12:43:59Z

Spoiler alert: Huge PR coming with big implications.

Related #6275

Sorry for the huge PR but sometimes big changes are unavoidable. The idea of this PR is to move the reusable components to build blocks from the blocks module to the editor module (the reasoning behind this is explained in #6275)

Notes

I'm polyfilling wp.blocks.* components to ensure backward compatibility
There are some "todos" still: (renaming filters and moving tests) that I left for a separate PR

Questions

We should communicate about this change, we'll leave time for people to upgrade their code but this is definitely going to impact all custom blocks.

How can we move forward with this?

noisysocks · 2018-05-02T02:49:37Z

We should communicate about this change, we'll leave time for people to upgrade their code but this is definitely going to impact all custom blocks.

A polyfill with a deprecated message that we leave in for 3-5 minor versions seems totally reasonable to me.

gziolo · 2018-05-04T09:06:23Z

editor/index.native.js

@@ -0,0 +1 @@
+export { default as PlainText } from './components/plain-text';


@hypest is it enough to make it work on mobile?

Thanks for asking @gziolo , yes, that's enough to make it work on mobile. I wonder though, wdyt about "native-ize" ./components/index.native.js instead of ./editor/index.native.js? Is the components module too web-specific you think?

The difference between editor and components is that components is meant to be generic UI components independent from WP backend or the editor while the editor is for components used to build blocks/editor.

PlainText and RichText can be bound to the editor state, that's wy they live in editor.

Thanks for the insight @youknowriad , that's helpful!

In this particular case, I'm actually referring to the editor/components mobule, not the top-level components. If I understand it correctly, PlainText and RichText live in the submodule in this PR, the index.js of which I'm suggesting to "native-ize". So, editor/index.native.js will still import * from './components' but editor/components/index.native.js will only export PlainText for now. Does that make sense?

Oh, Understood and actually, I don't know. I don't really know how your react-native setup works, I thought the most important thing would be the root module (editor/index) but yeah if you need to keep the rest of the editor/index as is. I can update to move this to editor/components/index.native.js

I thought the most important thing would be the root module (editor/index)

Glad you mention that so we can expand a bit. I guess the main premise for now is to use the index.jss that serve as muster points as a good opportunity for specialization/nativization. Not necessarily the root module ones though. All index.jss can fall under that pattern. Hope this makes sense.

I can update to move this to editor/components/index.native.js

Yes, let's go with that, thanks! 🙇

aduth · 2018-05-04T12:53:10Z

blocks/index.js

-export { default as RichText } from './rich-text';
-export { default as RichTextProvider } from './rich-text/provider';
-export { default as UrlInput } from './url-input';
-export { default as UrlInputButton } from './url-input/button';
 export { default as EditorSettings, withEditorSettings } from './editor-settings';


Why do editor-settings and editor-media-upload remain?

because I was planning to them separately. See #6500 for the editor settings. And both require the components to be moved first.

aduth · 2018-05-04T12:54:53Z

editor/components/index.js

@@ -1,3 +1,28 @@
+// Block Creation Component


Nit: "Component" -> "Components"

aduth · 2018-05-04T12:57:17Z

editor/deprecated.js

+			alternative: 'wp.editor.' + key,
+			plugin: 'Gutenberg',
+		} );
+		wrappedFunction( ...args );


Should this return ?

gziolo · 2018-05-04T13:39:13Z

@youknowriad, can we list all the remaining task so we could split work with follow-ups?
I can help with all test related and filter related tasks if you need want.

gziolo · 2018-05-04T13:51:35Z

Pushed c3463ca with RN changes.

youknowriad · 2018-05-04T13:59:14Z

@gziolo Sure, I updated the issue description with a todo list, feel free to add what I missed #6275

gziolo · 2018-05-04T14:06:53Z

editor/deprecated.js

+	withColorContext,
+	getColorClass,
+	getColorName,
+	withColors,


@jorgefilipecosta what's the difference between withColors and withColorContext?
It's the last chance to rename them to make it less confusing.

…he editor module

gziolo

I tested with a plugin that registers block that uses deprecated wp.blocks.* component and it worked properly and I could see the warning:

wp.blocks.InspectorControls is deprecated and will be removed from Gutenberg in 3.1. Please use wp.editor.InspectorControls instead.

There is one small debugging change introduced which should be removed. I also see the React warning coming from the Custom HTML block. It needs to be confirmed it isn't regression introduced by this PR:

Warning: Failed prop type: You provided a value prop to a form field without an onChange handler. This will render a read-only field. If the field should be mutable use defaultValue. Otherwise, set either onChange or readOnly.
in textarea (created by CodeEditor)
in CodeEditor (created by LazyCodeEditor)

In overall, this works as expected and looks great. I think we should proceed as soon as two issues raised are sorted out.

gziolo · 2018-05-07T10:22:55Z

lib/client-assets.php

@@ -471,10 +452,10 @@ function gutenberg_preload_api_request( $memo, $path ) {
 * @since 0.1.0
 */
 function gutenberg_register_vendor_scripts() {
-	$suffix = SCRIPT_DEBUG ? '' : '.min';
+	$suffix = SCRIPT_DEBUG || true ? '' : '.min';


|| true shouldn't be included here and in the next statement :)

youknowriad · 2018-05-07T10:55:41Z

Confirmed the React warning is also present in master, let's address separately.

gziolo · 2018-05-07T13:02:05Z

🎉

youknowriad added the Framework Issues related to broader framework topics, especially as it relates to javascript label May 1, 2018

youknowriad self-assigned this May 1, 2018

youknowriad requested a review from a team May 1, 2018 12:43

youknowriad force-pushed the try/move-components-2-editor branch from 7a9c1d8 to c7f3d23 Compare May 1, 2018 12:45

youknowriad force-pushed the try/move-components-2-editor branch from c7f3d23 to 0086b5e Compare May 2, 2018 09:27

gziolo mentioned this pull request May 2, 2018

Editor: Make sure createInnerBlockList never updates when passed using context #6420

Merged

4 tasks

youknowriad force-pushed the try/move-components-2-editor branch from 0086b5e to af2a4ad Compare May 4, 2018 08:49

gziolo reviewed May 4, 2018

View reviewed changes

aduth reviewed May 4, 2018

View reviewed changes

youknowriad force-pushed the try/move-components-2-editor branch from 0d97455 to 93f9bfc Compare May 4, 2018 13:03

youknowriad mentioned this pull request May 4, 2018

Rethink the separation between editor and blocks module #6275

Closed

7 tasks

gziolo reviewed May 4, 2018

View reviewed changes

youknowriad force-pushed the try/move-components-2-editor branch 3 times, most recently from 40920e9 to 2673327 Compare May 7, 2018 08:23

youknowriad and others added 11 commits May 7, 2018 10:11

Framework: Move reusable block components from the blocks module to t…

3db8496

…he editor module

Add deprecation message for wp.blocks.*

4b3ec1b

Update documentation to match the code

4e5f804

Fix unit tests

2bec9f4

More deprecations

402c220

Fix e2e tests

ffe0b40

Fix deprecated functions

d09fb7f

Editor: Update class name for RN files

387edcc

Use proper import statements for deprecated block components

50ab6ca

Ensure the editor module is loaded before third party plugins

5bc3a72

Tweak editor/components react-native version

d12ff98

youknowriad force-pushed the try/move-components-2-editor branch from ec95203 to d12ff98 Compare May 7, 2018 09:12

gziolo approved these changes May 7, 2018

View reviewed changes

Removing debugging code

b4c737d

youknowriad merged commit f95b2d5 into master May 7, 2018

youknowriad deleted the try/move-components-2-editor branch May 7, 2018 11:25

mcsf mentioned this pull request May 11, 2018

Classic: Fix absent editor styles #6709

Merged

4 tasks

This was referenced May 14, 2018

Blocks: Move applicable hooks from editor to blocks #6730

Closed

Tests: Move blocks raw handling tests to test/integration folder #6731

Merged

mtias added this to the 2.9 milestone May 17, 2018

aduth mentioned this pull request May 24, 2018

Editor: Simplify block context to eliminate createInnerBlockList #6943

Closed

ellatrix mentioned this pull request Jul 16, 2021

Styles: remove wp-editor from wp-edit-blocks dependencies #33496

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Framework: Move components from the blocks to the editor module #6521

Framework: Move components from the blocks to the editor module #6521

youknowriad commented May 1, 2018

noisysocks commented May 2, 2018

gziolo May 4, 2018

hypest May 4, 2018 •

edited

Loading

youknowriad May 4, 2018

hypest May 4, 2018 •

edited

Loading

youknowriad May 4, 2018

hypest May 4, 2018

aduth May 4, 2018

youknowriad May 4, 2018

aduth May 4, 2018

aduth May 4, 2018

gziolo commented May 4, 2018

gziolo commented May 4, 2018

youknowriad commented May 4, 2018

gziolo May 4, 2018

gziolo left a comment •

edited

Loading

gziolo May 7, 2018

youknowriad commented May 7, 2018

gziolo commented May 7, 2018

		@@ -0,0 +1 @@
		export { default as PlainText } from './components/plain-text';

Framework: Move components from the blocks to the editor module #6521

Framework: Move components from the blocks to the editor module #6521

Conversation

youknowriad commented May 1, 2018

noisysocks commented May 2, 2018

Choose a reason for hiding this comment

hypest May 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hypest May 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gziolo commented May 4, 2018

gziolo commented May 4, 2018

youknowriad commented May 4, 2018

Choose a reason for hiding this comment

gziolo left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad commented May 7, 2018

gziolo commented May 7, 2018

hypest May 4, 2018 •

edited

Loading

hypest May 4, 2018 •

edited

Loading

gziolo left a comment •

edited

Loading