Add new Categories block #2102

tyxla · 2017-07-31T09:41:06Z

Purpose

This PR introduces a new Categories dynamic block, fixes #1790.

Features

The new block comes with the following alignment options: none, left, right, center, full. It also supports the following attributes, which are similar to the core Categories widget:

Display as dropdown - default is false, which displays an unordered list.
Show post counts - default is false, indicates these are totally hidden.
Show hierarchy - default is false, meaning a flat, non-hierarchical list.

The server-side rendering part takes advantage of what the core widget uses - wp_list_categories() for the unordered list, and wp_dropdown_categories for the dropdown.

Preview

List view, no post counts, no hierarchy

List view, with post counts, no hierarchy

List view, no post counts, with hierarchy

List view, with post counts, with hierarchy

Dropdown view, without post counts, without hierarchy

Dropdown view, with post counts, without hierarchy

Dropdown view, without post counts, with hierarchy

Dropdown view, with post counts, with hierarchy

Block control options

Questions

The script for redirecting users to a category once they change it in the dropdown field in the frontend - I've borrowed it from the Categories core widget. I felt it would make sense to be consistent with what we have in the core, but if you folks have better suggestions, I'm all ears! Thanks!

codecov · 2017-07-31T10:47:11Z

Codecov Report

Merging #2102 into master will decrease coverage by 0.28%.
The diff coverage is 3.07%.

@@            Coverage Diff             @@
##           master    #2102      +/-   ##
==========================================
- Coverage    22.3%   22.01%   -0.29%     
==========================================
  Files         137      139       +2     
  Lines        4291     4356      +65     
  Branches      722      730       +8     
==========================================
+ Hits          957      959       +2     
- Misses       2815     2870      +55     
- Partials      519      527       +8

