-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Meta Box back compat and fall backs #3554
Conversation
…with Gutenberg, fall back to the classic editor.
…on to show as non-empty
…'re not rendered.
This is ready for merge, we can iterate on it more as we determine reasons for fallback. |
@@ -36,7 +70,7 @@ have to do, unless we want to move `createEditorInstance()` to fire in the foote | |||
or at some point after `admin_head`. With recent changes to editor bootstrapping | |||
this might now be possible. Test with ACF to make sure. | |||
|
|||
### Redux and React Meta Box Management. | |||
### Redux and React Meta Box Management | |||
|
|||
*The Redux store by default will hold all meta boxes as inactive*. When | |||
`INITIALIZE_META_BOX_STATE` comes in, the store will update any active meta box |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated but we should add this file to the Gutenberg Handbook.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I didn't even notice that it wasn't there. 🙂 Added!
foreach ( $contexts as $context => $priorities ) { | ||
foreach ( $priorities as $priority => $boxes ) { | ||
foreach ( $boxes as $id => $box ) { | ||
if ( ! is_array( $wp_meta_boxes[ $post_type ][ $context ][ $priority ][ $id ]['args'] ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we replace $wp_meta_boxes[ $post_type ][ $context ][ $priority ][ $id ]
by $box
in this loop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can really only replace the first instance - $box
is a copy of the array element, so modifications need to be applied to $wp_meta_boxes
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too many JS in my head :)
|
||
if ( isset( $box['args']['__back_compat_meta_box'] ) ) { | ||
$block_compatible |= (bool) $box['args']['__back_compat_meta_box']; | ||
unset( $box['args']['__back_compat_meta_box'] ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just per curiosity, why do we unset these keys?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copying the same pattern from Core - the meta box render function doesn't need to know about these keys.
*/ | ||
function gutenberg_show_meta_box_warning( $callback ) { | ||
// Only show the warning when WP_DEBUG is enabled. | ||
if ( ! WP_DEBUG ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should not show it even in "prod" while we're on the plugin phase and introduce this change later. To encourage people to check their metaboxes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this way while we're still working out the path for plugin devs to move their meta boxes to blocks - it doesn't seem fair to show a warning that the dev can't do anything to fix.
Once there are well documented upgrade paths, we can take away this check. Once it comes time to merge, we can evaluate how we want to merge it. It might even be valuable to have a "plugin x caused this fall back" message in prod, so users know what happened, and a "here are links to documentation and tutorials to upgrade your meta boxes" when WP_DEBUG
is set, so plugin devs have the information they need.
It currently only redirects if a meta box declares itself incompatible with Gutenberg - the idea is to have an opt-out system, combined with potentially detecting meta boxen that are incompatible.
Nice catch, fixed! |
Codecov Report
@@ Coverage Diff @@
## master #3554 +/- ##
==========================================
+ Coverage 34.9% 36.03% +1.13%
==========================================
Files 263 324 +61
Lines 6727 8453 +1726
Branches 1227 1571 +344
==========================================
+ Hits 2348 3046 +698
- Misses 3694 4564 +870
- Partials 685 843 +158
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this. Let's get this in. I personally prefer it to be opt-in by let's try this and see if we should revisit.
|
||
```php | ||
add_meta_box( 'my-meta-box', 'My Meta Box', 'my_meta_box_callback', | ||
null, 'normal', 'high', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mixed whitespace, tabs and spaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noice. #3684.
} | ||
|
||
// Incompatible meta boxes require an immediate redirect to the classic editor. | ||
if ( $incompatible_meta_box ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we not perform the redirect server-side here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the current code, no. gutenberg_collect_meta_box_data()
happens in the admin_head
action, well after HTML has started being sent.
@@ -219,10 +219,40 @@ function gutenberg_collect_meta_box_data() { | |||
|
|||
// If the meta box should be empty set to false. | |||
foreach ( $locations as $location ) { | |||
if ( isset( $_meta_boxes_copy[ $post->post_type ][ $location ] ) && gutenberg_is_meta_box_empty( $_meta_boxes_copy, $location, $post->post_type ) ) { | |||
if ( gutenberg_is_meta_box_empty( $_meta_boxes_copy, $location, $post->post_type ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This call is always returning false
for "side" meta boxes because the array doesn't have a "side" key even if I do have a side meta box? Any idea how to fix this @pento
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, I might have had a weird setup at that moment. It's working as expected right now.
Even after Gutenberg is merged into Core, and a plugin has upgraded all of its meta boxes to using Gutenberg APIs, those plugin will still need to register classic meta boxes, for backwards compatibility with the classic editor.
Changes
This PR makes a few changes along those lines.
Back Compat Meta Boxen
When registering a meta box, adding a
'__back_compat_meta_box' => true
arg to the$callback_args
argument ofadd_meta_box()
will cause Gutenberg to ignore that meta box in the meta box UI - it's assumed that a plugin developer who defines this arg will have registered a Gutenberg-specific UI for this meta box.For example:
Falling Back To The Classic Editor
If an unsupported meta box is detected while loading Gutenberg, we now redirect to the classic editor. Currently, this is any meta box that explicitly declares the
__block_editor_compatible_meta_box
arg as afalse
-y value.A future iteration could check if a meta box relies on complex scripts which are unlikely to work in Gutenberg, and fall back.
Warning the End User
Ultimately, the end user should know when a plugin causes a fallback to the classic editor. The warning looks something like this:
This currently only shows when
WP_DEBUG
is enabled, though would eventually be enabled at all times.It doesn't show for meta boxes that set the
__back_compat_meta_box
flag, and naturally wouldn't show for other meta boxes that don't trigger a fallback.Code Note
Obviously, some of this code is kind of weird, and a little bit hacky, to hook into the existing meta box rendering scheme.
gutenberg_intercept_meta_box_render()
andgutenberg_override_meta_box_callback()
, for example, will be reduced to some small changes todo_meta_boxes()
, come merge time.#952