WIP: Add descriptions to register_block_pattern. #23070

carolinan · 2020-06-10T17:50:51Z

Description

I have attempted to add descriptions to block patterns.
Fixes #23042

I am also not sure if I got the ID right.

const descriptionId = block-editor-inserter__patterns-item-description-${ instanceId };

Output:
<div class="components-visually-hidden" id="block-editor-inserter__patterns-item-description-4">A cover block with a gradient background and a paragraph with an extra large font size.</div>

How has this been tested?

I added descriptions to four of the existing patterns, as well as a pattern inside a theme.
I tested it by checking the markup on the inserter, and tested the aria described by and visually hidden element with the NVDA screen reader.

Types of changes

New feature.

Checklist:

My code is tested.
My code follows the WordPress code style.
[x ] 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.

sync

Adds visually hidden descriptions to block patterns in the inserter. For #23042.

youknowriad · 2020-06-11T07:46:30Z

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

 		>
 			<BlockPreview blocks={ blocks } viewportWidth={ viewportWidth } />
 			<div className="block-editor-inserter__patterns-item-title">
 				{ pattern.title }
 			</div>
+			<VisuallyHidden id={ descriptionId }>


you can add

{ !! pattern.description && ( // Your visibily hidden element } )

to render this conditionally depending on the presence of the description.

youknowriad

This is looking good to me. I'd appreciate @enriquesanchez's review

sync with master

sync branch with master

carolinan · 2020-06-11T12:15:27Z

How do you all feel about renaming "title" to label, matching register_block_style? Does it need to be a separate issue?

youknowriad · 2020-06-11T12:19:10Z

We do have "title" for blocks too, so it seems the inconsistency already exists.

Adds doing_it_wrong messages for the required properties. Adds descriptions to the block patterns. Updates documentation with the description and viewportWidth.

youknowriad · 2020-06-11T15:13:03Z

docs/designers-developers/developers/block-api/block-patterns.md

@@ -15,16 +15,19 @@ The `register_block_pattern` function receives the name of the pattern as the fi
 The properties of the pattern include:
 - `title` (required): A human-readable title for the pattern.
 - `content` (required): Raw HTML content for the pattern.
+ - `description` (required): A description of the pattern. Used to describe the pattern in the inserter.


I'm personally hesitant about making this required. I think it shouldn't be as a lot of existing patterns don't have it and we should fallback to the title. Sometimes the title is descriptive enough.

Is there a way to not have it be required but be really really encouraged?

mmm I don't know, we could console log a message if we don't find it maybe but we never did that before and it can be very verbose. I can't think of a way though.

Is compatibility for existing patterns the only concern? Because the function was just renamed, developers had to make changes then, why is adding a description a bigger concern?

I've thought about this more and I still think it shouldn't be mandatory. Some patterns are descriptive enough and the API should be as straightforward to use as possible.

Enforcing on a potential Patterns directory/repository is a separate question though and I'd see this being different there.

But I thought that was the point, the patterns are not descriptive if you can't see them.
And currently, neither the title or the content is required, even though the documentation says they are.
Without a title, the only thing announced by screen readers is "button".

Adding a description is more straight forward than adding categories (especially if you throw in registering the categories).

enriquesanchez · 2020-06-11T23:04:57Z

This is a great PR, @carolinan! Tested with VoiceOver + Safari and works as expected. The descriptions you added are very detailed and it's easy to picture the pattern.

youknowriad · 2020-07-01T09:01:23Z

How can we move this PR forward to include in 5.5

enriquesanchez · 2020-07-01T22:55:20Z

It'll be great to have this included with 5.5 and updated to reflect the changes in #23608.

Anything left to be done before we can merge?

youknowriad · 2020-07-01T23:37:52Z

I'd personally like for the mandatory aspect to be removed to include this as stated here #23070 (comment) especially since we're late in the 5.5 process and people are likely to have created a lot of patterns by now.

carolinan · 2020-07-05T03:32:55Z

So to summarize the discussion during the accessibility teams meeting on friday:
Title will be required.
Description will be strongly recommended.

#23608 needs to be merged first so that the list of patterns is up to date.

sync with master

trying to solve merge conflicts with the inserter updates

sync with master

Removes the check for the description property in the pattern registry. Update the documentation to show that descriptions are encouraged, not required.

Adds the description to the block patterns list. Outputs the visually hidden description after the title, if the description exists.

Updated the wording in the description for the viewportWidth.

youknowriad · 2020-07-06T09:28:48Z

@carolinan That sounds great 👍

#23608 is merged, do you have time to refresh the PR to include in today's RC?

sync branch with master

youknowriad · 2020-07-06T10:52:22Z

lib/class-wp-block-patterns-registry.php

+			$message = __( 'Pattern content must be a string.', 'gutenberg' );
+			_doing_it_wrong( __METHOD__, $message, '8.5.0' );
+			return false;
+		}


The change here needs dedicated Core patch.

I am not sure where to upload it :) https://core.trac.wordpress.org/attachment/ticket/50550/50550-doing-it-wrong.patch

