Skip to content

/gutenberg

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

Plugin: Remove replace_editor filter, extend core editor #13569

Merged
merged 3 commits into from Mar 13, 2019
+173 −702

Conversation

Reviewers

@jorgefilipecosta jorgefilipecosta

@mkaz mkaz

@youknowriad youknowriad

@nosolosw nosolosw

@talldan talldan

@gziolo gziolo

@ntwb ntwb

@ajitbohra ajitbohra

@chrisvanpatten chrisvanpatten

@nerrad nerrad

@notnownikki notnownikki

Assignees
No one assigned
Labels
Good First Review Priority High [Component] Plugin
Projects
  
Done in Remove PHP
Milestone
  5.3 (Gutenberg)
5 participants
@aduth @mcsf @youknowriad @jorgefilipecosta @gziolo
@aduth
Copy link
Member

commented Jan 29, 2019 •
edited

Related: #11015

This pull request seeks to eliminate Gutenberg's replace_editor feature, instead allowing the default core block editor to load, changing Gutenberg's behavior to instead extend the core block editor and replace the core script handles with the Gutenberg latest versions.

Tasks separated out:

  • Deprecate / remove gutenberg_add_admin_body_class (#13572)
  • Defer vendor scripts to core. (#13573)
  • Drop inline scripts in favor of override_script improvements. (#13581)
  • Load locale data. (#12559)
  • Remove Gutenberg's heartbeat proxying. (#13576)
  • Apply Gutenberg's built editor-styles.css to editor settings by filter. (#13625)
  • Extract wp-editor-font stylesheet override (#14176)
  • Remove deprecated _wpLoadGutenbergEditor, gutenberg theme supports (#14144)
  • Await E2E promise interactions (#14219)

Needs confirmation as core aligned:

  • In fixing #5667, Gutenberg filters user_can_richedit as always true in the context of the block editor. Was this implemented in core?
    • It was not implemented, but as discussed at #5670 (comment) it did not need to be. See #13608 for removal from Gutenberg.
  • Does Gutenberg still need to extend / replace the styles property from the block editor settings?
    • It's still necessary, as this is a build distributable from Gutenberg, and may need to change in source to override that provided by core.
  • Gutenberg's heartbeat-to-hooks proxying must be implemented in core
  • Confirm whether core implemented the "Screen Options" hiding equivalent to Gutenberg's screen_options_show_screen filtering

@aduth aduth added [Status] In Progress [Component] Plugin labels Jan 29, 2019

@aduth aduth added this to In Progress in Remove PHP via automation Jan 29, 2019

@youknowriad youknowriad referenced this pull request Jan 29, 2019

Closed

Remove PHP code merged into WordPress 5.0 #11015

5 of 5 tasks complete

This was referenced Jan 30, 2019

Merged
Merged
@mcsf

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Jan 30, 2019

  • In fixing #5667, Gutenberg filters user_can_richedit as always true in the context of the block editor. Was this implemented in core?

Commented in #5670 (comment)

@aduth aduth referenced this pull request Jan 30, 2019

Merged

Plugin: Remove `user_can_richedit` filtering #13608

@aduth aduth force-pushed the remove/replace-editor branch from 27a8cd8 to 4185fc6 Jan 31, 2019

This was referenced Jan 31, 2019

Merged
Closed

@azaozz azaozz referenced this pull request Feb 1, 2019

Open

Plugin: improve bootstrapping #13634

@aduth aduth referenced this pull request Feb 27, 2019

Merged

Try a generic block editor module #13088

5 of 5 tasks complete

@aduth aduth force-pushed the remove/replace-editor branch from 4185fc6 to e7a7d5e Feb 27, 2019

@aduth aduth requested review from ajitbohra, chrisvanpatten, gziolo, mkaz, nosolosw, notnownikki and youknowriad as code owners Feb 27, 2019

@aduth aduth force-pushed the remove/replace-editor branch from a65cf0c to b65649e Feb 28, 2019

@aduth aduth referenced this pull request Feb 28, 2019

Merged

Plugin: Remove wp-editor-font stylesheet override #14176

@aduth aduth force-pushed the remove/replace-editor branch 2 times, most recently from e6f47f1 to 44351ed Mar 1, 2019

@aduth aduth requested review from nerrad, ntwb and talldan as code owners Mar 1, 2019

@aduth aduth changed the title WIP: Plugin: Remove replace_editor filter, extend core editor Plugin: Remove replace_editor filter, extend core editor Mar 1, 2019

@aduth aduth removed the [Status] In Progress label Mar 1, 2019

@aduth aduth force-pushed the remove/replace-editor branch from 225d2f6 to a9df08f Mar 5, 2019

@aduth

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Mar 5, 2019 •
edited

Build is green now 🎉

For what it's worth, I was having some issues with intermittent failures for the new tags end-to-end tests introduced in #13129 (cc @jorgefilipecosta). It should have been improved with #14219, so I might think it could have just been some caching problem.

aduth added a commit that referenced this pull request Mar 6, 2019

@aduth
Testing: Remove core-block-theme-test.php 
Slated for removal as of #13569
Loading status checks…
651d562

@aduth aduth force-pushed the remove/replace-editor branch from d9a4482 to 5b77a48 Mar 6, 2019

@aduth aduth referenced this pull request Mar 7, 2019

Merged

If a more recent revision/autosave exists, store its state on editor setup #7945

4 of 4 tasks complete

@gziolo gziolo added the Good First Review label Mar 8, 2019

@aduth aduth referenced this pull request Mar 12, 2019

Merged

Try Legacy widget block #13511

@aduth aduth force-pushed the remove/replace-editor branch from 5b77a48 to e3a0dd5 Mar 12, 2019

@aduth

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Mar 12, 2019

@jorgefilipecosta Can you please review this in mind of rebased refactor of widgets interoperability, per changes of #13511.

@aduth aduth requested a review from jorgefilipecosta Mar 12, 2019

@jorgefilipecosta

jorgefilipecosta reviewed Mar 13, 2019
View changes

Copy link
Member

left a comment

I did a set of tests with legacy widgets (including JavaScript ones), and I did not notice any difference of behavior in this version when compared with the previous one. Thank you for applying these changes that isolate the legacy widget mechanism 👍

I also did some tests with metaboxes and everything worked as before.

I left a comment regarding a dependency removal that I did not understand but other than that the changes look good to me.
lib/client-assets.php
@@ -299,7 +299,7 @@ function gutenberg_register_scripts_and_styles() {
gutenberg_override_style(
'wp-block-library',
gutenberg_url( 'build/block-library/style.css' ),
current_theme_supports( 'wp-block-styles' ) ? array( 'wp-block-library-theme' ) : array(),

This comment has been minimized.

Sign in to view
Copy link
@jorgefilipecosta

jorgefilipecosta Mar 13, 2019

Member

I'm not understanding the reason for this change. I think 'wp-block-styles' should still depend on 'wp-block-library-theme' if the theme supports them.
gutenberg_override_style calls wp_register_style with the new dependencies so even if this dependency was set in core as we are registering it again we still need to pass it.

This comment has been minimized.

Sign in to view
Copy link
@youknowriad

youknowriad Mar 13, 2019

Contributor

I have vague memories that it's done in a different way in Core and this might align both approaches. The thing is, it's not possible to call current_theme_supports in the script loader in Core.

This comment has been minimized.

Sign in to view
Copy link
@aduth

aduth Mar 13, 2019

Author Member

Yes, there's a bit more of an explanation in in the extended comment description for 2282ee4 . It still exists, but it's manually enqueued in core's implementation of the block scripts and styles function:

https://github.com/WordPress/wordpress-develop/blob/e421f26/src/wp-includes/script-loader.php#L2626-L2630

You can confirm this adding the following snippet somewhere in a theme which adds support for wp-block-styles (e.g. Twentynineteen):

add_action( 'enqueue_block_assets', function() {
	var_export( current_theme_supports( 'wp-block-styles' ) );
	var_export( wp_style_is( 'wp-block-library-theme', 'enqueued' ) );
	exit;
} );

Should display "truetrue" on the editor screen.

This comment has been minimized.

Sign in to view
Copy link
@jorgefilipecosta

jorgefilipecosta Mar 13, 2019

Member

Thank you for the clarification @youknowriad, and @aduth.

@aduth aduth force-pushed the remove/replace-editor branch from e3a0dd5 to c31f85d Mar 13, 2019

@aduth

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Mar 13, 2019

I did another force-push since it seemed Travis became stuck again.
@jorgefilipecosta

jorgefilipecosta approved these changes Mar 13, 2019
View changes

Copy link
Member

left a comment

In think we should merge this soon so we have more time to test. In my tests I did not found any regression and the changes make sense to me (althougth I dont' have deep knowlodge in some of the areas).

Thank you for this changes and improvements 👍
@aduth

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Mar 13, 2019

Thanks for the reviews @jorgefilipecosta 👍

@aduth aduth merged commit 67656bc into master Mar 13, 2019

Remove PHP automation moved this from In Progress to Done Mar 13, 2019

@aduth aduth deleted the remove/replace-editor branch Mar 13, 2019

This was referenced Mar 18, 2019

Merged
Merged
Closed
Closed

@youknowriad youknowriad referenced this pull request Mar 22, 2019

Closed

Undefined index: description in [...]/plugins/gutenberg/lib/widgets.php on line 98 #14552

mkevins added a commit to mkevins/gutenberg that referenced this pull request Mar 26, 2019

@aduth @mkevins
Plugin: Remove replace_editor filter, extend core editor (WordPress#1… 
…3569)

* Plugin: Remove replace_editor filter, extend core editor

* Plugin: Avoid dynamic dependencies for wp-block-library style

This is handled by core in common blocks style enqueues behavior https://github.com/WordPress/wordpress-develop/blob/e421f26/src/wp-includes/script-loader.php#L2626-L2630

* Testing: Leverage saveDraft for meta boxes save
05642be

This was referenced Apr 18, 2019

Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.