Automattic / gutenberg-ramp

Control conditions under which Gutenberg loads - either from your theme code or from a UI
https://wordpress.org/plugins/gutenberg-ramp/
76 stars 15 forks source link

Prepare for WordPress 5.0 #70

Closed pyronaur closed 5 years ago

pyronaur commented 5 years ago

This PR prepares Ramp for the WordPress 5.0 release. I tried to be descriptive in each of the commit message titles and comments - please review each separately for a more detailed view.

To summarize - This is Gutenberg Ramp 1.1-rc.1 - a lot of stuff is removed for compatibility with WordPress 5.0 and beyond. Some important decisions were made along the way:

_p.s. Along the way I cleaned up a PHP Warning, a few variable names and moved adding filters out of the Gutenberg_Ramp class constructor to make the code a bit more object-oriented. I know that it's sub-optimal, but to move faster I decided to make those changes on the fly instead of making separate PRs that depend on this one_

mattoperry commented 5 years ago

@justnorris This looks really great. Well thought out and well done! I can clearly see how this works with 5.0, which will basically be our major concern from here on out. A few general questions just so I'm sure:

That's all of the questions I have -- this looks great.

pyronaur commented 5 years ago

what's the minimum version of the GB plugin now required by ramp? (3.5 it seems?)

Yes - Gutenberg 3.5 is the minimum required for this to work. However, as I understand it - Gutenberg versions below 4.1 aren't going to be compatible with WordPress 5.0 at all.

it seems that the new strategy is to always load the plugin and then use the new-ish filters to disable it. Is that right?

Yes

Are there load-order consideration if that disabling happens from another plugin?

The filter that can be used to disable Gutenberg is only executed after after_setup_theme - all plugins and the active theme functions.php haven been loaded by that time, so no special load order decisions are necessary.

In regards to the load order of the Gutenberg plugin itself - if Gutenberg functions aren't available by the time Ramp is loaded - Ramp will instantly include Gutenberg using an include() just like it did before - so nothing has changed in that area. The only difference is that now it does that only based on a single condition - whether or not register_block_type is present, where as previously we checked for should_load and will_load on init, which, I think, was the reason for plugin load oder concerns we had before.

mjangda commented 5 years ago

Ramp is no longer using WordPress options to cache the load decisions because the load decisions happen after after_setup_theme - so themes and plugins can now call the ramp loading function and see the effects instantly.

I'm assuming there are no functional changes other than the fact we were previously persisting the data in options. What testing do we need to do to make sure this doesn't break expected behaviour?

pyronaur commented 5 years ago

I'm assuming there are no functional changes other than the fact we were previously persisting the data in options.

There are no changes when it comes to how developers can/should interact with Ramp, but a lot has changed underneath - most significantly - not persisting the data and the timing for Ramp to make decisions (load order). The load order has changed for the better and is now more forgiving, so I don't expect trouble there :)

What testing do we need to do to make sure this doesn't break expected behaviour?

I don't think the update will break the expected behavior and I've tested Ramp in various configurations (post types, post ID's, using the UI, etc.). You can perform the same tests to make sure.

One area that I'm currently blank on is what happens when the following plugins are active at the same time: "Gutenberg Ramp" + "Classic Editor" + "Gutenberg" - but I think that's probably a separate issue to work on.

pyronaur commented 5 years ago

Added #71 to track fallback for Gutenberg < 3.5