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

Add a post categories block #22471

Closed
wants to merge 5 commits into from
Closed

Add a post categories block #22471

wants to merge 5 commits into from

Conversation

carolinan
Copy link
Contributor

@carolinan carolinan commented May 19, 2020

Description

Adds a new post categories block for full site editing for #15623

This is basically a copy paste of the current post tags block, but for categories and with alignments and CSS classes.

Eventually, the block would be improved on to include the same features mentioned here #20418

I have chosen to use a div to wrap the links instead of a list, only based on the feedback from theme developers who said they would prefer it to be a div. This may need to be updated to a list to improve accessibility.

How has this been tested?

Built and tested manually in both editors and on the front, with full site editing enabled, for posts with and without categories.

Types of changes

Adds a new post categories block that presents the categories assigned to the current post.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@youknowriad
Copy link
Contributor

Thanks for your work here, it's great.

My first reaction is I wonder if this should be a dedicated block or if we should make the "tags" block more of a "terms" block and implement several variations (tags, categories) and several style variations.

Any thoughts on that?

cc @carolinan @MichaelArestad @mtias @epiqueras

@youknowriad youknowriad added the [Type] Experimental Experimental feature or API. label May 19, 2020
@ajitbohra
Copy link
Member

@youknowriad unified block for tags & categories sounds better which can further expand to custom taxonomies

@carolinan
Copy link
Contributor Author

I had the same question, but I am not sure what is best.

}

export default function PostCategoriesEdit() {
if ( ! useEntityId( 'postType', 'post' ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

With the introduction of block context, the client-side implementation should also reference the context values as well, rather than via useEntityId and useEntityProp. At least in getting the postId and postType, it should largely amount to using context as a property of the edit function.

export default function PostCategoriesEdit( { context } ) {
    const { postId, postType } = context;
}

(postType will need to be listed in block.json context alongside the existing postId)

To get the categories, it's a little more work using core-data entities to get the post's categories:

const categories = useSelect( ( select ) => 
    select( 'core' ).getEditedEntityRecord( 'postType', postType, postId )?.categories )
);

See also for reference example: #22359 (specifically, #22359 (comment))

Related: #21665

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have not been able to figure out how to get all taxonomies from core-data, or a named custom taxonomy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a list of all hierarchical taxonomies, but I don't know if I can use this core-data to check if a post uses that taxonomy.

@epiqueras
Copy link
Contributor

I think that tags and categories will be different enough to make a common block for them too complicated and laden with configuration options.

Maybe technically they can share a common implementation, but I don't think we should expose it as a single block to users.

@youknowriad
Copy link
Contributor

I think that tags and categories will be different enough to make a common block for them too complicated and laden with configuration options.

What differences do you see?

@epiqueras
Copy link
Contributor

Categories are hierarchical, and that will shape the UI in a way that won't make sense for tags.

@ZebulanStanphill
Copy link
Member

If we go with a single block on the back-end, we could use block variations to expose both a "Post Categories" and a "Post Tags" block that are really just the same thing with different attribute values.

@carolinan
Copy link
Contributor Author

carolinan commented May 20, 2020

Categories are hierarchical, and that will shape the UI in a way that won't make sense for tags.

Custom taxonomies can also be made hierarchical, so where would they fit best?

For the post post type, it is not so common to display the hierarchy, and all links have the same importance. But it makes sense for custom post types like products or a portfolio. Or were you thinking of something else?

@youknowriad
Copy link
Contributor

The hierarchical difference is a good point, especially if we'd allow edition of these blocks (adding terms inline...). This convinces me that we need separate blocks personally.

Now, the question is whether we need a "tags" and "categories" blocks or more "hierarchical" and "non-hierarchical" terms blocks (to support custom taxonomies too) like we already have for the components used in the editor sidebar.

Since the blocks are experimental (we can change them), I'd be fine moving with a "categories" block for now, since we already have a tags one but we need to make a decision here and adapt.

@ZebulanStanphill
Copy link
Member

I think we should have a "Non-hierarchical Terms" block and a "Hierarchical Terms" block. Variations could be used to expose a "Post Categories", "Page Categories", "Post Tags", and other blocks in the editor that are really just pre-configured versions of the two base blocks. Plugins adding custom post types could easily add block variations to either of these two blocks if they wanted to.

@epiqueras
Copy link
Contributor

I think we should have a "Non-hierarchical Terms" block and a "Hierarchical Terms" block. Variations could be used to expose a "Post Categories", "Page Categories", "Post Tags", and other blocks in the editor that are really just pre-configured versions of the two base blocks. Plugins adding custom post types could easily add block variations to either of these two blocks if they wanted to.

This^^

@paaljoachim
Copy link
Contributor

I believe this issue might also have relevance (I might be totally off.): #22048

@MichaelArestad
Copy link
Contributor

Sorry I'm a bit late to this discussion.

I think that tags and categories will be different enough to make a common block for them too complicated and laden with configuration options.

Maybe technically they can share a common implementation, but I don't think we should expose it as a single block to users.

I agree with this. At a user level, even if they are identical in function and display, it would reduce confusion to have a block for each type.

@22677 Can you add screenshots of this in action?

@carolinan
Copy link
Contributor Author

If the two blocks share the same technical implementation, the tags block is much further ahead.

Even if that was not the case, this would most likely be too complex for me to create.
Do I keep the PR open just so that the conversations here do not get lost?

@youknowriad
Copy link
Contributor

@carolinan I think this PR is still valuable as it's a great start. If you need help maybe another dev here can take over and help you refactor this to make it a "hierarchical taxonomy block" + "categories variation"?

@annezazu annezazu added the Needs Dev Ready for, and needs developer efforts label Jun 17, 2020
@vindl vindl mentioned this pull request Jul 29, 2020
6 tasks
@mapk mapk added the [Block] Post Terms Affects the Post Terms Block label Aug 11, 2020
@carolinan
Copy link
Contributor Author

With #24091 merged, this can be closed.

@carolinan carolinan closed this Aug 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Post Terms Affects the Post Terms Block Needs Accessibility Feedback Need input from accessibility Needs Dev Ready for, and needs developer efforts New Block Suggestion for a new block [Type] Experimental Experimental feature or API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants