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

Post Hierarchical Terms Block #24091

Merged
merged 9 commits into from
Aug 26, 2020
Merged

Post Hierarchical Terms Block #24091

merged 9 commits into from
Aug 26, 2020

Conversation

Copons
Copy link
Contributor

@Copons Copons commented Jul 21, 2020

Description

Add a new Post Hierarchical Terms Block block with a single variation that displays the current post's categories, if any, in a list of links separated by a vertical bar.

The block already supports: text, background, link colors; font size, line height, and text alignment.
Note: the text color is applied to the vertical bar separator, which only appears when the post has at least 2 categories.

As a follow up, we could add a way to customize the separator string

How has this been tested?

Tested in Post, Page, and Site Editor, on Chrome 83 on macOS 10.15.5.

Screenshots

Screenshot 2020-08-12 at 12 37 51

(The variation label misaligned issue is unrelated to this PR; taking care of it in #24547)

Screenshot 2020-08-12 at 12 39 04

Variation loaded and customized.

Types of changes

New feature (non-breaking change which adds functionality)

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.

@github-actions
Copy link

github-actions bot commented Jul 21, 2020

Size Change: +601 B (0%)

Total Size: 1.17 MB

Filename Size Change
build/block-library/index.js 136 kB +596 B (0%)
build/edit-navigation/index.js 11.7 kB +1 B
build/edit-post/index.js 304 kB +2 B (0%)
build/edit-site/index.js 17 kB +2 B (0%)
build/edit-widgets/index.js 11.8 kB +1 B
build/media-utils/index.js 5.32 kB -1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.67 kB 0 B
build/api-fetch/index.js 3.44 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.99 kB 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-editor/index.js 126 kB 0 B
build/block-editor/style-rtl.css 10.7 kB 0 B
build/block-editor/style.css 10.7 kB 0 B
build/block-library/editor-rtl.css 8.5 kB 0 B
build/block-library/editor.css 8.5 kB 0 B
build/block-library/style-rtl.css 7.45 kB 0 B
build/block-library/style.css 7.45 kB 0 B
build/block-library/theme-rtl.css 729 B 0 B
build/block-library/theme.css 730 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.7 kB 0 B
build/components/index.js 200 kB 0 B
build/components/style-rtl.css 15.7 kB 0 B
build/components/style.css 15.7 kB 0 B
build/compose/index.js 9.67 kB 0 B
build/core-data/index.js 12.3 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.55 kB 0 B
build/date/index.js 5.38 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 4.48 kB 0 B
build/edit-navigation/style-rtl.css 1.16 kB 0 B
build/edit-navigation/style.css 1.16 kB 0 B
build/edit-post/style-rtl.css 5.61 kB 0 B
build/edit-post/style.css 5.61 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.js 45.3 kB 0 B
build/editor/style-rtl.css 3.8 kB 0 B
build/editor/style.css 3.79 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.71 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.41 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.9 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@ockham
Copy link
Contributor

ockham commented Jul 21, 2020

Should we limit this to Posts (to exclude e.g. pages?) See #24082 for some relevant discussion.

@ZebulanStanphill
Copy link
Member

I would think that a Post Categories and Page Categories would just be the same block exposed as two different block variations using the block variations API.

@youknowriad youknowriad mentioned this pull request Jul 21, 2020
53 tasks
@ockham
Copy link
Contributor

ockham commented Jul 21, 2020

I would think that a Post Categories and Page Categories would just be the same block exposed as two different block variations using the block variations API.

That's not what I meant, apologies for being unclear. We should be able to use the same block for both posts and pages -- it's just that AFAIK pages normally don't have categories, which is why I was suggesting to hide the block from the inserter when editing a page. See #24082 where we're discussing essentially the same question for tags.

@Copons
Copy link
Contributor Author

Copons commented Jul 22, 2020

@ockham By passing the context's postType to useEntityProp we are pretty much protected from this kind of scenarios.
If the post type doesn't support categories (or whatever taxonomy we are looking for), the block will simply display "No categories" in the editor, and won't render anything on the front end.

We could improve the messaging, by checking if the post type actually supports the taxonomy, but in my opinion it would go beyond the scope of this PR (which, yes, I haven't specified it, but it's in the context of having styling parity between FSE blocks and normal blocks, see #21087).

@ockham
Copy link
Contributor

ockham commented Jul 22, 2020

@ockham By passing the context's postType to useEntityProp we are pretty much protected from this kind of scenarios.
If the post type doesn't support categories (or whatever taxonomy we are looking for), the block will simply display "No categories" in the editor, and won't render anything on the front end.

We could improve the messaging, by checking if the post type actually supports the taxonomy, but in my opinion it would go beyond the scope of this PR (which, yes, I haven't specified it, but it's in the context of having styling parity between FSE blocks and normal blocks, see #21087).

That's fair enough, but it'd be nice to have the Categories and Tags blocks somewhat consistent in their messaging and behavior (since they're so similar in nature, and both currently under development) 😬 @jeyip has asked for design input there.

Anyway, I don't mean to block this. This is probably good enough for a first iteration -- all I'm saying is we should keep close track to avoid the Categories and Tags blocks to diverge too much in terms of UX.

@vindl
Copy link
Member

vindl commented Jul 29, 2020

There was already a similar PR started here #22471, but it didn't receive updates since May. Should we close that one out with a note? There were also some possibly relevant discussions there.

@epiqueras
Copy link
Contributor

#22471 (comment)

@youknowriad
Copy link
Contributor

What if we try to help @carolinan and update her PR to rename the block "Post Hierarchical Taxonomy" or something like that and add a variation for categories? Potentially we could hide the default block (without any variation) from the inserter.

@Copons
Copy link
Contributor Author

Copons commented Jul 30, 2020

There was already a similar PR started here #22471, but it didn't receive updates since May. Should we close that one out with a note? There were also some possibly relevant discussions there.

My bad, I should have checked for duplicates before opening this. 🤦

  • Rename to "Post Hierarchical Taxonomy" and have a variation for categories.

Cool! and it makes lots of sense!
I assume down the line we should also rename the Post Tags block into "Post Non-Hierarchical Taxonomy" (even though, maybe we should come up with a better name).
Also, it was mentioned in a comment to the other PR to use "Terms" instead of "Taxonomy". I agree with that.

  • Choose which PR to keep.

I'm totally ok with keeping either!
If I had to make a case for mine, it would mostly be because it uses a few newer features (experimental style attributes, light wrapper, a simpler edit structure consistent with other post blocks).
At the same time, @carolinan's PR came first, so I'm perfectly fine with closing this and help with #22471 if needed. 🙂

@ZebulanStanphill
Copy link
Member

There's already a way for plugins to register new block variations: registerBlockVariation.

@youknowriad youknowriad added the Needs Design Feedback Needs general design feedback. label Aug 13, 2020
@youknowriad
Copy link
Contributor

This is looking good and is decent for an initial merge I think. I added the design label to get some thoughts on the design and on potential follow-ups to add customization options.

@Copons
Copy link
Contributor Author

Copons commented Aug 13, 2020

There's already a way for plugins to register new block variations: registerBlockVariation.

Nice!
So it should be enough for a plugin to register a variation with { attributes: { term: 'foo' } } in order to make this block load the foo custom taxonomy.

Copy link
Contributor

@Addison-Stavlo Addison-Stavlo left a comment

Choose a reason for hiding this comment

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

Awesome, this is neat to see! Im not super familiar with variations so it may be good to get another set of eyes to review here.

However, the "Post Categories" variation seems to work as expected in testing! 🎉

@Copons
Copy link
Contributor Author

Copons commented Aug 18, 2020

Removed the now unnecessary CSS class from the PHP render, as suggested by @Addison-Stavlo.

Also finally took the time to test this with custom taxonomies, and I'd dare say it works just fine. 😄

Screenshot 2020-08-18 at 14 35 54

I've built a super simple plugin to test this, in case you want to try it yourselves.

Code hidden below for clarity

/test-hierarchical-taxonomy/test-hierarchical-taxonomy.php

<?php
/**
 * Plugin Name: Test Hierarchical Taxonomy
 */

function copons_register_hierarchical_taxonomy() {
	register_taxonomy( 'copons_hierarchical_taxonomy', [ 'post' ], [
		'labels' => [
			'name' => 'Test Hierarchical Terms',
			'singular_name' => 'Test Hierarchical Term',
			'search_items' => 'Search',
			'all_items' => 'All',
			'parent_item' => 'Parent',
			'parent_item_colon' => 'Parent:',
			'edit_item' => 'Edit',
			'update_item' => 'Update',
			'add_new_item' => 'Add New',
			'new_item_name' => 'Name',
			'menu_name' => 'Test Hierarchical Terms',
		],
		'rewrite' => [ 'slug' => 'copons-test-hierarchical-taxonomy' ],
		'public' => true,
		'show_ui' => true,
		'show_admin_column' => true,
		'hierarchical' => true,
		'show_in_rest' => true,
	] );
}
add_action( 'init', 'copons_register_hierarchical_taxonomy' );

function copons_enqueue_hierarchical_taxonomy_block_variation() {
	wp_enqueue_script(
		'copons-test-hierarchical-taxonomy',
		plugins_url( 'block-variation.js', __FILE__ ),
		[ 'wp-blocks' ]
	);
}
add_action( 'enqueue_block_editor_assets', 'copons_enqueue_hierarchical_taxonomy_block_variation' );

/test-hierarchical-taxonomy/block-variation.js

( function( blocks ) {
	blocks.registerBlockVariation( 'core/post-hierarchical-terms', {
		name: 'copons_hierarchical_taxonomy',
		title: 'Test Hierarchical Terms',
		icon: 'category',
		attributes: { term: 'copons_hierarchical_taxonomy' },
	} );
} )( window.wp.blocks );

(Note: #24547 is my proposed fix for the variations labels misalignment)

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Should we hide the "Post Hierarchical Block" from the inserter initially and just leave the variation?

Copy link
Member

@vindl vindl left a comment

Choose a reason for hiding this comment

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

Nice work @Copons! ✨

@Copons
Copy link
Contributor Author

Copons commented Aug 25, 2020

Should we hide the "Post Hierarchical Block" from the inserter initially and just leave the variation?

@youknowriad I'm not sure how to do it. 🤔
I've tried hiding the parent block with support: { inserter: false }, but that hides the variation as well.
Is there a way to "re-enable" the variation overriding the parent's settings?

@youknowriad
Copy link
Contributor

I've tried hiding the parent block with support: { inserter: false }, but that hides the variation as well.
Is there a way to "re-enable" the variation overriding the parent's settings?

Did you try adding the "support" with the flag set to true on the variation? I'm actually not sure it works but it might be something we explore later if it doesn't.

@Copons
Copy link
Contributor Author

Copons commented Aug 26, 2020

I've tried hiding the parent block with support: { inserter: false }, but that hides the variation as well.
Is there a way to "re-enable" the variation overriding the parent's settings?

Did you try adding the "support" with the flag set to true on the variation? I'm actually not sure it works but it might be something we explore later if it doesn't.

Yup, I've tried that and it doesn't seem to work.

@youknowriad
Copy link
Contributor

Ok, let's keep this visible for now and figure it out later.

@Copons Copons merged commit 132a01c into master Aug 26, 2020
@Copons Copons deleted the add/post-categories-block branch August 26, 2020 14:19
@github-actions github-actions bot added this to the Gutenberg 8.9 milestone Aug 26, 2020
@mtias
Copy link
Member

mtias commented Aug 26, 2020

This is great, thanks! It's a great way to test variations as well — let's make sure to follow up on being able to hide the source block and only show variations. We might also need to expand the API to support something like "hidden" if it makes sense.

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 Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants