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.

Sign up for GitHub

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

Jump to bottom

Display correct custom taxonomy labels #10962

Merged
merged 1 commit into from
Oct 31, 2018
Merged

Display correct custom taxonomy labels #10962

merged 1 commit into from
Oct 31, 2018

Conversation

earnjam
Copy link
Contributor

@earnjam earnjam commented Oct 23, 2018

Description

The check for taxonomy labels supplied by the REST API used an incorrect property path, so was always falling back to term for custom taxonomies.

Fixes #8448

Tested with a custom taxonomy called "Models" supplying it's own labels.

Flat

screen shot 2018-10-23 at 1 38 25 pm

screen shot 2018-10-23 at 1 38 35 pm

Hierarchical

screen shot 2018-10-23 at 2 04 59 pm

@earnjam earnjam added the REST API Interaction Related to REST API label Oct 23, 2018
@danielbachhuber
Copy link
Member

@earnjam Do you want to add some e2e tests for this so it doesn't happen again?

@earnjam
Copy link
Contributor Author

earnjam commented Oct 23, 2018

Good call. I'll work on adding a few to this PR.

Interesting thing I found in this logic is that we have 2 fallbacks for taxonomy labels.

  • One is on the core side when the taxonomy is registered. The labels will fall back to "tag" for flat taxonomies, and "category" for hierarchical. This means labels should always be returned in the REST API.
  • The other is on the JS side here and it falls back to "term" regardless of taxonomy type if it doesn't have labels.

Should these be consistent? I need to look into it more and see if there is a situation where a label ever wouldn't be provided by the REST API.

@danielbachhuber
Copy link
Member

I need to look into it more and see if there is a situation where a label ever wouldn't be provided by the REST API.

If the request were filtered with ?_fields=, then conceivably the labels could be missing. I don't think this is something we need to protect against, but the fallback labels are fine by me.

@danielbachhuber danielbachhuber added the [Type] Bug An existing feature does not function as intended label Oct 23, 2018
@desrosj desrosj added this to the WordPress 5.0 milestone Oct 25, 2018
@danielbachhuber danielbachhuber modified the milestones: WordPress 5.0, 4.3 Oct 31, 2018
@danielbachhuber danielbachhuber self-requested a review October 31, 2018 13:36
@danielbachhuber
Copy link
Member

Do you want to add some e2e tests for this so it doesn't happen again?

While I'd love unit tests, I'd also simply love for this to be fixed. We can follow up with e2e tests at a later date.

@danielbachhuber danielbachhuber merged commit 3503c0d into master Oct 31, 2018
@danielbachhuber danielbachhuber deleted the 8448-custom-tax-term-labels branch October 31, 2018 13:38
@danielbachhuber danielbachhuber mentioned this pull request Oct 31, 2018
Closed
daniloercoli added a commit that referenced this pull request Oct 31, 2018
@daniloercoli
Merge branch 'master' of https://github.com/WordPress/gutenberg into …
4d25e41 
…rnmobile/port-quote-block-step-1

* 'master' of https://github.com/WordPress/gutenberg: (21 commits)
  Fix property path on get() call (#10962)
  Fixed typos on block api documentation (#11298)
  Export `switchToBlockType` to be used mobile side when merging two blocks. (#11294)
  RichText: Remove unused `ref` assignment to RichText (#11222)
  Remove findDOMNode from Tooltip component (#11169)
  Components: Remove redundant onClickOutside handler from Dropdown (#11253)
  added myself to the contributors list (#11260)
  Add complete post type labels for Resuable Blocks (#11278)
  Increase specificity for active radio/checkbox input styling (#11290)
  Fixed "artifact" misspelling in docs. (#11291)
  Nux package: fix incorrect named deprecated import (#11283)
  Rename parentClientId to rootClientId for consistency (#11274)
  chore(release): update changelog files
  chore(release): publish
  Update plugin version to 4.2.0. (#11258)
  Data: Use turbo-combine-reducers in place of Redux (#11255)
  Revert using Icon in IconButton to avoid regression in plugin icons (pinned icons) (#11256)
  Block List: Use default Inserter for sibling insertion (#11018)
  Editor: Optimize Inserter props generation and reconciliation (#11243)
  RichText: fix format placeholder (#11102)
  ...

# Conflicts:
#	packages/block-library/src/quote/index.js
@youknowriad youknowriad mentioned this pull request Nov 3, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielbachhuber danielbachhuber danielbachhuber approved these changes

Assignees

@earnjam earnjam

Labels
REST API Interaction Related to REST API [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.
3 participants
@earnjam @danielbachhuber @desrosj