WIP: Install a block from inserter by ck-lee · Pull Request #16524 · WordPress/gutenberg

ck-lee · 2019-07-11T01:25:33Z

Description

Extend the inserter to show available blocks for download and silently install the block when the block is inserted into the editor

This PR is still a WIP. Can you help guide me with the implementation and avoiding any pitfalls? I'm new to the block editor.

Screenshots

Types of changes

Extend inserter to fetch a list of suggestBlocks similar to reusableBlocks
The list of suggestBlocks is currently served from a mock API. It is not built yet.
Extend the filter to show suggestBlocks if there are no results found.
When "Add" button is clicked, download the block assets from plugin.svn. Then, load and insert the block into the editor without installing it.
...

Relevant Links

[design] Main thread
https://make.wordpress.org/design/2019/04/26/block-library-installing-blocks-from-within-gutenberg/

[design] Figma
https://www.figma.com/file/QKhoOKXkBN2mHacqvrtdNuI6/Gutenberg-Block-Library%3A-Installation

[design] Mockup feedback
https://make.wordpress.org/design/2019/06/07/block-library-mockups-prototype

[dev] Block Directory Proposal
https://make.wordpress.org/meta/2019/03/08/the-block-directory-and-a-new-type-of-plugin/

[dev] Block Registration
https://github.com/WordPress/gutenberg/blob/master/docs/rfc/block-registration.md

[dev] Block Registration - assets JSON structure
#13693 (comment)

GitHub Block Directory issues
https://github.com/WordPress/block-directory/issues

GitHub Block Directory project
https://github.com/WordPress/block-directory/projects/1

packages/block-editor/src/components/block-ratings/index.js

packages/block-editor/src/components/inserter/menu.js

tellthemachines

The experiments settings bit looks good!

One question: should installing blocks also be enabled from the inserters on the Widgets screen and the Widget Blocks section in the Customizer? These inserters are still experimental (you need to enable the "Widgets" experiment for them to work) but they work exactly the same as the inserter on the main editor page, so it might make sense for them to include this functionality.

ck-lee · 2019-08-25T23:34:36Z

Some minor issues:

Not sure why, but on Windows, NVDA announces "No blocks found in your library" and on Mac, VO announces "0 results found" instead. I don't think this is a big deal but thought I should bring it up nevertheless.

On Windows, it'd be great if the number of reviews could be announced with a more descriptive label like on the Mac ("Zero total ratings"). I would also try to avoid announcing "Left parent. Zero. Right parent.".

Thank you, @enriquesanchez. Good to know that there are no major a18y issues. I will have a look at these minor ones and see if I can improve them.

ck-lee · 2019-08-25T23:45:34Z

should installing blocks also be enabled from the inserters on the Widgets screen and the Widget Blocks section in the Customizer?
@tellthemachines, Thank you for letting me know about the inserters in Widgets screen. I think the feature should be included too. I will look into that.

I hope that it is as as simple as including the block-directory package and fill/slot in the edit-widgets module.

…blocks

… into try/discover-new-blocks

ck-lee · 2019-08-28T23:46:34Z

This PR has now been split to 4 PRs in preparation for further review and merging:

