-
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
Unify the post tags and post categories block under a unique post terms block. #31458
Conversation
Size Change: +166 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
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.
These blocks are still experimental, they might be used by some experimental FSE blocks already but I think such a change is needed at the moment.
IMO it's good timing to tidy things up before making this block stable.
Code wise besides the small comments this looks good 👍
A thing that was before in PostHierarchicalTerms
block is the usage of BlockVariationPicker
which was not actually used as we had set isDefault
to a variation. Same goes here and I don't see any reason not to remove this. Any thoughts?
name: 'post_tag', | ||
title: __( 'Post Tags' ), | ||
description: __( "Display a post's tags." ), | ||
icon, |
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.
We need to set tag
icon here.
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.
We have both a category
and post-categories
icons, we probably need the same for tags which we don't have right now. Maybe a good design follow-up. cc @jasmussen
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 set a reminder to follow up here.
I think the reason we have this is that we don't want to show the |
I know and it makes sense, what I'm suggesting is that we could remove |
Oh got it @ntsekouras it should be done now. |
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.
Looks good 💯 , thank Riad!
closes #31422
This PR does a few things:
These blocks are still experimental, they might be used by some experimental FSE blocks already but I think such a change is needed at the moment.
What do you think?