Adding initial metabox support. #2583

BE-Webdesign · 2017-08-28T21:48:56Z

This is an imcomplete task but here is an initial jab at adding meta box
support. Keep it on the DL for now.

BE-Webdesign · 2017-08-28T21:54:13Z

youknowriad

This looks like the path we should take. Amazing work

I tried yoast and I guess yoast reads the editor's title and content to update its UI, I was wondering if we could add a hidden input for the title and a hidden TinyMCE for the post content, keeping them in sync with Gutenberg. Would the Yoast metabox (and other metaboxes relying on the editor) work as expected in this case?

Also, can we avoid the "update settings" button at all? Relying on Gutenberg publish button, when we hit it, submit the metaboxes and then fetch the post, apply Gutenberg edits and save it.

I left some notes, but I must say I'm not very familiar with this area.

youknowriad · 2017-08-29T15:04:18Z

lib/metabox-api.php

+		 */
+		?>
+		<header class="gutenberg-metaboxes__header" style="<?php echo $heading_style; ?>">
+			<h2 style="font-size: 14px;">Extended Settings</h2>


Should we get this out of the iframe and show it inside the Gutenberg editor instead and keep only the content in the iframe?. This would allow us to tie this easily to the state (toggle open/close), show a loading maybe while still loading the iframe.

Yeah, if we can take the submit button out of the heading and have Gutenberg trigger this, then we should also be able to have the editor state toggling the state of the metabox area.

youknowriad · 2017-08-29T15:07:45Z

lib/metabox-api.php

+			<div class="gutenberg-metaboxes">
+				<div id="postbox-container-2" class="postbox-container">
+		<?php
+		$locations = array( 'normal', 'advanced', 'side' );


Can we split this into two iframes, one containing advanced and normal locations (like the current one) and show the side metaboxes in the sidebar? Not sure about this, but just asking

I don't know if that will be possible, as the sidebar is managed by React. I could do something similar to what James did as well. Also definitely worth a try. One of the advantages of using the approach taken here is that it will be relatively easy to split different metabox groups into separate iframes etc. So rather than looping like we are here, we can have a query param specify whether we are rendering the normal, advanced, or side location.

youknowriad · 2017-08-29T15:09:42Z

lib/metabox-api.php

+		?>
+		<header class="gutenberg-metaboxes__header" style="<?php echo $heading_style; ?>">
+			<h2 style="font-size: 14px;">Extended Settings</h2>
+			<input name="save" type="submit" class="button button-primary button-large" id="publish" value="Update Settings">


Can we move this out of the iframe, catch the click, submit the iframe and maybe refetch the post object to the Gutenberg state on success?

I think so. Definitely worth a try.

BE-Webdesign · 2017-08-29T16:15:46Z

I tried yoast and I guess yoast reads the editor's title and content to update its UI, I was wondering if we could add a hidden input for the title and a hidden TinyMCE for the post content, keeping them in sync with Gutenberg. Would the Yoast metabox (and other metaboxes relying on the editor) work as expected in this case?

Also, can we avoid the "update settings" button at all? Relying on Gutenberg publish button, when we hit it, submit the metaboxes and then fetch the post, apply Gutenberg edits and save it.

Yeah, these are all really good ideas. The Yoast one will be tricky to do, but I was thinking of a similar approach. We can definitely get rid of the update settings button.

I left some notes, but I must say I'm not very familiar with this area.

Same.

I will probably have time after tomorrows meeting to work more on this, and polish it up. Hopefully the meeting can give us some more insightful feedback as well. I like the idea of trying to integrate it more into React/Redux world, but we should bounce that idea off of others as well.

aduth · 2017-08-30T02:37:56Z

lib/metabox-api.php

+		$notice = false;
+		$form_extra = '';
+		if ( 'auto-draft' == $post->post_status ) {
+			if ( 'edit' == $action )


Getting a notice here at this line:

Notice: Undefined variable: action in /srv/www/editor/htdocs/wp-content/plugins/gutenberg/lib/metabox-api.php on line 204

aduth · 2017-08-30T02:38:46Z

lib/metabox-api.php

+		</html>
+
+		<?php
+		exit( 'YOLO, Gutenberg metaboxes in da haus' );


That's the only true way to exit.

aduth · 2017-08-30T02:41:07Z

lib/register.php

+	global $post;
+	$post_id = $post->ID;
+	$meta_box_api_url = get_admin_url();
+	$meta_box_api_url = $meta_box_api_url . 'post.php';


Could consolidate these two lines as admin_url( 'post.php' )

aaronjorbin · 2017-08-30T02:45:57Z

Also, can we avoid the "update settings" button at all? Relying on Gutenberg publish button, when we hit it, submit the metaboxes and then fetch the post, apply Gutenberg edits and save it.

I think I remember there being issues, but I can't exactly remember what, related to AJAX saving of metadata. This came up during the last distraction free writing update. @iseulde do you remember? There likely is a core ticket somewhere with this info.

Also, it shouldn't just be on publish that metadata is saved.

aduth · 2017-08-30T02:47:31Z

assets/js/iframeResizer.contentWindow.js

+		return;
+	} // don't run for server side render
+
+	let


