First pass at Gutenberg compatibility #484

rinatkhaziev · 2018-12-10T14:44:09Z

This PR lays the groundwork for implementing compatibility for all Gutenberg and also implements Custom Status module for Gutenberg.

BLOCKS.md describes the anatomy of an Edit Flow block and how to implement a new one.

… to implement Gutenberg compat for other modules

…eact and ReactDOM as external to avoid an extra copy loaded

Regenerate package-lock.json to get rid of stray node modules. PHPDoc. First pass at BLOCKS.md that describes how to implement compatibility for Gutenberg: general PHP section.

…ssets are minified.

pyronaur

I left a few inline comments to consider.

It seems you came to the same conclusion as I did ( have a look at the individual commits and messages ) when I tried to refactor Edit Flow - the current module dependency system is a bit difficult to work with.

At first solved this with traits too, but then I realized that I'm now keeping track of the "which module am I" instance inside the traits, which I think is just going to make it more difficult to understand things.

That said - "Gutenberg Compatibility" does sound like a nice trait name and use case, I just want to make sure that we make Edit Flow code easier to follow after we add to it - so I don't know the right answer here :)

pyronaur · 2018-12-10T14:54:58Z

BLOCKS.md

+
+**Important**
+
+To avoid any sort of class inheritance Edit Flow compat files use a trait [Block_Editor_Compatible](common/php/trait-block-editor-compatible.php). Right now it only contains the constructor that's shared between compat modules, but in the future using traits approach might be more flexible.


Why do we want to avoid class inheritance? How will traits provide more flexibility over inheritance?

Imagine a situation where you have 50 plugins using the same pattern (adding hooks on class instantiation or something similar) and you need all of them working with Gutenberg. With the trait it's easy to just drop it in, abstract it a bit more, and make small changes to the class logic. Class inheritance complicates things.

I feel like we just need to expand on that a bit in the docs.

blocks/src/custom-status/block.js

pyronaur · 2018-12-10T15:44:51Z

common/php/trait-block-editor-compatible.php

+   * @return void
+   */
+  function action_admin_init() {
+    // This conditional is probably not catching all the instances where Gutenberg might be enabled.


The only way to detect if Gutenberg is being currently used is probably through listening for the use_block_editor_for_post (and the post type hook equivalent) on priority 999999 and capturing the result and basing the decision on that instead of `classic-editor.

Also - I'm not sure what exactly is being un-hooked and re-hooked. Maybe add a comment about the what and the why?

@justnorris that part got me puzzled, I wasn't sure what's the way to 100% reliably tell whether Gutenberg is enabled and whether we're editing a Gutenberg page. classic-editor seemed to work. ok.

Checking for classic editor will only check for the classic editor scenario, this will fail when Gutenberg is disabled with a hook, for example here: https://github.com/Automattic/gutenberg-ramp/blob/master/gutenberg-ramp.php#L179

@justnorris I have reworked that part, as a resident Guten-compat expert do you think the new version covers all of it?

pyronaur · 2018-12-10T15:52:03Z

modules/custom-status/compat/block-editor.php

+	 * @return void
+	 */
+	function action_admin_enqueue_scripts() {
+		if ( $this->ef_module->disable_custom_statuses_for_post_type() || !function_exists( 'is_gutenberg_page' ) || ! is_gutenberg_page() )


I think a WordPress 5.0 Ready check is necessary here. Maybe WP_Screen->is_block_editor() is usable? http://developer.wordpress.org/reference/classes/wp_screen/is_block_editor/

modules/custom-status/custom-status.php

pyronaur · 2018-12-10T15:55:20Z

modules/custom-status/compat/block-editor.php

+		if ( $this->ef_module->disable_custom_statuses_for_post_type() || !function_exists( 'is_gutenberg_page' ) || ! is_gutenberg_page() )
+			return;
+
+		wp_enqueue_style( 'edit-flow-block-custom-status', EDIT_FLOW_URL . 'blocks/dist/custom-status.editor.build.css', false, null );


Is it necessary to enqueue the new styles in a separate file? Can we use the already existing Edit Flow CSS file?

@justnorris Yep, the reason being we don't want to load any dead CSS and JS.

pyronaur · 2018-12-10T15:57:39Z

modules/custom-status/custom-status.php

@@ -59,10 +70,9 @@ function __construct() {
 				'title' => __('Overview', 'edit-flow'),
 				'content' => __('<p>Edit Flow’s custom statuses allow you to define the most important stages of your editorial workflow. Out of the box, WordPress only offers “Draft” and “Pending Review” as post states. With custom statuses, you can create your own post states like “In Progress”, “Pitch”, or “Waiting for Edit” and keep or delete the originals. You can also drag and drop statuses to set the best order for your workflow.</p><p>Custom statuses are fully integrated into the rest of Edit Flow and the WordPress admin. On the calendar and story budget, you can filter your view to see only posts of a specific post state. Furthermore, email notifications can be sent to a specific group of users when a post changes state.</p>', 'edit-flow'),
 				),
-			'settings_help_sidebar' => __( '<p><strong>For more information:</strong></p><p><a href="http://editflow.org/features/custom-statuses/">Custom Status Documentation</a></p><p><a href="http://wordpress.org/tags/edit-flow?forum_id=10">Edit Flow Forum</a></p><p><a href="https://github.com/danielbachhuber/Edit-Flow">Edit Flow on Github</a></p>', 'edit-flow' ),
+			'settings_help_sidebar' => __( '<p><strong>For more information:</strong></p><p><a href="http://editflow.org/features/custom-statuses/">Custom Status Documentation</a></p><p><a href="http://wordpress.org/tags/edit-flow?forum_id=10">Edit Flow Forum</a></p><p><a href="https://github.com/Automattic/Edit-Flow">Edit Flow on Github</a></p>', 'edit-flow' ),


To shorten the amount of changed lines, maybe avoid changes unrelated to this particular feature and whitespace changes?

Although it'd be great to clean up a ton of EF code - maybe a separate PR for that?

My bad, it's just my editor's setting to automatically trim trailing whitespaces. A separate PR with clean up would be nice, but hiding whitespace changes in Diff settings also works just fine.

common/php/trait-block-editor-compatible.php

mjangda

Left mostly minor notes. The only big thing is the 5.4 version check with the introduction of traits.

One other slight worry I have is that the trait stuff is a bit of an over-optimization for now (since we haven't scoped out what other other Guten-compat work we need), but the changes are simple enough that we can modify later if needed.

mjangda · 2018-12-10T22:06:03Z

webpack.config.js

+  return acc;
+}, {});
+
+// @todo


What do we need to do? :)

Who knows? Todos that happened on a plane stay on that plane :) I'll either remove or expand on it.

mjangda · 2018-12-11T07:19:54Z

BLOCKS.md

@@ -0,0 +1,107 @@
+# Block Editor Compatibility


❤️ (thanks for this!)

blocks/src/custom-status/block.js

edit_flow.php

modules/custom-status/compat/block-editor.php

@justnorris

Fix a typo, props @justnorris Co-Authored-By: rinatkhaziev <[email protected]>

@mjangda

Version the assets when calling `wp_enqueue_scripts`. Props @mjangda Co-Authored-By: rinatkhaziev <[email protected]>

Add missing comma. Co-Authored-By: rinatkhaziev <[email protected]>

Co-Authored-By: rinatkhaziev <[email protected]>

common/php/trait-block-editor-compatible.php

… determines whether compat classes should be loaded

Fix whitespace. Co-Authored-By: rinatkhaziev <[email protected]>

CS: fix an inline control structure. Co-Authored-By: rinatkhaziev <[email protected]>

rinatkhaziev added 7 commits December 8, 2018 00:55

First pass at Custom Post statuses in Gutenberg, some foundation work…

6d35da8

… to implement Gutenberg compat for other modules

Custom Status block, Webpack build tools

ea1af4c

Remove @wordpress/components dep, instead use wp.components, define R…

28641d5

…eact and ReactDOM as external to avoid an extra copy loaded

Optimize webpack config to generate files for each Edit Flow module.

315d77f

Slight refactoring of over-engineering.

1477856

Regenerate package-lock.json to get rid of stray node modules. PHPDoc. First pass at BLOCKS.md that describes how to implement compatibility for Gutenberg: general PHP section.

Readme updates, Webpack config updates

b0995b1

Minify production code, fix 'build' command to make sure production a…

9f1f3e3

…ssets are minified.

rinatkhaziev added this to the 0.9 milestone Dec 10, 2018

pyronaur requested changes Dec 10, 2018

View reviewed changes

mjangda reviewed Dec 10, 2018

View reviewed changes

common/php/trait-block-editor-compatible.php Show resolved Hide resolved

mjangda requested changes Dec 11, 2018

View reviewed changes

Norris and others added 4 commits December 11, 2018 10:00

Update modules/custom-status/custom-status.php

87267a7

Fix a typo, props @justnorris Co-Authored-By: rinatkhaziev <[email protected]>

Apply suggestions from code review

c68815c

Version the assets when calling `wp_enqueue_scripts`. Props @mjangda Co-Authored-By: rinatkhaziev <[email protected]>

Apply suggestions from code review

9a01266

Add missing comma. Co-Authored-By: rinatkhaziev <[email protected]>

Update language for status note when the post is already published.

55079a4

Co-Authored-By: rinatkhaziev <[email protected]>

pyronaur reviewed Dec 12, 2018

View reviewed changes

common/php/trait-block-editor-compatible.php Outdated Show resolved Hide resolved

pyronaur reviewed Dec 12, 2018

View reviewed changes

common/php/trait-block-editor-compatible.php Outdated Show resolved Hide resolved

rinatkhaziev and others added 5 commits December 13, 2018 19:19

Better docs for Block_Editor_Compatible trait, re-work the logic that…

42771b9

… determines whether compat classes should be loaded

Bump minimum PHP version to 5.4

5453a05

Update blocks/src/custom-status/block.js

e39dc8e

Fix whitespace. Co-Authored-By: rinatkhaziev <[email protected]>

BLOCKS.md update: add File Structure section, fix minor grammar issues.

e71c409

Apply suggestions from code review

a734cf9

CS: fix an inline control structure. Co-Authored-By: rinatkhaziev <[email protected]>

rinatkhaziev merged commit 4a91391 into master Jan 10, 2019

GaryJones deleted the guten-compat branch December 9, 2019 15:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First pass at Gutenberg compatibility #484

First pass at Gutenberg compatibility #484

rinatkhaziev commented Dec 10, 2018

pyronaur left a comment

pyronaur Dec 10, 2018

rinatkhaziev Dec 11, 2018

pyronaur Dec 10, 2018

rinatkhaziev Dec 11, 2018

pyronaur Dec 12, 2018

rinatkhaziev Dec 17, 2018

pyronaur Dec 10, 2018

pyronaur Dec 10, 2018

rinatkhaziev Dec 11, 2018

pyronaur Dec 10, 2018

rinatkhaziev Dec 17, 2018

mjangda left a comment

mjangda Dec 10, 2018

rinatkhaziev Dec 11, 2018

mjangda Dec 11, 2018


		Important

		To avoid any sort of class inheritance Edit Flow compat files use a trait [Block_Editor_Compatible](common/php/trait-block-editor-compatible.php). Right now it only contains the constructor that's shared between compat modules, but in the future using traits approach might be more flexible.

First pass at Gutenberg compatibility #484

First pass at Gutenberg compatibility #484

Conversation

rinatkhaziev commented Dec 10, 2018

pyronaur left a 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

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

mjangda left a 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