Add block directory search settings (#17182)
Add an extension slot for inserter menu. (#17183)
Implement REST controller for block directory features. (#17243)
Implement new block-directory package (#17244)

…blocks

talldan · 2019-09-13T14:22:31Z

packages/block-directory/src/components/downloadable-block-header/index.js

+					onClick();
+				} }
+			>
+				{ __( 'Add' ) }


For screenreader users, the add button gives no context of the what the user is adding.

packages/editor/src/components/provider/index.js

talldan · 2019-09-13T14:42:38Z

packages/editor/src/components/provider/index.js

 		if ( this.props.settings !== prevProps.settings ) {
 			this.props.updateEditorSettings( this.props.settings );
 		}
+		if ( ! isEqual( this.props.downloadableBlocksToUninstall, prevProps.downloadableBlocksToUninstall ) ) {


I don't quite understand what this code is for—would be good to add a comment.

Ah, so the blocks get uninstalled if they're unused. I think that's how it's supposed to work.

I think this functionality would be best moved to the edit-post package, in a component. It'd be similar to how AutosaveMonitor is a component.

Cross-linking a related discussion fo reference: #17576 (comment)

This functionality was removed in #17576. It should be added back once we agree on a new approach.

talldan · 2019-09-13T14:43:55Z

packages/editor/src/components/provider/index.js

+			this.props.downloadableBlocksToUninstall.forEach( ( blockType ) => {
+				this.props.uninstallBlock( blockType, noop, () => {
+					this.props.createWarningNotice(
+						__( 'Block previews can\'t uninstall.' ), {


This text seems like it'll need some improvement.

talldan · 2019-09-13T15:56:37Z

packages/block-directory/src/components/downloadable-blocks-list/index.js

+					const createdBlock = onSelect( item );
+
+					const onInstallBlockError = () => {
+						createErrorNotice(


Inline notices within the inserter popover would provide a better experience, I think. Currently the message is hidden behind the inserter.

Let's fix this in a follow-up PR.

packages/block-directory/src/components/downloadable-blocks-panel/index.js

packages/block-directory/src/components/downloadable-block-author-info/style.scss

packages/block-directory/src/store/index.js

lib/class-wp-rest-block-directory-controller.php

noisysocks · 2019-09-13T14:18:11Z

lib/class-wp-rest-block-directory-controller.php

+		);
+		register_rest_route(
+			$this->namespace,
+			'/' . $this->rest_base . '/uninstall',


It doesn't feel very "restful" to use different URLs for the create and delete actions. Could "installation" be a resource? e.g. so POST /installations creates and DELETE /installations/:id deletes?

Let's do this in a follow-up PR.

noisysocks · 2019-09-13T14:34:06Z

packages/block-directory/src/components/downloadable-block-info/index.js

+					<Icon icon="chart-line"></Icon>{ sprintf( _n( '%d active installation', '%d active installations', activeInstalls ), activeInstalls ) }
+				</div>
+				<div className="block-directory-downloadable-block-info__column">
+					<Icon icon="update"></Icon><span aria-label={ sprintf( __( 'Updated %s' ), humanizedUpdated ) }>{ humanizedUpdated }</span>


It wasn't clear to me at first that this icon meant 'last updated'. Could we add title text? Relatedly, do these icons need alt text?

lib/class-wp-rest-block-directory-controller.php

noisysocks · 2019-09-13T16:23:02Z

packages/block-directory/src/components/downloadable-block-header/index.js

+		<div className="block-directory-downloadable-block-header__row">
+			{
+				icon.match( /\.(jpeg|jpg|gif|png)/ ) !== null ?
+					<img src={ icon } alt="block icon" /> :


Is there something more descriptive than "block icon" that we can use? Maybe the block's title? Regardless, this string needs to be localised.

packages/block-directory/src/components/downloadable-block-header/index.js

packages/block-directory/src/components/block-ratings/stars.js

noisysocks · 2019-09-13T16:30:26Z

packages/block-directory/src/components/downloadable-block-author-info/index.js

+				{ sprintf( __( 'Authored by %s' ), author ) }
+			</span>
+			<span className="block-directory-downloadable-block-author-info__content">
+				{ sprintf( _n( 'This author has %d block, with an average rating of %d.', 'This author has %d blocks, with an average rating of %d.', authorBlockCount, authorBlockRating ), authorBlockCount, authorBlockRating ) }


Does an average rating of 0 imply that the author has no ratings? If so, should we exclude the second part of this sentence when the average rating is 0? It sounds al little awkward otherwise:

This author has 1 block, with an average rating of 0.

Let's think about this in a follow-up PR.

packages/block-directory/src/store/resolvers.js

packages/block-directory/src/components/downloadable-block-header/style.scss

packages/block-directory/src/components/downloadable-block-info/style.scss

packages/block-directory/src/components/downloadable-block-list-item/style.scss

jasmussen

This is very impressive work, very very solid. I can't wait for this feature to land, it's breathtaking.

I left a few tiny comments to use variables here and there, but the CSS overall is well written, well structured and superb as is.

The only thing I think probably should be fixed before shipping is this one: https://github.com/WordPress/gutenberg/pull/16524/files#r324309944 — but I did leave a couple of suggestions for how to fix that.

noisysocks · 2019-09-13T19:57:20Z

Closing in favour of #17431.

CK added 18 commits July 3, 2019 12:34

Add new properties and panel for discover items

1060588

Add basic component with styling for discover blocks

8fc202a

Dynamically load block into editor.

ebd1148

Increase icon size.

e85b2a0

Handle insert block properly.

7dbdf01

Added another discover block.

673475e

Revert unncessary changes.

069d796

Retrieve discover blocks from a mock api.

a02a3c7

Only show discover blocks when no blocks are found.

7824941

Style discover items header

e08184b

Fill discover item footer elements.

b5e1756

Use BlockIcon components.

810546b

Merged

78e7883

Update documentation.

e363149

Update mock URL.

c67d4f3

Revert changes in BlockIcon for size and use Icon directly.

bdda0cc

Use Fragment from wp.element.

acca61d

Created BlockRatings component.

049297f

ck-lee requested review from ajitbohra, chrisvanpatten, ellatrix, gziolo, mkaz, talldan and youknowriad as code owners July 11, 2019 01:25

youknowriad reviewed Jul 11, 2019

View reviewed changes

packages/block-editor/src/components/block-ratings/index.js Outdated Show resolved Hide resolved

packages/block-editor/src/components/block-ratings/index.js Outdated Show resolved Hide resolved

packages/block-editor/src/components/inserter/menu.js Outdated Show resolved Hide resolved

CK added 2 commits August 23, 2019 11:53

Tidy up code.

54a891c

Update bottom section to be footer in an article.

2bab650

tellthemachines reviewed Aug 23, 2019

View reviewed changes

Merged from master

8bb30b1

ck-lee mentioned this pull request Aug 26, 2019

Add block directory search settings. #17182

Closed

3 tasks

Fix phpcs warning.

4a9c324

ck-lee mentioned this pull request Aug 26, 2019

Added an extension slot for inserter menu. #17183

Closed

5 tasks

Mapped author information for block directory

d1687ad

ck-lee mentioned this pull request Aug 28, 2019

Consider adjusting or removing help panel WordPress/block-directory#15

Closed

Merged

11978b1

ck-lee mentioned this pull request Aug 28, 2019

Implement REST controller for block directory features. #17243

Closed

3 tasks

CK added 2 commits August 29, 2019 11:14

Merge remote-tracking branch 'upstream/master' into try/discover-new-…

868cd14

…blocks

Merge branch 'try/discover-new-blocks' of github.com:ck-lee/gutenberg…

3f5804e

… into try/discover-new-blocks

ck-lee mentioned this pull request Aug 28, 2019

Implement new block-directory package #17244

Closed

5 tasks

Merge remote-tracking branch 'upstream/master' into try/discover-new-…

15e0012

…blocks

talldan reviewed Sep 13, 2019

View reviewed changes

noisysocks reviewed Sep 13, 2019

View reviewed changes

talldan mentioned this pull request Sep 13, 2019

Add block directory to inserter menu #17431

Merged