Given that this is enqueued verbatim, the ES2015 let won't fly in IE11. I wonder if we need a huge library for this, or if we can lean on prior art like the Sandbox component frame observing or similar frame resizing in mce-view.js.

Yeah, let got put in because I ran an eslint fix on these files. so the original I found is just straight es5. I think finding something smaller is a better solution though.

aaronjorbin · 2017-08-30T02:51:19Z

lib/client-assets.php

+		'metabox-resizing-gutenberg',
+		gutenberg_url( 'assets/js/iframeResizer.js' ),
+		array( 'jquery' ),
+		filemtime( gutenberg_dir_path() . 'assets/js/iframeResizer.js' ),


Since this is an external script, should we use the version number?

Yeah, definitely, this will probably be a throwaway script though. Aduth pointed out there are already iframe resizing things being done in Gutenberg.

aduth · 2017-08-30T14:32:19Z

My initial thoughts with iframes was more of a per-metabox approach, but in reflecting on the implementation and some of the discussion here, I think a single iframe could work well. Particularly if we want to try to emulate fields of the current editor (the example of Yoast reading from title). I could imagine the iframe rendering these fields, visually hidden from the user but kept in sync with the parent Gutenberg screen. Already mentioned, but the challenging part is coordinating this sync:

If a field in Gutenberg changes, how do we communicate this to the metabox iframe?
If the Gutenberg post is saved, how do we relay this same command to the metabox iframe?
Conversely, if a metabox field changes, how do we surface this back up to the Gutenberg editor (particularly for flagging as needing save)?

I foresee abundant (ab)use of postMessage 😄

Edit: Since the frames are same origin, maybe we don't need postMessage and can simply call into the frame's contentWindow, and window.parent from the frame.

aduth · 2017-08-30T14:50:50Z

lib/metabox-api.php

