Add SlotFill in Status & Availability panel to match existing hook #6300

ryanwelcher · 2018-04-20T01:44:55Z

Description

This PR adds a slotFill and is meant as a first pass to fix #6299

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.

…tus and Availability panel. Relies on WordPress/gutenberg#6300

adamsilverstein · 2018-04-20T13:05:47Z

This looks good @ryanwelcher , thanks for contributing! I can definitely see the use case for adding a slot here and am surprised there isn;t something here already. It would be good to add some docs for this slot, and possibly tests especially if we already have testing around other slots.

youknowriad · 2018-04-20T13:13:12Z

edit-post/components/sidebar/plugin-post-status-visibility/index.js

+ * Defines extensibility lot for the Status & Visibility panel
+ */
+
+import { createSlotFill } from '../../../../components/slot-fill';


This will include the whole components bundle in the edit-post bundle, to import from other modules, we use @wordpress/*. This should be import { createSlotFille } from '@wordpress/components'

youknowriad · 2018-04-20T13:13:35Z

edit-post/components/sidebar/post-status/index.js

@@ -17,6 +17,8 @@ import PostSticky from '../post-sticky';
 import PostAuthor from '../post-author';
 import PostFormat from '../post-format';
 import PostPendingStatus from '../post-pending-status';
+import PluginPostStatusVisibility from '../plugin-post-status-visibility';
+import PanelRow from '../../../../components/panel/row';


Same as above (external import)

@youknowriad are you meaning the PanelRow and the PluginPostStatusVisibility import? I've done the first but not 100% sure on how to do the second.

I was only talking about PanelRow, all good now

youknowriad · 2018-04-20T13:14:41Z

edit-post/index.js

@@ -77,5 +77,6 @@ export function initializeEditor( id, post, settings ) {
 	};
 }

+export { default as PluginPostStatusVisibility } from './components/sidebar/plugin-post-status-visibility';


I'll defer to others on the name and the slot itself cc @gziolo @mtias @aduth

We usually export as PluginPostStatusVisibility (Fill) + PluginPostStatusVisibility.Slot, but I'm not a big fun of this approach. This one exports them as PluginPostStatusVisibility.Fill + PluginPostStatusVisibility.Slot. The only benefit of what we have in other places it that Fill is used in many places but Slot usually only in one.

I think we should only export the Fills (in the external API)

gziolo · 2018-04-20T13:21:50Z

It is indeed a very good request, but against the overall plan for the user experience planned for the editor. @jasmussen and @mtias should be able to answer this question, but as far as I am aware, we don't want to allow plugins to add their own functionalities in Document Settings sidebar. Instead we are going to allow to add their own controls in the Pre-Publish and Post-Publish screens as described in #3330.

Pre-publish Checkup

Some information is the most useful when it is shown to you in context of the publishing act. Things like scheduling and post visibility. It's also a space where plugins can add useful last-minute checkup notices, such as an SEO or readability score, or perhaps a way to customize a publicize-to-Twitter message before publish.

There is work started by @c-shultz in #5795 on this functionality which hopefully will land soon in master.

adamsilverstein · 2018-04-20T13:44:01Z

we don't want to allow plugins to add their own functionalities in Document Settings sidebar.

Where would you suggest plugins add information about the status and visibility of a post - including a published post?

I understand the need to show plugin information during publish flow, however there is also a great use case for displaying document status and visibility information inside the status and visibility area of the editor. In particular, when a plugin has information to add about the status or visibility of the post.

There are many examples, our particular use case is for the Distributor plugin which enables sharing content between WordPress sites. When a post has been shared in this way, that information shows up in the status area:

This information is related to the status of the post you are editing and we would like to show it in Gutenberg in the matching location:

Showing the status information in the publish flow is not helpful the goal of the message is to inform the user of the distribution status when they open the editor. Published posts also need to show this information, and no publish flow is available for these posts. As soon as users open the post edit screen, it should be clear if the post is a distributed (shared) post. Even for drafts, the publish flow is not available until changes are made to the post, which is disallowed for distributed posts.

jasmussen · 2018-04-20T13:51:00Z

Thanks so much for your contribution here!

My personal opinion is this is a valid use case, and it is eloquently described by Adam as well.

However I do recall a bunch of discussions around how to build the extensibility, and remember that @mtias had some thoughts about how this is best architected for the future. These discussions, I believe included some thoughts about adding items to existing metaboxes, exactly as is suggested here.

Alas, I can't recall the exact details, and Matías is AFK this week, but I believe he will be back some time next week. I would prefer that he gets to share his thoughts on this before we merge. But we should be able to have a decision before 2.8.

gziolo · 2018-04-20T13:59:24Z

edit-post/components/sidebar/post-status/index.js

@@ -32,6 +33,9 @@ function PostStatus( { isOpened, onTogglePanel } ) {
 			<PostSticky />
 			<PostPendingStatus />
 			<PostAuthor />
+			<PanelRow>


I don't think <PanelRow> should be included here. At the moment every item in this Panel is wrapped with PanelRow. Instead, we can expose Fill which is wrapped with PanelRow. In effect, you would add new items as follows:

<PluginPostStatusVisibility> My item </PluginPostStatusVisibility>

and Fill would work as follows:

const PluginPostStatusVisibility = ( { children } ) => ( <Fill> <PanelRow> { children } </PanelRow> </Fill> );

Alternative solution would be to leave it to plugin developers in case they wanted to add more than one item:

<PluginPostStatusVisibility> <PanelRow>My item 1</PanelRow> <PanelRow>My item 2</PanelRow> </PluginPostStatusVisibility>

@gziolo I was thinking the same thing, I would prefer to have a single component. I am new using createSlotFill and am not sure how to implement what you are asking for. Where does

const PluginPostStatusVisibility = ( { children } ) => ( <Fill> <PanelRow> { children } </PanelRow> </Fill> );

go?

It can go in here: https://github.com/WordPress/gutenberg/pull/6300/files#diff-ee98bfe279b9987ab8753b1c2fd680f0R7

You will have to juggle a bit with exports.

@gziolo can you provide some examples of how I would be able to do this - specifically the reference to juggling the exports?

aaronjorbin · 2018-04-20T14:26:58Z

Along with documentation, I would love to see a simple example added to https://github.com/wordpress/gutenberg-examples

Maybe something as simple as showing the post ID.

gziolo · 2018-04-20T14:29:59Z

Along with documentation, I would love to see a simple example added to https://github.com/wordpress/gutenberg-examples
Maybe something as simple as showing the post ID.

I was thinking we should rather create a plugin that would be included in e2e tests similar to https://github.com/WordPress/gutenberg/blob/master/test/e2e/test-plugins/templates.php but with JS code enqueued. I plan to do something like that for the Sidebar and MenuItem working with Plugins API.

aaronjorbin · 2018-04-20T14:33:49Z

edit-post/components/sidebar/plugin-post-status-visibility/index.js

@@ -0,0 +1,7 @@
+/**
+ * Defines extensibility lot for the Status & Visibility panel


slot, not lot

gziolo · 2018-04-20T14:39:02Z

@ryanwelcher and @adamsilverstein - We need to wait for @mtias to confirm that this change is expected or there is another way that it can be achieved.

In the meantime, do you think it is expected to have other panels extensible in the Document Settings sidebar? I wanted to ensure we provide flexible enough solution. If we want to have more panels extensible then we should seek for a more general solution like:

<PluginDocumentSettings panel="status-visibility" />

gziolo · 2018-04-20T14:48:54Z

@ryanwelcher - sharing the example for the Sidebar for reference, to better picture how it looks in action: https://gist.github.com/gziolo/e2954aa83aa1f823b2b05ca1660c2223. This is also how you would extend Status & Visibility if we agree that it is the way to go.

adamsilverstein · 2018-04-20T14:52:44Z

I wanted to ensure we provide flexible enough solution. If we want to have more panels extensible then we should seek for a more general solution like

I was thinking we need a more general higher level way of creating these slots and wasn't sure how... so in your example <PluginDocumentSettings panel="status-visibility" /> you would have one of these in each panel, and then plugins would be able to fill them specifically by passing the panel attribute?

gziolo · 2018-04-20T15:01:13Z

@adamsilverstein yes, that's the idea to have matching Slot for each of them: <PluginDocumentSettings.Slot panel="status-visibility" />. Behind the scenes, it would technically create a few Slot & Fill pairs but would be at least exposed using unified API. It would be much easier to document and use rather than coming up with a name for every possible panel :)

adamsilverstein · 2018-04-20T15:12:35Z

@gziolo in general I like the concept of what you are describing. So looking at the existing panels:

I see the following panels:

status-visibility
revisions
categories
tags
featured
excerpt
discussion

Do you already have an idea of how this might work behind the scenes?

youknowriad · 2018-04-20T15:19:14Z

As developers, we tend to like extensibility APIs by default, because we know these will empower what we would be able to achieve with plugins etc... And part of me is the same, I want to be able to extend everything in my plugin.

But as Core developers, we have to be very careful and don't just add slots everywhere because in the end, these will end up abused by extensions even if the said extension doesn't really make sense in this Area. Plugin authors will try to be as visible as they can regardless of the User Experience, they just want to be used.

Not saying, we shouldn't add slots in the Document Panel at specific positions but I'm saying we should be very careful and if it's possible to leverage a unique extensibility API (menuitem + sidebar|modal|screentakeover|post/prepublishpanel), I'd prefer to stick with it even if it's not perfect for all use-cases.

aaronjorbin · 2018-04-20T15:24:08Z

To add to @rianrietveld's statement (which is 💯), future compatibility is easier the less extensible something is. The amount of community pushback when extension points are removed is incredible, and rightfully so. Creating trust requires a stable API and trust is a foundation for the success of a project.

youknowriad · 2018-04-20T15:29:15Z

The amount of community pushback when extension points are removed is incredible, and rightfully so. Creating trust requires a stable API and trust is a foundation for the success of a project.

The API proposed here is a new API, even if it's similar to an existing API, we can't say it's stable because people will rewrite their applications anyway. I think we should consider this as an opportunity to think about the API. Whether we really need it or not. It's easier to add APIs later than to remove.

We should not neglect the fact that adding more and more slots will inevitably create an inconsistent User Experience.

adamsilverstein · 2018-04-20T15:41:34Z

as Core developers, we have to be very careful and don't just add slots everywhere because in the end, these will end up abused by extensions even if the said extension doesn't really make sense in this Area

I completely agree with this and wondered about this at first when reading the more general proposal. After considering though, I could imaging a use case for each of the existing panels and I'm sure there are many use cases that I haven't imagined.

Perhaps we should avoid changing the API for now and simple add slots in spots on a case by case basis where a clear use case can be articulated. We can look to our existing extension points (exposed as do_action events in php) for guidance precisely because these actions have been added based on prior use cases and discussion that we should not ignore. We can also look to existing popular plugins for guidance, where we see a valuable use we should consider providing extensibility.

We should not neglect the fact that adding more and more slots will inevitably create an inconsistent User Experience.

That is a risk, and we need to find a balance between the desire for consistent user experience with the desire to be extensible and useful to the developers who make the platform itself great for users. Any extension point is an opportunity for unanticipated possibly poor UX. Hopefully we can alleviate that by providing consistent APIs and documenting simple code pattern examples.

gziolo · 2018-04-23T14:23:57Z

Let's put it on hold until we hear from @mtias to make sure there isn't an alternative solution that we missed. If it turns out we should add new slots, I will provide all necessary help to make it both flexible and easy to use.

gziolo · 2018-05-04T13:06:52Z

Merge with master didn’t go well 🙁

ryanwelcher · 2018-05-04T13:08:02Z

@gziolo it's that kind of day already :) I'm addressing now.

adamsilverstein · 2018-05-04T13:24:52Z

edit-post/README.md

@@ -90,3 +90,32 @@ The [Dashicon](https://developer.wordpress.org/resource/dashicons/) icon slug st
 - Required: No


+## SlotsFill


"Slot Fills"?

ryanwelcher · 2018-05-04T13:28:19Z

@gziolo I've addressed the bad merge with master and added some documentation. Would love some feedback to be sure the docs are clear and complete.

adamsilverstein · 2018-05-04T13:28:35Z

edit-post/README.md

@@ -90,3 +90,32 @@ The [Dashicon](https://developer.wordpress.org/resource/dashicons/) icon slug st
 - Required: No


+## SlotFills
+SlotFills provide plugin or theme authors the ability to add items into the Gutenberg interface. While these slots appear in certain locations, 


Question: are the items above on this page also slotfills? can you check the codebase and if they are move this header section to the top of the page (eg are PluginSidebar, PluginSidebarMoreMenuItem slots)?

@adamsilverstein the do have slots defined in them yes. Perhaps we don't need the SlotFills header and I add the note about the implementation vs location under the header for the actual slot.

adamsilverstein · 2018-05-04T18:03:33Z

@gziolo can you please review this again, especially the documentation - I'm really not sure where this belongs, is there one place in the docs that lists all of the slots that are available?

Thanks!

gziolo · 2018-05-04T18:15:07Z

It’s placed properly. We will have to find a better way to expose it in Gutenberg handbook. At the moment this Readme is only linked from Extensibility docs. I didn’t look closely, I’ll review on Monday. There might be some small stylistic tweaks required but nothing major really. Previous code examples use JSX and there some other markdown differences. This Slot & Fill idea needs to be better explained in the foreword or sth.

gziolo · 2018-05-04T18:19:04Z

edit-post/components/sidebar/plugin-post-status-info/index.js

+/**
+ * Defines as extensibility slot for the Status & Visibility panel
+ */
+import { createSlotFill, PanelRow } from '@wordpress/components';


