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.

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

Patterns: Reveal the Featured pattern category, and make it the first in the list #49907

Merged
merged 6 commits into from
Feb 15, 2021

Conversation

andrewserong
Copy link
Member

Changes proposed in this Pull Request

  • Reveal the Featured pattern category in the pattern inserter
  • Unregister existing pattern categories, and re-register them along with the pattern categories we're registering in the ETK plugin, so that they're all sorted alphabetically
  • Once sorted alphabetically by slug, move the Featured category to be the first in the list

Screenshot

featured-category-pattern-inserter

Testing instructions

  • Apply this change in your sandbox, and test the pattern inserter. Are the pattern categories working correctly and sorted correctly?
  • What about if we have the Twenty Twenty One theme applied (or any theme with its own patterns), does the sorting hold up (I haven't tested this yet)
  • What about in another language, is the sorting correct? (We might need to adjust to sort by the label instead of the slug 🤔

Related to 120-gh-Automattic/view-design
CC: @joanrho

@matticbot
Copy link
Contributor

@andrewserong andrewserong requested a review from a team February 10, 2021 06:52
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Feb 10, 2021
@andrewserong andrewserong self-assigned this Feb 10, 2021
@andrewserong andrewserong added the [Feature] Block Patterns Pattern content itself, and the functionality that lets you create patterns. label Feb 10, 2021
@matticbot
Copy link
Contributor

matticbot commented Feb 10, 2021

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@matticbot
Copy link
Contributor

Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D56791-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing the Editing Toolkit Plugin for more info: PCYsg-ly5-p2

@glendaviesnz
Copy link
Contributor

This tested as advertised for me with english, but the sorting is out in Spanish:

Screen Shot 2021-02-11 at 1 13 16 PM

@andrewserong
Copy link
Member Author

Thanks for testing @glendaviesnz! I've updated it to sort by the label instead of the slug, and to be case insensitive just in case there are strings with the wrong case (like we have for contacto in Spanish, which should probably be Contacto?). Fixing the translation is a separate issue, but at least we can make sure it's in the correct order in this PR 🙂

image

Copy link
Contributor

@glendaviesnz glendaviesnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me - over to you if you want to explore the union operator option mentioned

@andrewserong andrewserong force-pushed the update/switch-on-featured-category-for-patterns branch from e97e7ac to cd4fe68 Compare February 15, 2021 06:23
@andrewserong andrewserong merged commit 01ab92f into trunk Feb 15, 2021
@andrewserong andrewserong deleted the update/switch-on-featured-category-for-patterns branch February 15, 2021 23:14
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block Patterns Pattern content itself, and the functionality that lets you create patterns.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants