PrePublishPanel: suggest tags and postformat #8235

oandregal · 2018-07-26T17:36:59Z

Related #7426

This PR aims to include the Tags and the PostFormat components in the pre-publish panel:

Tags component is shown only if the user hasn't tagged the post yet.
PostFormat is shown if the suggested PostFormat is different from the current one.

How to test

Create a new post.
Add a block image (doesn't need to have any actual image).
Click Publish.
The expected behavior is that the pre-publish panel includes both the Tags and the PostFormat components to prompt the user to add them. Both should be collapsed. See:

Closed	Opened

Test variations of the above: with the document sidebar closed (as this is when tags are fetched), with tags and image as the current post format, etc. The suggested post format is shown below the current post format, if there is any suggestion - typing wp.data.select('core/editor').getSuggestedPostFormat() in the console will give you the same result (or null if none is to be suggested).

TODO

Make text translatable.
Try a more conversational post format suggestion (see Use Pre-Publish Panel for Tags and Post Format #7426 (comment))
Not shown conditions:
- Post already has some tag applied.
- It's a page, not a post (more generally, if the post type allows tags).
Fix:
- Don't crash if tags weren't fetched yet.
- Panel is hidden after adding one tag in the pre-publish panel.

Open questions

Should we suggest only tags or any available taxonomy? We're going to go with tags as a starter.

oandregal · 2018-07-26T17:45:17Z

To the open question regarding whether to include only tags or all available taxonomies in the PrePublishPanel: I lean towards adding any taxonomy that has at least one term (is there any other heuristic we can use to infer which taxonomies are relevant for a given site?).

chrisvanpatten · 2018-07-26T19:00:42Z

To the open question regarding whether to include only tags or all available taxonomies in the PrePublishPanel: I lean towards adding any taxonomy that has at least one term (is there any other heuristic we can use to infer which taxonomies are relevant for a given site?)

I would just make it tags or categories, but nothing else by default. Perhaps make it filterable so developers can opt their taxonomies in, but I think it's dangerous to assume all other taxonomies are relevant or worthy of a notification.

sarahmonster · 2018-07-31T08:08:56Z

I think the original concept here (cc @mtias) was to show the tags module only when no tags have been added to the post—so it wouldn't appear at all times, but it's there to serve as a reminder if the user has forgotten to add any tags.

I’m not sure categories are as relevant to the modern #hashtag web—and I suspect people are often unsure of the difference between the two—so I think starting with tags would be sensible. (I see a lot of people who don't even use categories, but I'm not sure if that's because the categories are less surfaced in the UI or because they're just an advanced feature.)

The ideal would probably be if we suggested some tags based on the post content and/or the users’ commonly used tags, but that’s obviously a bit more complicated to implement. Failing that, suggesting the most commonly-used tags (maybe three?) would be a good start.

I’m not certain if tag suggestions are better in the pre-publish panel or in the post-publish panel, since they aren't really critical to the pre-publish experience, per se. Before publishing, I want to make sure the date is right, check on anything that's going to automatically send out... basically make sure I have control over everything that happens "immediately." Adding tags can help users find your post, but ultimately it doesn't matter much if you add them before or after publishing the post.

That said, users are likely to be a bit more focussed at the pre-publish stage, and since we currently aren't bombarding them with information and Things to Check at this stage, having them here for the time being probably works. If this panel ends up being used heavily by plugins, we may want to consider shifting them to the post-publish panel in order to reduce noise and make publishing as frictionless as possible.

oandregal · 2018-08-01T17:16:06Z

@sarahmonster this is ready for design review.

cc @gziolo for code review.

gziolo · 2018-08-02T07:50:14Z

packages/editor/src/components/post-publish-panel/maybe-tags-panel.js

+	constructor( props ) {
+		super( props );
+		this.state = {
+			hadTags: props.hasTags,


Can you help me understand why it is not possible to set state only once on initial load and keep it this way for the rest of interaction with this panel? Checks like:

if ( currentHadTags === undefined && hasTags !== undefined ) {

indicate that it can be undefined for some reasons. If that is the reason, can we postpone mounting this component until this data is fetched?

That's explained it in the long getDerivedStateFromProps comment. TLDR is: tags may be not fetched yet at this point. How to test this behavior:

Create a new post.

Make sure the sidebar document hasn't been opened. If it is, collapse it, and reload the page.

Click the publish button. At this point, the pre-publish panel will be opened but the taxonomies aren't fetched yet (because the sidebar document is closed).

I'd love to get rid of this complexity, but Gutenberg fetching mechanisms are still fuzzy for me. Any suggestion about code examples I should look at for inspiration and learn-up?

Found a way to do this by using the ifCondition HOC component suggested by Riad, see e7dde0f.

youknowriad · 2018-08-02T09:17:25Z

packages/editor/src/components/post-publish-panel/maybe-tags-panel.js

+		const tags = select( 'core' ).getTaxonomy( 'post_tag' );
+		const terms = tags ? select( 'core/editor' ).getEditedPostAttribute( tags.rest_base ) : [];
+		return {
+			hasTags: tags && terms && terms.length > 0,


Can't this condition be something like that instead:

const tagTaxonomy = select( 'core' ).getTaxonomy( 'post_tag' ); const shouldShowMoreTags = tagTaxonomy && ( ! select( 'core/editor' ).getEditedPostAttribute( tags.rest_base ) || select( 'core/editor' ).getEditedPostAttribute( tags.rest_base ).length ===0 ) && tagTaxonomy.types.some( ( type ) => type === postType );

Something in that vein?

And I guess we can leverage the ifCondition HigherOrderComponent to avoid the MaybeTagsPanel component entirely?

Thanks for the ifCondition suggestion: I've used that in e66d09d to prevent the component from mounting if some conditions aren't met.

In this case, the internal component state is useful: we need a place to encode whether the post had tags when the user clicked publish. We can't use the hasTags prop because it'll become true if the user adds a new tag in the pre-publish panel, causing a re-render that hides the tags component, and so preventing the user from adding more than one tag.

I've also refactored the logic to make the assumptions more explicit.

Oh ok, I get it now and in that case, I think you were right originally and now it can fail if the taxonomy is not loaded initially (in theory, because in practice, it's already there).

But I'm wondering if the hasTags condition makes sense at all, why can't we just always show the tags there if the post supports it? It seems better for me even if I already entered tags to check them on pre-publish. It also makes the code way simpler.

Oh ok, I get it now and in that case, I think you were right originally and now it can fail if the taxonomy is not loaded initially (in theory, because in practice, it's already there).

It's still correct. Note that if the taxonomy is not loaded (which do happen when the user clicks the publish button without the sidebar being opened first), the areTagsFetched will be false. e66d09d decoupled two separate concerns that were previously encoded in only one variable: whether or not the tags taxonomy has been fetched (areTagsFetched) and if the post has tags (hasTags).

It seems better for me even if I already entered tags to check them on pre-publish. It also makes the code way simpler.

That's a design decision that I defer to Matías and Sarah. It seems sensible to limit the amount of info in the pre-publish panel - we don't want to have here info that's already filled in the sidebar by the user.

oandregal · 2018-08-02T09:50:31Z

I was thinking of adding the PostFormat component in a different PR but figured it'd better to include it here as well - see cd57766. It helps to review the design changes holistically (things like whether the panels should be opened/closed, the space they take, and so on). I also hope this minimizes code review time.

gziolo · 2018-08-02T11:38:29Z

packages/editor/src/components/post-publish-panel/prepublish.js

@@ -41,6 +43,8 @@ function PostPublishPanelPrepublish( {
 					] }>
 						<PostSchedule />
 					</PanelBody>
+					<MaybePostFormatPanel />


Have you checked how <PostSchedule / or <PostVisibility /> are implemented?

https://github.com/WordPress/gutenberg/tree/master/packages/editor/src/components/post-schedule
https://github.com/WordPress/gutenberg/tree/master/packages/editor/src/components/post-visibility

They contain two parts the components which render the panel's body and a wrapper component with suffix Check. It would be nice to align.

@youknowriad, why we don't wrap those components with checks in the publish panel? We do it in the sidebar:
https://github.com/WordPress/gutenberg/blob/master/edit-post/components/sidebar/post-schedule/index.js#L15-L35

Because it's already taken care of in the component itself. In the sidebar, we don't want the extra panels if the check is false, here, there's no extra wrappers added.

I can extract the ifCondition to a Check component if that's preferred. We use a variety of patterns through the codebase, though, so unless we want to consolidate towards using a particular one I'm happy as it is.

oandregal · 2018-08-02T15:51:08Z

Changes introduced in 46a181a

gave dashicon a size of 18 (it was 20 previously)
added the yellow color
updated copy

Before	After

oandregal · 2018-08-02T16:47:07Z

Changes in b4de407 make the post format suggestion more conversational:

Before	After

oandregal · 2018-08-02T17:00:58Z

7ffe43a, 06f2b7f and 73b6f3b update the copy text:

Text below the title was consolidated to complement why that's important in both blocks.
Format post type: due to translations corcerns, call to action and suggested post format should be different chunks of text (not mixed).

Before	After

sarahmonster

I know I suggested it, but I'm definitely not crazy about the yellow lightbulb. The yellow is way too much, and the icon feels a bit awkward. We could try replicating the tips that are used elsewhere (ie, a blue dot), but I think that may be a bit confusing. Let's just nix the icon altogether for now.

Your readers will be able to find your posts more easily.

Suggestion:

This will help readers find your posts.

I also don't seem to be able to get the post format suggestion to display:

PostFormatSuggested@http://localhost:8888/wp-content/plugins/gutenberg/build/editor/index.js?ver=1533257225:23894:1
yh@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:95:430
lg@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:120:88
mg@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:120:386
gc@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:127:202
vb@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:126:230
ub@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:126:65
wg@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:137:182
Ze@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:41:24
EventListener.handleEvent*p@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:40:392
W@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:49:377
qf@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:52:256
zh@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:108:355
kg@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:118:263
lg@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:120:121
mg@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:120:386
gc@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:127:202
vb@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:126:230
ub@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:126:65
zd@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:124:449
ra@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:123:319
enqueueSetState@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.82e21c65.js:189:231
q.prototype.setState@http://localhost:8888/wp-content/plugins/gutenberg/vendor/react.min.ab6b06d4.js:18:428
subscribe/this.unsubscribe<@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:7467:13
globalListener/<@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:8027:14
globalListener@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:8026:5
registerReducer/<@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:8068:9
dispatch@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:6065:7
dispatch@http://localhost:8888/wp-admin/post-new.php:1:38397
_callee2$@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:8217:21
tryCatch@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:6338:37
invoke@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:6564:22
defineIteratorMethods/</prototype[method]@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:6390:16
asyncGeneratorStep@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:437:16
_next@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:459:9
run@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:3228:22
notify/<@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:3245:30
flush@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:1938:9
MutationCallback*./node_modules/core-js/library/modules/_microtask.js/module.exports@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:1957:5
./node_modules/core-js/library/modules/es6.promise.js@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:3166:17
__webpack_require__@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:21:12
./node_modules/core-js/library/fn/promise.js@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:1037:1
__webpack_require__@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:21:12
./node_modules/@babel/runtime/core-js/promise.js@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:203:18
__webpack_require__@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:21:12
./node_modules/@babel/runtime/helpers/asyncToGenerator.js@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:433:16
__webpack_require__@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:21:12
./packages/data/build-module/registry.js@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:7828:97
__webpack_require__@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:21:12
./packages/data/build-module/default-registry.js@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:7503:67
__webpack_require__@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:21:12
./packages/data/build-module/index.js@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:7648:75
__webpack_require__@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:21:12
this.wp.data<@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:70:18
@http://localhost:8888/wp-content/plugins/gutenberg/build/data/index.js?ver=1533257225:2:11

Side note: this looks pretty busy right now, but I think could be improved a great deal by merging #7879, which seems to have gotten stuck in limbo. I'll look into pushing that change through in a more incremental way.

oandregal · 2018-08-03T09:15:24Z

Changes in b379a78, 0fcc49e, and 1f20a64

Before	After

michelleweber · 2018-08-03T09:49:02Z

On the tags, I'd love to add just a few more words that add detail on the what/why of tags:

Enter a few words that describe your post's subject to help interested readers find it.

And little tweaks on post format:

Your theme uses post formats -- styling tweaks that complement different kinds of content. The Image format would be great for this post.

oandregal · 2018-08-03T10:02:28Z

Implemented Michelle's suggestions at c29f3b0

oandregal · 2018-08-13T10:11:26Z

@sarahmonster @tofumatt can we 🚢 this or is there anything else we should address?

tofumatt · 2018-08-13T10:23:05Z

ah, sorry, if you could re-request me for review once things are ready to check again I'll see it in my review queue and have a look. I didn't realise it was ready.

tofumatt

Thanks for the changes and sorry for all the back-and-forth on this. I dig it! 👍

sarahmonster · 2018-08-13T13:57:50Z

Thanks for all your patience with this, @nosolosw! This looks good by me now. 🚢 !

oandregal · 2018-08-17T11:56:10Z

packages/editor/src/components/post-publish-panel/maybe-tags-panel.js

+		return {
+			areTagsFetched: tagsTaxonomy !== undefined,
+			isPostTypeSupported: tagsTaxonomy && tagsTaxonomy.types.some( ( type ) => type === postType ),
+			hasTags: tagsTaxonomy && select( 'core/editor' ).getEditedPostAttribute( tagsTaxonomy.rest_base ).length,


I learned that pages don't have taxonomies the hard way #9082

afercia · 2018-08-27T16:12:02Z

packages/editor/src/components/post-publish-panel/maybe-post-format-panel.js

+					onUpdatePostFormat={ onUpdatePostFormat }
+					suggestedPostFormat={ suggestion.id }
+					suggestionText={ sprintf(
+						__( 'Apply the "%1$s" format.' ),


This misses a translators comment. Will try to address it in #9363

oandregal self-assigned this Jul 26, 2018

oandregal added the [Status] In Progress Tracking issues with work in progress label Jul 26, 2018

oandregal mentioned this pull request Jul 26, 2018

Use Pre-Publish Panel for Tags and Post Format #7426

Closed

oandregal changed the title ~~Suggest adding tags in PrePublishPanel~~ Suggest adding tags in PrePublishPanel is no tag was added Jul 26, 2018

oandregal requested a review from sarahmonster July 26, 2018 17:46

oandregal force-pushed the add/tags-to-prepublishpanel branch 2 times, most recently from 47be367 to e927c9a Compare August 1, 2018 16:04

oandregal removed the [Status] In Progress Tracking issues with work in progress label Aug 1, 2018

oandregal requested a review from gziolo August 1, 2018 17:16

gziolo requested review from a team and youknowriad August 2, 2018 07:36

gziolo reviewed Aug 2, 2018

View reviewed changes

youknowriad reviewed Aug 2, 2018

View reviewed changes

oandregal force-pushed the add/tags-to-prepublishpanel branch from 860ea4a to 09b8f23 Compare August 2, 2018 09:48

oandregal changed the title ~~Suggest adding tags in PrePublishPanel is no tag was added~~ PrePublishPanel: suggest tags and postformat Aug 2, 2018

gziolo reviewed Aug 2, 2018

View reviewed changes

sarahmonster added the Needs Copy Review Needs review of user-facing copy (language, phrasing) label Aug 3, 2018

sarahmonster requested changes Aug 3, 2018

View reviewed changes

sarahmonster mentioned this pull request Aug 3, 2018

Improve pre-publish panel styling #7879

Merged

4 tasks

oandregal and others added 16 commits August 13, 2018 11:29

Make a more conversational post format

f8d386d

Update text copy

ab0bf56

Update copy text

9ef07b3

Update copy text for transations

b2073e8

Remove icons

603c7a1

Improve copy text for tags panel

e40d773

Add section title

620dbbd

Update copy

1baa309

chore: Make some tweaks to copy

053038d

Shorten hasTags check

5d18b23

Update to use suggestion

ed42917

Refactor logic for get suggestion

52ab3d3

Update copy

e230479

Update copy

713d5a6

chore: Use clearer suggeston text

c1ac898

Refactor for clarity

afffd27

oandregal force-pushed the add/tags-to-prepublishpanel branch from 0bcaa57 to afffd27 Compare August 13, 2018 09:48

tofumatt self-requested a review August 13, 2018 10:23

tofumatt removed the Needs Copy Review Needs review of user-facing copy (language, phrasing) label Aug 13, 2018

chore: Tweak name of component for readability

dc3d61a

tofumatt approved these changes Aug 13, 2018

View reviewed changes

sarahmonster approved these changes Aug 13, 2018

View reviewed changes

oandregal merged commit 28e00cd into master Aug 13, 2018

oandregal deleted the add/tags-to-prepublishpanel branch August 13, 2018 14:55

mtias added this to the 3.6 milestone Aug 16, 2018

oandregal commented Aug 17, 2018

View reviewed changes

afercia reviewed Aug 27, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PrePublishPanel: suggest tags and postformat #8235

PrePublishPanel: suggest tags and postformat #8235

oandregal commented Jul 26, 2018 •

edited

Loading

oandregal commented Jul 26, 2018

chrisvanpatten commented Jul 26, 2018

sarahmonster commented Jul 31, 2018

oandregal commented Aug 1, 2018

gziolo Aug 2, 2018

oandregal Aug 2, 2018 •

edited

Loading

oandregal Aug 2, 2018

youknowriad Aug 2, 2018

oandregal Aug 2, 2018

youknowriad Aug 2, 2018

oandregal Aug 2, 2018

oandregal commented Aug 2, 2018 •

edited

Loading

gziolo Aug 2, 2018

youknowriad Aug 2, 2018

oandregal Aug 2, 2018

oandregal commented Aug 2, 2018

oandregal commented Aug 2, 2018 •

edited

Loading

oandregal commented Aug 2, 2018 •

edited

Loading

sarahmonster left a comment •

edited

Loading

oandregal commented Aug 3, 2018

michelleweber commented Aug 3, 2018

oandregal commented Aug 3, 2018

oandregal commented Aug 13, 2018

tofumatt commented Aug 13, 2018

tofumatt left a comment

sarahmonster commented Aug 13, 2018

oandregal Aug 17, 2018

afercia Aug 27, 2018

PrePublishPanel: suggest tags and postformat #8235

PrePublishPanel: suggest tags and postformat #8235

Conversation

oandregal commented Jul 26, 2018 • edited Loading

How to test

TODO

Open questions

oandregal commented Jul 26, 2018

chrisvanpatten commented Jul 26, 2018

sarahmonster commented Jul 31, 2018

oandregal commented Aug 1, 2018

Choose a reason for hiding this comment

oandregal Aug 2, 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

oandregal commented Aug 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal commented Aug 2, 2018

oandregal commented Aug 2, 2018 • edited Loading

oandregal commented Aug 2, 2018 • edited Loading

sarahmonster left a comment • edited Loading

Choose a reason for hiding this comment

oandregal commented Aug 3, 2018

michelleweber commented Aug 3, 2018

oandregal commented Aug 3, 2018

oandregal commented Aug 13, 2018

tofumatt commented Aug 13, 2018

tofumatt left a comment

Choose a reason for hiding this comment

sarahmonster commented Aug 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal commented Jul 26, 2018 •

edited

Loading

oandregal Aug 2, 2018 •

edited

Loading

oandregal commented Aug 2, 2018 •

edited

Loading

oandregal commented Aug 2, 2018 •

edited

Loading

oandregal commented Aug 2, 2018 •

edited

Loading

sarahmonster left a comment •

edited

Loading