-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Terms List block: Add Categories-specific variation #65434
Conversation
name: 'terms', | ||
title: __( 'Terms List' ), | ||
icon, | ||
attributes: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you add isDefault: true
to this variation, then it will resolve the issue with double entries in the inserter. That should make it quite a compelling solution to the problem I like how you handled the Categories vs the rest case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! TIL 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the name for the property should be phrased differently, but the intent is to have a simple way to change the default version of the block in the UI through the variation, instead of using WP filters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const taxonomies = allTaxonomies?.filter( | ||
( t ) => | ||
t.visibility.public && | ||
( taxonomySlug === 'category' ) === ( t.slug === 'category' ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the risk if the `category is listed here? When switching a choice a different block variation would get selected but otherwise it should remain seamless for the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, @fabiankaegy also suggested that over here. I'll quote my reply here for convenience:
TBH, that would feel inconsistent to me in terms of UX; I think it would be somewhat surprising to the user if selecting "Categories" as the taxonomy from that dropdown gets special treatment, while all other taxonomies don't 😕
I'm open to revisiting this. It's something I'd like to get feedback from designers on, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tentatively applied this change in 9f70db6. We can revert, depending on the feedback we get.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Size Change: +89 B (+0.01%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either way it's a nice improvement that ensures that folks familiar with the Categories block will be able to continue using it as before. For the rest, Terms List will serve as a great way to pick a proper taxonomy type.
I'll leave the decision about including the categories option in the combo box to other contributors. It's also an easy change to apply later based on the feedback from users.
Flaky tests detected in 9f70db6. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10918171883
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the follow up :)
Well, this has two approvals, and seems to remove some confusion around where the old Categories List block went without any major negative side effects, so I'm going to merge this. Thanks @gziolo and @fabiankaegy! I'm starting to wonder if at some point it'll make sense to really introduce one variation per taxonomy -- I think the main counter-argument against #64805 was the "Transform to" dropdown, but I've since learned that we can remove that by excluding |
This was also my thinking that we don’t have to show the transform combo box by removing the scope from variations. |
What?
Add two variations to the Terms List block (i.e.
core/categories
and named "Categories List" prior to #65272): One for Categories, and another one for all other taxonomies.Why?
Mostly for better discoverability of what used to be the Categories List block under its new name. For more background, please refer to this thread.
How?
By creating two block variations.
Drawbacks
Also discussed in this thread.
Alternatives
One alternative would be to create a block variation for each individual taxonomy. This has been explored in #64805. (That PR is now closed, but it can be reopened anytime.)
Testing Instructions
TBD, but largely similar to #65272 or #64805.
Screenshots or screencast