Impacted Files	Coverage Δ
blocks/library/categories/data.js	`0% <0%> (ø)`
blocks/library/categories/index.js	`3.22% <3.22%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 207274b...c501a4b. Read the comment docs.

jasmussen · 2017-07-31T15:02:59Z

This works great! Thanks so much for working on it.

This PR feels very very ready to go.

It doesn't show empty categories by default, it seems. Is the same behavior as the widget?

aduth · 2017-07-31T15:10:19Z

blocks/library/categories/data.js

@@ -0,0 +1,13 @@
+/**
+ * Returns a Promise with the categories or an error on failure.


Technically it returns a jqXHR which implements the jQuery.Deffered interface.

http://backbonejs.org/#Collection-fetch

👍 86afe7a

aduth · 2017-07-31T15:12:25Z

blocks/library/categories/index.js

+			];
+		}
+
+		componentWillUnmount() {


Not prescribed, but general convention that render should be last method defined in a component class. I tend to prefer to keep lifecycle methods together toward the top of the class.

What do you think? Maybe we should document this, since I find I'm recommending it fairly often.

I agree, updated in 2af85c8.

aduth · 2017-07-31T15:13:32Z

blocks/library/categories/index.js

+		}
+
+		toggleDisplayAsDropdown() {
+			const { displayAsDropdown } = this.props.attributes;


Minor (Stylistic): We could avoid nested property access by including attributes in the destructure of this.props:

const { attributes, setAttributes } = this.props; const { displayAsDropdown } = attributes;

👍 30ba95e

aduth · 2017-07-31T15:16:01Z

blocks/library/categories/index.js

+			this.categoriesRequest = getCategories();
+
+			this.categoriesRequest
+				.then( categories => this.setState( { categories } ) );


Can we confirm whether this callback will be invoked if the request is aborted while in-flight? At that point the component would no longer be mounted either, so setState could cause trouble.

Sure - it will not be invoked, as the deferred .then is executed only if the request is resolved.

aduth · 2017-07-31T15:17:04Z

blocks/library/categories/index.js

+		getCategories( parentId = null ) {
+			const { categories } = this.state;
+			if ( ! categories.length ) {
+				return [];


Should we return categories; here? Why do we need this condition anyways? I doubt Array#filter on an empty array has much overhead to be worried about.

Yeah, good call about returning categories here - updated in 0fa3f29. I'm more inclined to keep the condition though - no need to continue if categories are not loaded yet.

aduth · 2017-07-31T15:18:14Z

blocks/library/categories/index.js

+
+			return (
+				<li key={ category.id }>
+					<a href={ category.link } target="_blank">{ category.name.trim() || __( '(Untitled)' ) }</a>


Curious since I've dealt with issues before: How well does this handle entities, like an ampersand & ? Might need to do some unescaping.

👍 Good call, done in 428d791.

aduth · 2017-07-31T15:22:01Z

blocks/library/categories/index.js

+
+			return [
+				<option key={ category.id }>
+					{ new Array( level * 3 ).fill( '\xa0' ) }


We don't include a polyfill (yet). This will error in IE11:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/fill#Browser_compatibility

Try instead:

Array.apply( null, new Array( level * 3 ) ).map( () => '\xa0' );

Or with Lodash:

_.times( 5, () => '\xa0' );

https://lodash.com/docs/4.17.4#times

Right, updated in aa5d4ad.

aduth · 2017-07-31T15:22:38Z

blocks/library/categories/index.js

+					{ category.name.trim() || __( '(Untitled)' ) }
+					{
+						!! showPostCounts
+							? ` ( ${ category.count } )`


Above we don't include spaces in parentheses around category.count, but here we do. Should we be consistent?

Of course! Updated in f1f7f0b.

aduth · 2017-07-31T15:24:04Z

lib/blocks/categories.php

+	);
+
+	if ( ! empty( $attributes['displayAsDropdown'] ) ) {
+		$id = 'wp-block-categories-' . wp_rand();


Why wp_rand ? Uniqueness? We're not guaranteed unique values between two subsequent wp_rand calls.

Makes sense. I've updated it to use an increasing static counter, see c501a4b

aduth · 2017-07-31T15:26:03Z

lib/blocks/categories.php

+				location.href = "<?php echo home_url(); ?>/?cat=" + dropdown.options[ dropdown.selectedIndex ].value;
+			}
+		}
+		dropdown.onchange = onCatChange;


Should this apply here: https://github.com/evcohen/eslint-plugin-jsx-a11y/blob/master/docs/rules/no-onchange.md ?

I could see it being frustrating to use by keyboard if it navigated by keying through the list of options.

I'm more inclined to keep the behavior we have for the core widget here - I like keeping things consistent. If we update the core widget to use onBlur, I'd be happy to update this one as well.

westonruter · 2017-07-31T17:31:45Z

blocks/library/categories/data.js

+export function getCategories() {
+	const categoriesCollection = new wp.api.collections.Categories();
+
+	const categories = categoriesCollection.fetch();


Will this not introduce the possibility of there being many requests when there is a large deeply-nested category tree?

Not really - this one will be called only per a Categories block.

I see. I was confusing this getCategories function with the getCategories method on the component, and the latter is called recursively.

westonruter · 2017-07-31T20:07:55Z

gutenberg.php

@@ -27,4 +27,5 @@

 	// Register server-side code for individual blocks.
 	require_once dirname( __FILE__ ) . '/lib/blocks/latest-posts.php';
+	require_once dirname( __FILE__ ) . '/lib/blocks/categories.php';


This needs to be changed in light of #2014.

This was fixed with a combination of rebase and f23a5da.

aduth · 2017-08-03T13:05:29Z

blocks/library/categories/index.js

+/**
+ * WordPress dependencies
+ */
+import { Component } from 'element';


Note with #2172, these need to be updated to prefix the dependencies with @wordpress/. You will need to perform a rebase against the latest version of master and apply your changes:

git fetch origin git rebase origin/master

👍 Rebased and updated in 515dad4.

tyxla · 2017-08-03T14:13:56Z

@aduth @westonruter thanks so much for the constructive feedback!

I've addressed it all, feel free to have a new look.

mtias

This looks good to me for a first iteration on categories.

tyxla · 2017-08-04T09:51:31Z

I'm happy to give this one a final round of testing and merge it, if everyone agrees it's good.

tyxla added [Feature] Blocks Overall functionality of blocks New Block Suggestion for a new block [Status] In Progress Tracking issues with work in progress labels Jul 31, 2017

tyxla self-assigned this Jul 31, 2017

tyxla force-pushed the try/categories-block branch from fdbdbd5 to f694b1f Compare July 31, 2017 10:47

tyxla force-pushed the try/categories-block branch 2 times, most recently from f604a79 to 380769b Compare July 31, 2017 11:06

tyxla mentioned this pull request Jul 31, 2017

Add Block: Categories #1790

Closed

tyxla requested review from youknowriad, mtias, aduth and ellatrix July 31, 2017 12:50

tyxla removed the [Status] In Progress Tracking issues with work in progress label Jul 31, 2017

aduth reviewed Jul 31, 2017

View reviewed changes

westonruter reviewed Jul 31, 2017

View reviewed changes

aduth reviewed Aug 3, 2017

View reviewed changes

tyxla added 6 commits August 3, 2017 16:13

Add Categories block with dropdown, counts and hierarchy options

03789b3

Add server-side rendering for Categories block

6f49fc2

Hook up the new Categories block

3f546d0

Add Categories block to full content tests

2893607

Add frontend scripts for the Categories block in dropdown mode

2dfb411

Add an empty Select Category option to Categories block dropdown

2cc2804

tyxla force-pushed the try/categories-block branch from f63ded5 to 2cc2804 Compare August 3, 2017 13:14

tyxla added 3 commits August 3, 2017 16:19

Fix categories data fetching description comment

86afe7a

Update npm scope of WP imports

515dad4

Clean up props destructuring in Categories block

30ba95e

tyxla added 7 commits August 3, 2017 16:35

Cleanup in Categories block data retrieval

0fa3f29

Unescape HTML entities in category names

428d791

Replace Array.fill with lodash.times

aa5d4ad

Remove unnecessary spaces inside count brackets

f1f7f0b

Move component lifecycle methods up top

2af85c8

Move categories server-side rendering to block dir

f23a5da

Update wp_rand() usage to a more unique index approach

c501a4b

mtias approved these changes Aug 3, 2017

View reviewed changes

mtias merged commit e5130c3 into master Aug 7, 2017

mtias deleted the try/categories-block branch August 7, 2017 12:57

afercia added the [Feature] Widgets Screen The block-based screen that replaced widgets.php. label Jan 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new Categories block #2102

Add new Categories block #2102

tyxla commented Jul 31, 2017 •

edited

Loading

codecov bot commented Jul 31, 2017 •

edited

Loading

jasmussen commented Jul 31, 2017

aduth Jul 31, 2017 •

edited

Loading

tyxla Aug 3, 2017

aduth Jul 31, 2017

tyxla Aug 3, 2017

aduth Jul 31, 2017

tyxla Aug 3, 2017

aduth Jul 31, 2017

tyxla Aug 3, 2017

aduth Jul 31, 2017

tyxla Aug 3, 2017

aduth Jul 31, 2017

tyxla Aug 3, 2017

aduth Jul 31, 2017

tyxla Aug 3, 2017

aduth Jul 31, 2017

tyxla Aug 3, 2017

aduth Jul 31, 2017

tyxla Aug 3, 2017

aduth Jul 31, 2017

tyxla Aug 3, 2017

westonruter Jul 31, 2017

tyxla Aug 3, 2017

westonruter Aug 3, 2017

westonruter Jul 31, 2017

tyxla Aug 3, 2017

aduth Aug 3, 2017

tyxla Aug 3, 2017

tyxla commented Aug 3, 2017

mtias left a comment

tyxla commented Aug 4, 2017

		@@ -0,0 +1,13 @@
		/**
		* Returns a Promise with the categories or an error on failure.

Add new Categories block #2102

Add new Categories block #2102

Conversation

tyxla commented Jul 31, 2017 • edited Loading

Purpose

Features

Preview

Questions

codecov bot commented Jul 31, 2017 • edited Loading

Codecov Report

jasmussen commented Jul 31, 2017

aduth Jul 31, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tyxla commented Aug 3, 2017

mtias left a comment

Choose a reason for hiding this comment

tyxla commented Aug 4, 2017

tyxla commented Jul 31, 2017 •

edited

Loading

codecov bot commented Jul 31, 2017 •

edited

Loading

aduth Jul 31, 2017 •

edited

Loading