This import statement should have WordPress dependencies comment as it is done in all other files.

gziolo

I addressed my own comments to speed up the process. It is good to go in my opinion. @ryanwelcher and @adamsilverstein hank you for driving this task 🙇

ryanwelcher pushed a commit to 10up/distributor that referenced this pull request Apr 20, 2018

Adding registerPlugin to inject the Distributor details in to the Sta…

8fe3da1

…tus and Availability panel. Relies on WordPress/gutenberg#6300

ryanwelcher mentioned this pull request Apr 20, 2018

Add SlotFill in Status & Availability panel to match existing hook. #6299

Closed

2 tasks

adamsilverstein requested a review from youknowriad April 20, 2018 13:06

youknowriad reviewed Apr 20, 2018

View reviewed changes

ryanwelcher force-pushed the feature/visbility-and-status-extensibility branch from eefa198 to 90d1d41 Compare April 20, 2018 13:25

adamsilverstein added Needs Design Feedback Needs general design feedback. [Feature] Extensibility The ability to extend blocks or the editing experience labels Apr 20, 2018

gziolo reviewed Apr 20, 2018

View reviewed changes

aaronjorbin reviewed Apr 20, 2018

View reviewed changes

gziolo requested a review from mtias April 20, 2018 14:49

Ryan Welcher and others added 6 commits May 4, 2018 08:29

Rename slot to PluginPostStatusInfo

8591618

Structural changes and only exporting the Fill to the public api

1c13ac0

Lint fixes

51b72c4

Edit Post: Use PluginPostStatusInfo alias for Fill

9da8053

Edit Post: Fix export for the PluginPostStatusInfo

bbc9da9

Adding documentation notes

183beaf

Ryan Welcher added 2 commits May 4, 2018 09:17

Merge remote-tracking branch 'upstream/master'

28eb3a8

Merge branch 'master' into feature/visbility-and-status-extensibility

a9025b0

ryanwelcher force-pushed the feature/visbility-and-status-extensibility branch from 0001a6c to a9025b0 Compare May 4, 2018 13:18

adamsilverstein reviewed May 4, 2018

View reviewed changes

Fixes typo in SlotFills

198a552

adamsilverstein reviewed May 4, 2018

View reviewed changes

Changes the docs

1ec48fa

gziolo removed the Needs Design Feedback Needs general design feedback. label May 4, 2018

gziolo added this to the 2.9 milestone May 4, 2018

gziolo reviewed May 4, 2018

View reviewed changes

gziolo added 2 commits May 7, 2018 09:55

Edit Post: Update documentation for plugins

a71c4e9

Edit Post: Small tweak with inline comments

8efddae

