Skip to content
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

ACF Metaboxes are always visible #12571

Closed
elliotcondon opened this issue Dec 4, 2018 · 24 comments
Closed

ACF Metaboxes are always visible #12571

elliotcondon opened this issue Dec 4, 2018 · 24 comments
Assignees
Labels
[Feature] Meta Boxes A draggable box shown on the post editing screen [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@elliotcondon
Copy link

Describe the bug
All field groups created with the Advanced Custom Fields plugin are shown as empty visible metaboxes when editing a post irrelevant of those field group's location rules.

screen shot 2018-12-04 at 1 41 20 pm

Image above shows all field groups as empty metaboxes when editing the Hello World post.

Understanding the bug
The reason for this is due to Gutenberg removing/modifying the visibility of metaboxes during initialization. Please see example code below to replicate the issue.

Here is a little background on the problem and why this is an important one to fix before 5.0 is released...

ACF registers all field groups as metaboxes and then uses JS to hide/show them based on their location rules. This allows for dynamic updating of metaboxes when post attributes (such as category, post_format, post_parent) are being edited. This approach has allowed ACF to honor the custom metabox order and position settings set when dragging around metaboxes.

With Gutenberg activated (or WordPress 5.0 RC2), all metaboxes are shown as visible. I believe that some JS code within Gutenberg is setting the visibility of metaboxes without checking if the metabox is already hidden by custom JS.

Why this needs to be fixed
There are lots of reasons why this needs to be fixed, but I will try to win you over with just a few:

  1. User experience. Any user with ACF installed will run into this issue within the first few minutes of activating Gutenberg / updating to 5.0. This presents a negative user experience to those who have customized websites. Users will wonder "what else is broken"?
  2. Adoption. This issue unfortunately reflects badly on Gutenberg. The problem is only visible when Gutenberg is active and will ultimately lead to unhappy developers / users who have ACF installed.

To Reproduce
Here is some example code and some attachments to demonstrate the problem:

<?php 

// Register a metabox.
add_action('add_meta_boxes', 'test_add_meta_boxes', 10, 2);
function test_add_meta_boxes( $post_type, $post ) {
	
	add_meta_box( 'test-metabox', 'Test Metabox', 'test_render_meta_box', $post_type, 'normal', 'high' );
}

// Render metabox HTML.
function test_render_meta_box() {
	?>
	<script type="text/javascript">
	(function($) {
		$('#test-metabox').hide();
		$('#test-metabox .inside').html('This should be hidden.');
	})(jQuery);	
	</script>
	<?php
}

?>

screen shot 2018-12-04 at 1 24 57 pm

Expected behavior
Metaboxes hidden via custom JS should remain hidden.

Additional context

  • Issue present in all versions of Gutenberg or WP 5.0
@designsimply designsimply added [Type] Bug An existing feature does not function as intended [Feature] Meta Boxes A draggable box shown on the post editing screen labels Dec 4, 2018
@noisysocks noisysocks added this to the WordPress 5.0.1 milestone Dec 4, 2018
@noisysocks noisysocks self-assigned this Dec 4, 2018
@noisysocks
Copy link
Member

This happens because MetaBoxVisibility sets style.display to '' when it mounts:

componentDidMount() {
this.updateDOM();
}

One easy fix would be to update this to only update the DOM if we need to hide the meta box:

componentDidMount() {
	if ( ! this.props.isVisible ) {
		this.updateDOM();
	}
}

However the user would still be able to show the meta box again by opening Options and toggling off/on the checkbox for that meta box.

@elliotcondon: Are you able to unregister the meta box using remove_meta_box() instead of hiding it with CSS? This way the meta box will be removed from both the editor and from the Options modal.

@elliotcondon
Copy link
Author

@noisysocks Thanks for the reply and info.

I'm happy to look into an alternative solution for dynamically updating metaboxes, however, it will not be possible to roll out an update to all users before Thursday's WP5.0 release.

Is it possible to "mount" the metaboxes without removing their "display" style?
This would allow Gutenberg to inherit the metabox HTML as expected.

@noisysocks
Copy link
Member

Is it possible to "mount" the metaboxes without removing their "display" style?

Yes this is essentially what the change I proposed above would do. It only mitigates the bug, though, since Gutenberg needs to set style.display to '' or 'none' so that the user is able to enable and disable meta boxes via the Options modal.

@elliotcondon
Copy link
Author

That makes sense. Is this easy enough to push into the 5.0 release?

@noisysocks
Copy link
Member

I'm sorry to say that it is too late in the release cycle for this to be fixed for 5.0 and that this isn't a blocking issue. I've added it to the 5.0.1 milestone which is targeted for two weeks after 5.0 is released on December 6.

As a workaround, you could look into hiding the element using an approach that doesn't modify style.display, e.g.

- $('#test-metabox').hide();
+ $('#test-metabox').addClass( 'hidden' );

@maddisondesigns

This comment has been minimized.

@logicalphase

This comment has been minimized.

@webdados

This comment has been minimized.

@rochow
Copy link

rochow commented Dec 5, 2018

I assume enabling the classic editor fixes this issue? Unsure how much Gutenberg code is still included with it on

@earnjam
Copy link
Contributor

earnjam commented Dec 5, 2018

@elliotcondon Is this a regression, or has it been functioning this way all along?

@noisysocks
Copy link
Member

It’s likely that this is a regression introduced a month ago by #11084.

Please let’s stay on topic. This isn’t the place to be discussing the 5.0 release process which is what it is.

@bupotalovo
Copy link

What does "super hacky approach" in #11084 mean?

Wouldn't it be better to use a "non hacky approach", as WordPress 5.0 "is ready" now? Or isn't it?

@rpkoller
Copy link

rpkoller commented Dec 5, 2018

@noisysocks have you successfully tested your pull request against ACF upfront before the merge like @youknowriad recommended?

@noisysocks
Copy link
Member

Apologies, I got my wires crossed there. This regression was introduced in 4.1 six weeks ago by #10676.

@elliotcondon
Copy link
Author

Hi all. We are starting to see a lot of support tickets regarding this issue. Please keep this GitHub ticket updated with your plans to address the problem.

@noisysocks
Copy link
Member

noisysocks commented Dec 7, 2018

#12628 fixes the issue and will be shipped as part of WordPress 5.0.1 in ~2 weeks.

Hiding the meta box using $( '#test-metabox' ).addClass( 'hidden' ) or wp.data.dispatch( 'core/edit-post' ).removeEditorPanel( 'meta-box-test-metabox' ) remains the preferred workaround.

Going forward, we recommend that removeEditorPanel() or remove_meta_box() is used for cases like these.

@elliotcondon
Copy link
Author

It sounds like we will need to release an update to fix this from our end.
It is a shame that such a little fix couldn't have been added to WP 5.0 prior to the release.
This directly effects every ACF user updating to 5.0.

@gmsdesignworks

This comment has been minimized.

@noisysocks
Copy link
Member

Yes, it's regrettable that this bug wasn't discovered and fixed in time for the 5.0 code freeze.

Please let's stay on topic. GitHub is our workplace. It's important that we keep the conversation here friendly and on topic so that existing contributors are able to do their best work and new contributors are made welcome to the project.

@gmsdesignworks

This comment has been minimized.

@maddisondesigns

This comment has been minimized.

@pento
Copy link
Member

pento commented Dec 7, 2018

As @noisysocks mentioned, this repository is where we work: just as you wouldn't wander into someone's office and bad mouth them or their work, doing it here is equally unacceptable.

I understand that it's frustrating to run into a bug, particularly if it affects sites that you manage in production. However, that's no excuse to vent your frustration in a bug report, which exists specifically for the purpose of fixing that bug.

#12628 will be in WordPress 5.0.1, which will be released in ~2 weeks. If you'd like to help ensure that it's fixed completely, I would encourage you to test that PR and see if it fixes the issue for you, particularly for more complex ACF configurations.

In the mean time, I've hidden the off topic comments in this thread. Please keep further discussion on topic.

@murshed
Copy link

murshed commented Dec 7, 2018

Still, have this issue after updating 5.0

Any luck?

@gmsdesignworks

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Meta Boxes A draggable box shown on the post editing screen [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests