-
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
Categories List block: Add dropdown for taxonomies #65272
Conversation
Size Change: +1.75 kB (+0.1%) 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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @MikeB8s, @sophiegyo. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
This was the implementation I envisioned when I answered to #33302 a few years ago. Glad to see this pushed forward. I really think this is the good way to do it! Thanks for the work on this! |
@@ -2,11 +2,15 @@ | |||
"$schema": "https://schemas.wp.org/trunk/block.json", | |||
"apiVersion": 3, | |||
"name": "core/categories", | |||
"title": "Categories List", | |||
"title": "Terms List", |
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.
Given that we are changing the name of the block here I would recommend we at least add Categories
to the keywords
array so it still appears in search.
My personal recommendation however would be that for backwards compatibility we register one singular block variation for the category taxonomy. This way we are not renaming an existing block. Users still get the same result. But they now have an additional control to change the block to query other taxonomies.
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.
Given that we are changing the name of the block here I would recommend we at least add
Categories
to thekeywords
array so it still appears in search.
Good point 👍
My personal recommendation however would be that for backwards compatibility we register one singular block variation for the category taxonomy. This way we are not renaming an existing block. Users still get the same result. But they now have an additional control to change the block to query other taxonomies.
Interesting. I gave this a quick spin (see below diff). I'm assuming you mean with scope
set to exclude transform
, so that there won't be the dropdown to transform variations (as there is in #64805) as it'd be otherwise confusing to have two dropdowns to control which variation to display.
This posed the following problem: For back-compat, we need to keep category
as the default value for the taxonomy
attribute, so that any existing occurrences of <!-- wp:categories /-->
will continue to be rendered as the Categories List variation.
But that raises the question of the default value for that attribute when inserting a new instance of the generic Terms List variation. It cannot be omitted, as that would translate to category
(per the above), causing the block to be promptly rendered as the Categories List variation.
So we need a different default value for the Terms List variation. I guess Tags -- the only other built-in and public taxonomy -- is the logical contender here, so this is what I chose.
Furthermore, there's the question of the dropdown. IMO, it needs to be hidden altogether in case of the Categories List variation, as it would otherwise allow to transform it to the Terms List variation. In the same vein, the "Categories" option needs to be removed from the dropdown in case of the Terms List variation, to prevent transforming into the Categories List variation.
But that means that the Terms List block is now solely for Tags and custom taxonomies, whereas Categories get their own variation (which looks like a separate block, for all intents and purposed). This feels a bit arbitrary IMO; I'm not sure it's that much better in terms of UX than not having the separate Categories List block variation.
Diff
diff --git a/packages/block-library/src/categories/edit.js b/packages/block-library/src/categories/edit.js
index 866feb884c8..bafb2a3273a 100644
--- a/packages/block-library/src/categories/edit.js
+++ b/packages/block-library/src/categories/edit.js
@@ -46,7 +46,11 @@ export default function CategoriesEdit( {
'taxonomy'
);
- const taxonomies = allTaxonomies?.filter( ( t ) => t.visibility.public );
+ const taxonomies = allTaxonomies?.filter(
+ ( t ) =>
+ t.visibility.public &&
+ ( taxonomySlug === 'category' ) === ( t.slug === 'category' )
+ );
const taxonomy = taxonomies?.find( ( t ) => t.slug === taxonomySlug );
@@ -185,21 +189,24 @@ export default function CategoriesEdit( {
<TagName { ...blockProps }>
<InspectorControls>
<PanelBody title={ __( 'Settings' ) }>
- { Array.isArray( taxonomies ) && (
- <SelectControl
- __nextHasNoMarginBottom
- __next40pxDefaultSize
- label={ __( 'Taxonomy' ) }
- options={ taxonomies.map( ( t ) => ( {
- label: t.name,
- value: t.slug,
- } ) ) }
- value={ taxonomySlug }
- onChange={ ( selectedTaxonomy ) =>
- setAttributes( { taxonomy: selectedTaxonomy } )
- }
- />
- ) }
+ { taxonomySlug !== 'category' && // For back-compat, the Category taxonomy has its own block variation.
+ Array.isArray( taxonomies ) && (
+ <SelectControl
+ __nextHasNoMarginBottom
+ __next40pxDefaultSize
+ label={ __( 'Taxonomy' ) }
+ options={ taxonomies.map( ( t ) => ( {
+ label: t.name,
+ value: t.slug,
+ } ) ) }
+ value={ taxonomySlug }
+ onChange={ ( selectedTaxonomy ) =>
+ setAttributes( {
+ taxonomy: selectedTaxonomy,
+ } )
+ }
+ />
+ ) }
<ToggleControl
__nextHasNoMarginBottom
label={ __( 'Display as dropdown' ) }
diff --git a/packages/block-library/src/categories/index.js b/packages/block-library/src/categories/index.js
index 8cdcad45086..d30c55667d1 100644
--- a/packages/block-library/src/categories/index.js
+++ b/packages/block-library/src/categories/index.js
@@ -9,6 +9,7 @@ import { category as icon } from '@wordpress/icons';
import initBlock from '../utils/init-block';
import metadata from './block.json';
import edit from './edit';
+import variations from './variations';
const { name } = metadata;
@@ -18,6 +19,7 @@ export const settings = {
icon,
example: {},
edit,
+ variations,
};
export const init = () => initBlock( { name, metadata, settings } );
diff --git a/packages/block-library/src/categories/variations.js b/packages/block-library/src/categories/variations.js
new file mode 100644
index 00000000000..06003ec6420
--- /dev/null
+++ b/packages/block-library/src/categories/variations.js
@@ -0,0 +1,36 @@
+/**
+ * WordPress dependencies
+ */
+import { __ } from '@wordpress/i18n';
+import { category as icon } from '@wordpress/icons';
+
+const variations = [
+ {
+ name: 'terms',
+ title: __( 'Terms List' ),
+ icon,
+ attributes: {
+ // We need to set an attribute here that will be set when inserting the block.
+ // We cannot leave this empty, as that would be interpreted as the default value,
+ // which is `category` -- for which we're defining a distinct variation below,
+ // for backwards compatibility reasons.
+ // The logical fallback is thus the only other built-in and public taxonomy: Tags.
+ taxonomy: 'post_tag',
+ },
+ isActive: ( blockAttributes ) =>
+ // This variation is used for any taxonomy other than `category`.
+ blockAttributes.taxonomy !== 'category',
+ },
+ {
+ name: 'categories',
+ title: __( 'Categories List' ),
+ description: __( 'Display a list of all categories.' ),
+ icon,
+ attributes: {
+ taxonomy: 'category',
+ },
+ isActive: [ 'taxonomy' ],
+ },
+];
+
+export default variations;
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.
Looking at the diff I would not remove the category
option from the dropdown of available taxonomies. If a user happens to insert the "Term List" block and then selects the "Category" taxonomy in the drop-down I'd be fine with that changing the name of the block to the correct variation.
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.
At the same time I would be fine with users switching from the Category List to another term :)
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.
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 😕
If it's not too much of a dealbreaker for you, I'd rather consider the idea of a dedicated variation for the Categories List block separately -- i.e. in a follow-up PR, where it can also get the attention by designers that it deserves, without conflating it with the principal subject of this PR of adding the taxonomy
attribute and dropdown in the first place 😊
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!
I've added "keywords": [ "categories" ]
in 95a6cf3.
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.
It looks like block variations were discussed extensively in:
One drawback is that users won't see Categories in the inserters when typing categories
term. The icon remains the same, so maybe that isn't that much of an issue, as with the keywords
, it will remain discoverable. Although, we should follow the feedback closely during beta and RC phase of the release.
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.
That's a fair point and one that I hadn't considered; I've shared some more thoughts below.
I'll add one more note for posterity: With the suggested change we've been discussing in this thread -- i.e. adding two block variations, one for Categories specifically, and a "generic" one for all other cases -- there can also be some confusion in the inserter: I've called the "generic" variation "Terms List", just like the block itself; but since both the block itself and all its variations are shown in the inserter, there would be two "Terms List" blocks:
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 filed a draft PR for this regardless, in case we'd like to revisit this: #65434
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'll add one more note for posterity: With the suggested change we've been discussing in this thread -- i.e. adding two block variations, one for Categories specifically, and a "generic" one for all other cases -- there can also be some confusion in the inserter: I've called the "generic" variation "Terms List", just like the block itself; but since both the block itself and all its variations are shown in the inserter, there would be two "Terms List" blocks:
Turns out I was wrong about this one: #65434 (comment) 😅
@@ -92,8 +102,8 @@ function build_dropdown_script_block_core_categories( $dropdown_id ) { | |||
( function() { | |||
var dropdown = document.getElementById( '<?php echo esc_js( $dropdown_id ); ?>' ); | |||
function onCatChange() { | |||
if ( dropdown.options[ dropdown.selectedIndex ].value > 0 ) { | |||
location.href = "<?php echo esc_url( home_url() ); ?>/?cat=" + dropdown.options[ dropdown.selectedIndex ].value; | |||
if ( dropdown.options[ dropdown.selectedIndex ].value !== -1 ) { |
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.
There is an opportunity to refactor this block to use Interactivity API. @DAreRodz and @michalczaplinski, was it on the radar?
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.
It looks good to me and addresses long-standing issues. I left some minor feedback about the fallback message presented on the frontend when no terms are available for the specific type. Should there be a customized message printed instead of "No categories". The next step would be to digest feedback from #64805 and see how it can be further improved to remove the ambiguity between core/categories
and core/post-terms
.
Ah yeah, that's a bit unfortunate 😕 I hope that's not going to pose a UX issue, but that's something we can maybe get some feedback on during the next couple of weeks and fix accordingly. I think the suggestion was to merge the Tag cloud block (
Ah, good spot! Under the hood, we're using We should also change the message in the editor, as it's kind of misleading (and... wrong? 😅 ) |
Looping in some design and product folks (👋 @jasmussen and @annezazu) for their feedback on the following discovery that @gziolo made:
Note that the "Categories" block seen in the inserter in the above screenshot is There are two known approaches to mitigate this problem, but they each have their own drawbacks (discussed in the respective PRs)
|
$show_label = empty( $attributes['showLabel'] ) ? ' screen-reader-text' : ''; | ||
$default_label = $taxonomy->label; | ||
$label_text = ! empty( $attributes['label'] ) ? $attributes['label'] : $default_label; | ||
$wrapper_markup = '<div %1$s><label class="wp-block-categories__label' . $show_label . '" for="' . esc_attr( $id ) . '">' . $label_text . '</label>%2$s</div>'; |
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.
See comment from @peterwilsoncc WordPress/wordpress-develop#7226 (comment)
Escaping label text
It was a preexisting issue, though.
@sophiegyo mind confirming your WordPress.org username so I can ensure you're included in the 6.7 credits listing? |
@jeffpaul I've done the linking! It's |
@sophiegyo very much counts and thankful for every and all contribution, thank you! |
What?
Allow using the Categories List block for other taxonomies.
Alternative to #64805.
Why?
There's been some demand for a "Terms List" block that would render a list of existing taxonomy terms for a given taxonomy -- much like the Categories List block has so far done for categories.
Fixes #33302.
Closes #26555 (which is quite outdated by now).
How?
By adding a
taxonomy
attribute to the Categories List block, and a dropdown to the block inspector to set that attribute.The principal choice here was to either use block variations -- or not. I originally filed a PR for the former case (#64805), but feedback indicated that folks preferred the latter.
What is this not?
A full-fledged "Terms Query" block, as requested by #49094.
However, there's been a few issues and PRs floating around for a while for just a "simple" Terms List block (e.g. #58033, #26555), and I recently found myself working on a project where I could've used a "Terms List" block myself. I concluded that it might make sense to add such a block after all; this doesn't preclude us from adding a more powerful (and complex) Terms Query block later.
Testing Instructions
Add the following code (e.g. to your theme's
functions.php
) and use the "Project Types" panel in the block inspector to create a number of "Project Type" terms and assign them to a given post.Code
Then, navigate to the Site Editor and insert the "Terms List" block into a template of your choosing, and select "Project Type" from the Taxonomy dropdown in the block inspector. Save the template, and verify on the frontend that the block does indeed render a list of project type terms.
Furthermore, make sure to test with built-in taxonomies, both hierarchical (Categories) and flat (Tags).
Finally, test the toggles in the block inspector. Specifically, change the block's appearance to a dropdown, and verify that it's rendered correctly both in the editor, and on the frontend. Also verify that selecting individual items from the dropdown on the frontend causes WordPress to navigate to the corresponding taxonomy term's archive page.
Screenshots or screencast