Editor: Use TermTokenField to edit non-hierarchical CPT taxonomies #6348

nylen · 2016-06-27T23:09:15Z

This PR adds support for fetching and editing non-hierarchical terms using a TokenField. Adds a new TermTokenField component which is modeled after post-editor/editor-tags/index.jsx.

Planned future steps:

After Editor: Manage post title edits using global Redux state #5445 lands connecting the edited post to Redux state, get rid of the postTerms property in favor of getEditedPostValue( ..., 'terms' )
Use reduxified TermTreeSelector and TermTokenField in EditorCategoriesTagsAccordion to edit categories and tags, respectively. (This is why I renamed EditorTaxonomiesAccordion to EditorCategoriesTagsAccordion - I expect the logic there will still be used, but to render only categories and tags rather than all taxonomies.)
Use EditorDrawerTaxonomies to render all taxonomies, including tags and categories if present
Get rid of old code to edit categories and tags, including TagListData and CategoryListData components

To test

Edit a post of a custom type that has one or more non-hierarchical taxonomies. Ensure that existing taxonomies are loaded correctly in the TokenField:

values (terms assigned to the saved post)
suggestions (all terms from the site)

Ensure that terms can be updated and saved as expected.

Test live: https://calypso.live/?branch=update/editor/cpt-term-token-field

timmyc · 2016-06-27T23:53:08Z

client/post-editor/term-token-field/index.jsx

+				</span>
+				<QueryTerms
+					siteId={ this.props.siteId }
+					taxonomy={ this.props.taxonomyName }


QueryTerms uses the default number of 100 per page for a query. I think in the old <TokenField/> looks like we are fetching 1,000 tags in the first request. Do we want to make a larger request here too?

How does paging for these query components usally work? Will it initiate another page on its own, or is there an easy way to do that?

If not, then I agree we should go ahead and fetch 1,000 tags.

Looking through the code some more, it doesn't look like it. Pushed 0532210 to address this. I'm having trouble fully testing the fix due to /me/sites timing out but will try again tomorrow.

How does paging for these query components usally work?

Sorry missed this question earlier. Pagination so far has been implemented using react-virtualized/VirtualScroll seen here in conjunction with the 🔮 of QueryManager.

Currently we don't do any pagination in the TokenField correct? So I do think grabbing the large number of tags up-front is the way to go.

timmyc · 2016-06-28T00:19:57Z

Confirmed that 100 tags are being loaded, I have up to "Tag100" and a few other random ones and not all of the Tag9x are showing up:

Guessing we want to do 1,000 just like post-tags, but not sure. Couple of other minor items above but looking great!

Use reduxified TermTreeSelector

I have that working locally and would be happy to add to this branch or wait for a follow-up PR.

nylen · 2016-06-28T00:58:32Z

Fine with me if you wanna push stuff up here too, it's one less link in the chain of PRs we have to wait on.

timmyc · 2016-06-28T19:09:56Z

client/post-editor/term-token-field/index.jsx

+	render() {
+		const termNames = ( this.props.terms || [] ).map( term => term.name );
+
+		const query = {


Since this query is static, what do you think about putting it in TermsConstants in-lieu of a new const on each render?

Good idea, this also allowed me to use the query in the existing EditorTags component. Done in rebased 53a571b.

timmyc · 2016-06-28T19:15:01Z

I'll follow up with another PR after this one lands. This is looking good to me. Left a few small comments above, but fine to merge as-is, or you can make changes if you please.

This component will be used to render categories and/or tags, if supported by the current post type. The more generic EditorDrawerTaxonomies will render all taxonomies.

nylen · 2016-06-28T21:09:44Z

client/lib/terms/constants.js

+	number: module.exports.MAX_TAGS,
+	order_by: 'count',
+	order: 'DESC'
+};


I considered bringing this file up to date with ES6 but didn't want to also change the existing imports and usage in this PR.

Doesn't matter much here since I suspect this is a file we'll seek to eliminate altogether, but in general I don't think it's a big deal to mix import / export syntaxes. If anything, the eyesore will hasten follow-up 😄

aduth · 2016-07-06T12:41:56Z

client/post-editor/term-token-field/index.jsx

+	}
+
+	render() {
+		const termNames = ( this.props.terms || [] ).map( term => term.name );


Maybe I'm a bit Lodash obsessed, but the Lodash map implementation could simplify this a bit, removing the need for fallback array and callback function:

const termNames = map( this.props.terms, 'name' );

:) I'll incorporate this into #6548.

nylen added [Feature] Post/Page Editor The editor for editing posts and pages. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Custom Post Types (CPT) labels Jun 27, 2016

nylen self-assigned this Jun 27, 2016

timmyc reviewed Jun 27, 2016
View reviewed changes

timmyc added [Status] Needs Author Reply and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jun 28, 2016

nylen added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Author Reply labels Jun 28, 2016

timmyc reviewed Jun 28, 2016
View reviewed changes

timmyc added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jun 28, 2016

nylen added 2 commits June 28, 2016 16:08

Rename EditorTaxonomiesAccordion to EditorCategoriesTagsAccordion

b77a72b

This component will be used to render categories and/or tags, if supported by the current post type. The more generic EditorDrawerTaxonomies will render all taxonomies.

Use new <TermTokenField /> to edit non-hierarchical CPT taxonomies

53a571b

nylen force-pushed the update/editor/cpt-term-token-field branch from 0532210 to 53a571b Compare June 28, 2016 21:08

nylen reviewed Jun 28, 2016
View reviewed changes

nylen merged commit d98ba96 into master Jun 30, 2016

nylen deleted the update/editor/cpt-term-token-field branch June 30, 2016 17:45

matticbot removed the [Status] Ready to Merge label Jun 30, 2016

aduth reviewed Jul 6, 2016
View reviewed changes

aduth mentioned this pull request Aug 8, 2016

Offline: duplicate post-edit-store flux actions in placeholder redux actions #2993

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Editor: Use TermTokenField to edit non-hierarchical CPT taxonomies #6348

Editor: Use TermTokenField to edit non-hierarchical CPT taxonomies #6348

nylen commented Jun 27, 2016 •

edited by matticbot

Loading

timmyc Jun 27, 2016

nylen Jun 28, 2016

nylen Jun 28, 2016

timmyc Jun 28, 2016

timmyc commented Jun 28, 2016

nylen commented Jun 28, 2016

timmyc Jun 28, 2016

nylen Jun 28, 2016

timmyc commented Jun 28, 2016

nylen Jun 28, 2016

aduth Jun 28, 2016

aduth Jul 6, 2016

nylen Jul 6, 2016

Editor: Use TermTokenField to edit non-hierarchical CPT taxonomies #6348

Editor: Use TermTokenField to edit non-hierarchical CPT taxonomies #6348

Conversation

nylen commented Jun 27, 2016 • edited by matticbot Loading

To test

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

timmyc commented Jun 28, 2016

nylen commented Jun 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timmyc commented Jun 28, 2016

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

nylen commented Jun 27, 2016 •

edited by matticbot

Loading