youknowriad

Looks great. Thanks @carolinan

youknowriad · 2020-07-06T11:53:23Z

I merged too quickly without rewording the commit message. I guess it's fine for this time :).

carolinan added 4 commits May 23, 2020 09:24

Merge pull request #1 from WordPress/master

7880b05

sync

Merge pull request #3 from WordPress/master

2e83755

sync

Merge pull request #5 from WordPress/master

117b302

sync

Add description to block patterns

84bef7f

Adds visually hidden descriptions to block patterns in the inserter. For #23042.

carolinan requested review from ellatrix, spacedmonkey, TimothyBJacobs and youknowriad as code owners June 10, 2020 17:50

youknowriad reviewed Jun 11, 2020

View reviewed changes

carolinan added 2 commits June 11, 2020 14:02

Merge pull request #6 from WordPress/master

af4fcd9

sync with master

Merge pull request #7 from carolinan/master

1305287

sync branch with master

Add doing it wrong messages

851113c

Adds doing_it_wrong messages for the required properties. Adds descriptions to the block patterns. Updates documentation with the description and viewportWidth.

carolinan requested review from ajitbohra, chrisvanpatten and mkaz as code owners June 11, 2020 15:04

youknowriad reviewed Jun 11, 2020

View reviewed changes

Merge branch 'master' into update/block-patterns

4ee9c79

carolinan and others added 5 commits July 5, 2020 05:35

Merge pull request #8 from WordPress/master

02d507f

sync with master

trying to solve merge conflicts

1ff2603

trying to solve merge conflicts with the inserter updates

Merge pull request #9 from carolinan/master

a336d2e

sync with master

No longer require descriptions

52d8e64

Removes the check for the description property in the pattern registry. Update the documentation to show that descriptions are encouraged, not required.

Add the description to the block patterns list

a4bef7a

Adds the description to the block patterns list. Outputs the visually hidden description after the title, if the description exists.

Update block-patterns.md

f8c4310

Updated the wording in the description for the viewportWidth.

carolinan and others added 5 commits July 6, 2020 12:34

remove the deleted block patterns

b25cb94

Text update to solve merge conflict

362cfb4

remove description

c0af6be

update pattern content

c5a69f7

Merge pull request #11 from WordPress/master

3dfb767

sync branch with master

youknowriad reviewed Jul 6, 2020

View reviewed changes

youknowriad approved these changes Jul 6, 2020

View reviewed changes

youknowriad merged commit cafa3fe into WordPress:master Jul 6, 2020

github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Jul 6, 2020

github-actions bot added this to the Gutenberg 8.5 milestone Jul 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Add descriptions to register_block_pattern. #23070

WIP: Add descriptions to register_block_pattern. #23070

carolinan commented Jun 10, 2020 •

edited

Loading

youknowriad Jun 11, 2020

youknowriad left a comment

carolinan commented Jun 11, 2020

youknowriad commented Jun 11, 2020

youknowriad Jun 11, 2020

enriquesanchez Jun 11, 2020

youknowriad Jun 12, 2020

carolinan Jun 12, 2020

youknowriad Jun 24, 2020

carolinan Jul 1, 2020

enriquesanchez commented Jun 11, 2020

youknowriad commented Jul 1, 2020

enriquesanchez commented Jul 1, 2020

youknowriad commented Jul 1, 2020

carolinan commented Jul 5, 2020

youknowriad commented Jul 6, 2020

youknowriad Jul 6, 2020

carolinan Jul 6, 2020

youknowriad left a comment

youknowriad commented Jul 6, 2020

WIP: Add descriptions to register_block_pattern. #23070

WIP: Add descriptions to register_block_pattern. #23070

Conversation

carolinan commented Jun 10, 2020 • edited Loading

Description

How has this been tested?

Types of changes

Checklist:

Choose a reason for hiding this comment

youknowriad left a comment

Choose a reason for hiding this comment

carolinan commented Jun 11, 2020

youknowriad commented Jun 11, 2020

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

enriquesanchez commented Jun 11, 2020

youknowriad commented Jul 1, 2020

enriquesanchez commented Jul 1, 2020

youknowriad commented Jul 1, 2020

carolinan commented Jul 5, 2020

youknowriad commented Jul 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad left a comment

Choose a reason for hiding this comment

youknowriad commented Jul 6, 2020

carolinan commented Jun 10, 2020 •

edited

Loading