+	die( 'Silence is golden.' );
+}
+
+function gutenberg_meta_box_api() {


The "API" naming here could be misleading. It's not really much of an API (at least in the RESTful sense), more just a partial page.

gutenberg_metabox_partial_page()?

BE-Webdesign · 2017-08-30T16:12:42Z

I think I remember there being issues, but I can't exactly remember what, related to AJAX saving of metadata. This came up during the last distraction free writing update. @iseulde do you remember? There likely is a core ticket somewhere with this info.

Also, it shouldn't just be on publish that metadata is saved.

We will definitely want to check that out. We want metadata saved on Publish and Update correct? Draft saving as well I imagine?

aaronjorbin · 2017-08-30T16:33:32Z

We will definitely want to check that out. We want metadata saved on Publish and Update correct? Draft saving as well I imagine?

If the UI says "saved", users will expect that all content on the page is saved. Users should never have to know the difference between what is "the javascript based editor" and what is "an iframe".

BE-Webdesign · 2017-08-30T16:49:21Z

Sounds good

BE-Webdesign · 2017-09-02T01:26:14Z

Interesting new hurdles of using Redux/React:

We will need a way to mark the state as "dirty", when metabox values are changed.

A solution to this could create mutation observers for the iframes and when they detect a change we can mark the post as "dirty". I think that would work well but would love to hear other ideas.

Triggering the submission of a form in an iframe from JS rather than HTML does not appear to trigger HTML validation. Metaboxes will need to 100% rely on server side validation, which they should already, to be secure. If any iframe gurus know about form submission via iframe/JS, chime in with any ideas.
PHPCS gives warning about hooks not matching requirements. Well not sure what to do about that, as these are the hooks WordPress Core uses. Ignoring PHPCS is probably the best solution, unless someone has a better idea?

codecov · 2017-09-03T19:27:08Z

Codecov Report

Merging #2583 into master will decrease coverage by 0.25%.
The diff coverage is 21.96%.

@@            Coverage Diff             @@
##           master    #2583      +/-   ##
==========================================
- Coverage   33.69%   33.43%   -0.26%     
==========================================
  Files         185      187       +2     
  Lines        5594     5724     +130     
  Branches      976      997      +21     
==========================================
+ Hits         1885     1914      +29     
- Misses       3141     3224      +83     
- Partials      568      586      +18

Impacted Files	Coverage Δ
editor/metaboxes/index.js	`0% <0%> (ø)`
editor/index.js	`0% <0%> (ø)`	⬆️
editor/effects.js	`38.96% <0%> (-1.04%)`	⬇️
editor/metaboxes/iframe.js	`0% <0%> (ø)`
editor/sidebar/index.js	`0% <0%> (ø)`	⬆️
editor/layout/index.js	`0% <0%> (ø)`	⬆️
editor/actions.js	`47.36% <100%> (+6.19%)`	⬆️
editor/reducer.js	`89.4% <100%> (+0.75%)`	⬆️
editor/selectors.js	`96.75% <100%> (+0.35%)`	⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26369c3...bc1046f. Read the comment docs.

BE-Webdesign · 2017-09-08T20:35:32Z

@youknowriad, @aduth, @matias: I think this is now at a pretty decent baseline of functionality. Would love feedback. The last piece before starting to put this in front of Metabox people, is getting Yoast to work.

youknowriad · 2017-09-16T17:46:19Z

Hey @BE-Webdesign I wanted to try this, I'm thinking we should try to move it to a mergeable state but I had trouble making it work, the settings just didn't show up. what am i missing?

BE-Webdesign · 2017-09-16T18:46:02Z

@youknowriad Should be fixed now, sorry about the confusion. Thank you for taking the time to help track down that bug.

BE-Webdesign · 2017-09-17T00:49:54Z

Do not change the editor behavior if there are no metaboxes: no extra fetch/save request, no settings button.

Yup, should be reasonably doable.

I'm seeing custom taxonomies metaboxes showing up but we already have them baked in Gutenberg

This is partly tied into the above issue, but it should be relatively easy to fix.

I think the metaboxes using the content editor and the title input can be handled as a follow-up PR. We should try to iterate on this, we can't get it right for all metaboxes in the first iteration.

Yeah, that is also a possibility. I am pretty sure this covers the vast majority of metabox use cases. Iteration is totally fine by me.

Also the metabox blinking effect when updated is not a great experience but this also could be handled separately, we have solutions for this.

Yeah, personally I would like to put that in before merge, if possible, but am open to whatever. I think the blinking will really throw people off, and on what is already a very sensitive topic, we want to make sure that this is handled as seamlessly as possible.

What do you think?

I agree with all of your points. I think this works pretty well and with the addition of some of these minor changes, we can have something mergeable.

I think we want to also come up with a plan to deprecate these legacy metaboxes as well, this is some pretty hacktastic code, that should not live in Gutenberg forever. I think block templates will largely solve that problem, and hopefully with the superior user experience people will switch over, making an eventual deprecation smooth.

I will try and do these changes and see how that looks. If you can help with the blinking iframes that would be cool.

BE-Webdesign · 2017-09-18T13:24:12Z

Going to patch this up now.

BE-Webdesign · 2017-09-18T14:32:22Z

Detecting whether metaboxes are empty and instructing React/Redux is definitely more tricky than it ought to be. I have another hacky solution almost ready.

BE-Webdesign · 2017-09-19T16:09:20Z

This is sadly a wormhole of a problem. Luckily the problem is related to how ACF registers metaboxes. I will try to hunt down what is going on exactly. Going to take a bit more time than expected.

BE-Webdesign · 2017-09-19T16:11:30Z

how ACF registers metaboxes

Nevermind not as bad, was just hook timing.

BE-Webdesign · 2017-09-19T16:12:49Z

Nevermind still terrible.

karmatosed · 2017-09-21T20:35:36Z

I am currently trying to give a design review for this and one thing strikes me on first loading, that's the double areas. For example, I was using ACF and see this:

I think this is being mentioned and worked on and that the one without the field in it should just not show.

I wanted to add a visual though of the issue so those following along design wise can catch up. As we progress, if we are able to show screenshots, it really helps include everyone to get a review.

BE-Webdesign · 2017-09-21T23:20:31Z

I think this is being mentioned and worked on and that the one without the field in it should just not show.

I wanted to add a visual though of the issue so those following along design wise can catch up. As we progress, if we are able to show screenshots, it really helps include everyone to get a review.

Sounds great! Thank you karmatosed. I am indeed working on hiding any empty metabox areas. It is pretty much done, and working locally. I just need to touch things up before I push it live.

Basic metabox support please see docs/metabox.md for details.

BE-Webdesign · 2017-09-24T16:40:08Z

@youknowriad, @matias, @karmatosed and anyone else who is interested!:

Please see general overview for details on how this works.

Design review would be great! I tried to imitate what Joen had in the original #952. I still probably need to add the open close for the extended settings as a whole. Shouldn't be too hard to do that now. Solving the iframe flicker would also be nice. I can do that, but if y'all already have good solutions for that we can use those.

youknowriad · 2017-09-25T13:45:45Z

@BE-Webdesign Awesome document, it really helps clarifying how it works.

Solving the iframe flicker would also be nice. I can do that, but if y'all already have good solutions for that we can use those.

I tied to play a bit with this, but my "hack" was not satisfying. I tried saving the iframe's HTML into a variable and set it into a second iframe while submitting the form, it worked but sometimes weird things happen :). I think I was not setting the temporary iframe's HTML properly for some unknown reason.

BE-Webdesign · 2017-09-25T13:48:46Z

Awesome document, it really helps clarifying how it works.

There are definitely some details missing in it, but it will hopefully provide a good general overview.

On iframe flicker:

I have a solution, if you want me to try it. I am gonna clone the iframe when it goes to update, hide the iframe that is updating, and on reload delete the clone.

youknowriad · 2017-09-25T13:56:58Z

I have a solution, if you want me to try it. I am gonna clone the iframe when it goes to update, hide the iframe that is updating, and on reload delete the clone.

yep, go for it, that's what I tried but miserably failed to achieve :)

BE-Webdesign · 2017-09-25T14:00:55Z

yep, go for it, that's what I tried but miserably failed to achieve :)

I am sure you could solve it, I will try to solve it. Is there anything else I should try to update? I think after the flicker is fixed, this will be in a decent place, then tackling the Yoast stuff will be a next step?

youknowriad · 2017-09-25T14:07:08Z

Yep, we should merge this, to gather some feedback and test it by plugin developers.

BE-Webdesign · 2017-09-25T14:35:15Z

Okay sounds good. Some of the comments are possibly applicable lol.

karmatosed · 2017-09-25T18:20:13Z

For a design review, I feel there are still 2 things I would love fixed, although I don't feel that is a merge breaker. I think it can be refined after merge.

1: The fact that we have overly long input fields. I can see a need for strong stress testing of this area.
2. The empty area when another has it. I would suggest only one area is open by default.

I feel other things will come up as we get people to test with their plugins, so holding back for now. The principle is true to the initial concept.

BE-Webdesign · 2017-09-25T22:05:44Z

The fact that we have overly long input fields. I can see a need for strong stress testing of this area.

I can address that, and have things be more in line with the original mockup.

The empty area when another has it. I would suggest only one area is open by default.

I will also change this to be more inline with the original mockup.

BE-Webdesign · 2017-09-26T16:44:13Z

@karmatosed, @joen.

Another idea for the design layout brought up by @aduth, is that maybe we could have the metabox area fixed at the bottom of the content area, which could then expand upwards to show metaboxes. In the current wp-admin experience, metaboxes must be scrolled to, past large swaths of content. It might create a better user experience being able to quickly open and collapse metaboxes. Using this UI approach, we could also reduce the complexity of this PR in two aspects:

Eliminate the need to have an iframe resizing hack, as the expanded metabox area will become a scrollable element.
Eliminate the need to remove iframe flicker when the metaboxes are saved (double buffering). We could instead collapse the metabox area with a nice animation of sorts on save.

This could have drawbacks as well to the user experience as the metabox area might become "cramped" being set in a set height scrollable area.

Word are probably not a good way to communicate these ideas, so what would be the proposed method of comparing, both solutions?

BE-Webdesign · 2017-09-29T13:29:51Z

Closing in favor of #2804. A new branch was made so that returning to the non expandable version of metaboxes would be painless and easy.

BE-Webdesign added Needs Design Needs design efforts. Needs Design Feedback Needs general design feedback. [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible [Status] In Progress Tracking issues with work in progress labels Aug 28, 2017

BE-Webdesign mentioned this pull request Aug 28, 2017

Add Filter to make it possible to only add gutenberg links on a per post type basis #2528

Merged

youknowriad reviewed Aug 29, 2017

View reviewed changes

aduth reviewed Aug 30, 2017

View reviewed changes

aaronjorbin reviewed Aug 30, 2017

View reviewed changes

aduth reviewed Aug 30, 2017

View reviewed changes

aduth mentioned this pull request Aug 31, 2017

ACF, Meta Not Showing. Classic Editor failing to load on localhost as well. #2631

Closed

BE-Webdesign force-pushed the support/metaboxes-with-endpoint branch 2 times, most recently from 347689e to 500dfea Compare September 2, 2017 01:18

BE-Webdesign force-pushed the support/metaboxes-with-endpoint branch from 500dfea to 77df0d6 Compare September 3, 2017 19:27

karmatosed removed the Needs Design Feedback Needs general design feedback. label Sep 4, 2017

BE-Webdesign force-pushed the support/metaboxes-with-endpoint branch from 6fd936c to 29c7cca Compare September 8, 2017 20:30

BE-Webdesign force-pushed the support/metaboxes-with-endpoint branch from d164007 to 9a57062 Compare September 24, 2017 16:23

Adding initial metabox support.

bc1046f

Basic metabox support please see docs/metabox.md for details.

BE-Webdesign force-pushed the support/metaboxes-with-endpoint branch from 9a57062 to bc1046f Compare September 24, 2017 16:33

WordPress deleted a comment from BE-Webdesign Sep 25, 2017

aduth mentioned this pull request Sep 26, 2017

Layout: Add metabox placeholder shell #2800

Merged

karmatosed removed the Needs Design Needs design efforts. label Sep 27, 2017

BE-Webdesign closed this Sep 29, 2017

nylen mentioned this pull request Sep 29, 2017

Render "normal" metaboxes created by Advanced Custom Fields #2251

Closed

BE-Webdesign deleted the support/metaboxes-with-endpoint branch October 21, 2017 00:41

snyk-bot mentioned this pull request Sep 11, 2022

[Snyk] Upgrade react-native-webview from 11.6.2 to 11.23.0 captainbarber99/gutenberg#4

Open

npneeraj mentioned this pull request Jan 14, 2023

[Snyk] Upgrade react-native-webview from 11.6.2 to 11.26.0 npneeraj/gutenberg#3

Open

Adding initial metabox support. #2583

Adding initial metabox support. #2583

Conversation

BE-Webdesign commented Aug 28, 2017

BE-Webdesign commented Aug 28, 2017 • edited

youknowriad left a comment • edited

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

BE-Webdesign Aug 29, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BE-Webdesign commented Aug 29, 2017

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

aaronjorbin commented Aug 30, 2017

Choose a reason for hiding this comment

BE-Webdesign Aug 30, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BE-Webdesign Aug 30, 2017 • edited

Choose a reason for hiding this comment

aduth commented Aug 30, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BE-Webdesign commented Aug 30, 2017

aaronjorbin commented Aug 30, 2017

BE-Webdesign commented Aug 30, 2017

BE-Webdesign commented Sep 2, 2017 • edited

codecov bot commented Sep 3, 2017 • edited

Codecov Report

BE-Webdesign commented Sep 8, 2017

youknowriad commented Sep 16, 2017

BE-Webdesign commented Sep 16, 2017

BE-Webdesign commented Sep 17, 2017

BE-Webdesign commented Sep 18, 2017

BE-Webdesign commented Sep 18, 2017

BE-Webdesign commented Sep 19, 2017

BE-Webdesign commented Sep 19, 2017

BE-Webdesign commented Sep 19, 2017

karmatosed commented Sep 21, 2017 • edited

BE-Webdesign commented Sep 21, 2017 • edited

BE-Webdesign commented Sep 24, 2017

youknowriad commented Sep 25, 2017

BE-Webdesign commented Sep 25, 2017

youknowriad commented Sep 25, 2017

BE-Webdesign commented Sep 25, 2017

youknowriad commented Sep 25, 2017

BE-Webdesign commented Sep 25, 2017

karmatosed commented Sep 25, 2017

BE-Webdesign commented Sep 25, 2017

BE-Webdesign commented Sep 26, 2017 • edited

BE-Webdesign commented Sep 29, 2017

BE-Webdesign commented Aug 28, 2017 •

edited

youknowriad left a comment •

edited

BE-Webdesign Aug 29, 2017 •

edited

BE-Webdesign Aug 30, 2017 •

edited

BE-Webdesign Aug 30, 2017 •

edited

aduth commented Aug 30, 2017 •

edited

BE-Webdesign commented Sep 2, 2017 •

edited

codecov bot commented Sep 3, 2017 •

edited

karmatosed commented Sep 21, 2017 •

edited

BE-Webdesign commented Sep 21, 2017 •

edited

BE-Webdesign commented Sep 26, 2017 •

edited