gziolo approved these changes May 7, 2018

View reviewed changes

gziolo merged commit 555c29a into WordPress:master May 7, 2018

adamsilverstein mentioned this pull request May 7, 2018

Add Gutenberg sidebar status availability panel 10up/distributor#122

Merged

youknowriad mentioned this pull request May 9, 2018

Add option to Status & Visibility? #6661

Closed

gziolo mentioned this pull request May 21, 2018

Edit Post: Wrap PluginPostStatusInfo with PanelRow rather than Slot #6867

Merged

4 tasks

rmleg mentioned this pull request Jan 29, 2020

Needs porting to Gutenberg NOVALISTIC/keep-comments-open#1

Open

		@@ -0,0 +1,7 @@
		/**
		* Defines extensibility lot for the Status & Visibility panel

		@@ -90,3 +90,32 @@ The [Dashicon](https://developer.wordpress.org/resource/dashicons/) icon slug st
		- Required: No


		## SlotsFill

Add SlotFill in Status & Availability panel to match existing hook #6300

Add SlotFill in Status & Availability panel to match existing hook #6300

Conversation

ryanwelcher commented Apr 20, 2018 • edited by gziolo Loading

Description

Checklist:

adamsilverstein commented Apr 20, 2018 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad Apr 20, 2018 • edited Loading

Choose a reason for hiding this comment

gziolo commented Apr 20, 2018

Pre-publish Checkup

adamsilverstein commented Apr 20, 2018

jasmussen commented Apr 20, 2018

gziolo Apr 20, 2018 • edited Loading

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 Apr 20, 2018

gziolo commented Apr 20, 2018

Choose a reason for hiding this comment

gziolo commented Apr 20, 2018 • edited Loading

gziolo commented Apr 20, 2018

adamsilverstein commented Apr 20, 2018

gziolo commented Apr 20, 2018

adamsilverstein commented Apr 20, 2018

youknowriad commented Apr 20, 2018

aaronjorbin commented Apr 20, 2018

youknowriad commented Apr 20, 2018

adamsilverstein commented Apr 20, 2018

gziolo commented Apr 23, 2018

gziolo commented May 4, 2018

ryanwelcher commented May 4, 2018

adamsilverstein May 4, 2018 • edited Loading

Choose a reason for hiding this comment

ryanwelcher commented May 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamsilverstein commented May 4, 2018

gziolo commented May 4, 2018

gziolo May 4, 2018 • edited Loading

Choose a reason for hiding this comment

gziolo left a comment

Choose a reason for hiding this comment

ryanwelcher commented Apr 20, 2018 •

edited by gziolo

Loading

adamsilverstein commented Apr 20, 2018 •

edited

Loading

youknowriad Apr 20, 2018 •

edited

Loading

gziolo Apr 20, 2018 •

edited

Loading

gziolo commented Apr 20, 2018 •

edited

Loading

adamsilverstein May 4, 2018 •

edited

Loading

gziolo May 4, 2018 •

edited

Loading