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

Block Bindings: Improve accessibility for non-editable, bound fields #58595

Open
artemiomorales opened this issue Feb 2, 2024 · 18 comments
Open
Labels
[Feature] Block bindings [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback. [Type] Bug An existing feature does not function as intended

Comments

@artemiomorales
Copy link
Contributor

artemiomorales commented Feb 2, 2024

Description

The Block Bindings API, which gives users the ability to connect block attributes to values obtained from different sources, was recently introduced. It also powers Synced Pattern Overrides.

As part of the UX around this new feature set, we've disabled editing of connected block attributes as seen in the following PRs:

However, while testing accessibility via keyboard navigation and NVDA in Windows Firefox, I realized that these read-only attributes are not announced as non-editable by screen readers — in other words, they are not announced as different from normal block attributes in any way.

As a result, if users attempt to edit these fields or expect to see them as editable like in normal blocks, the current keyboard behavior either sends them entirely out of context with no indication or skips over the fields entirely, likely causing confusion.

Here follow some ideas on how we can improve this and related UX around the Block Bindings:

  1. Regarding patterns specifically, I found navigating to the actual editable content in a pattern using a keyboard to be unintuitive and inconsistent. We should consider improving this flow by ensuring content is readable in the sidebars so users know what it is they'll be editing, as well as adding non-editable content to the sidebars so users can have proper context, and distinguishing between the editable and non-editable fields. We should also consider what the best tabbing behavior should be when in Edit Mode. Note: This may be a broader issue regarding the interplay of Navigation Mode and Edit Mode.

  2. In both of these modes, it'd be good to announce to users when fields are non-editable.

  3. Additionally, when users press Enter on read-only block attributes in pattern instances, namely paragraph or heading content, normally this would activate Edit Mode but this currently unexpectedly sets focus elsewhere and causes context to be lost; when doing this with other kinds of block bindings, the resulting behavior is similarly unexpected. We should aim to handle these scenarios gracefully in a way that doesn't cause users to lose context.

  4. For disabled fields like image alt, since users may be expecting to see them as editable in the block settings like in normal blocks, we should announce that those fields are read-only rather than skipping over them when using tab navigation.

  5. Somewhat related to the above, how one should edit the content of a buttons block just using the keyboard was unclear to me. If this is indeed an issue, we can take that into account when determining any improvements to make, either as part of an overall fix or coordinating with another fix that may be in progress.

Note: This was first raised in the following comment.

Step-by-step reproduction instructions

Patterns

Setup

  1. Add a new synced pattern
  2. Add 2 headings, 2 paragraphs, 2 images, and 2 buttons blocks
  3. Under Advanced, enable pattern overrides for one of each kind of block
  4. Save the pattern
creating-pattern-v2.mp4

Testing Pattern Overrides with a Keyboard and Screenreader

  1. In a new post, add the patten
  2. Using keyboard and a screenreader, tab until you reach the pattern
  3. Press Enter to go into Edit Mode
  4. Tab a few times to reach the editable content — see that the screen reader doesn't announce what the content of the pattern is, so it's hard to know what one would be editing
  5. Now press Enter to edit one of the blocks, then press Escape twice to enter Navigation Mode
  6. Tab to a non-editable block, and press Enter to switch to Edit Mode — see that focus and context are lost
  7. Tab until you reach the editable pattern content again
  8. Press Enter on one of the editable fields, then tab away from the field — consider what the expected behavior should be in this scenario
editing-pattern.mp4

I only go through the heading and paragraph block in the video above, but similar UX inconsistencies are present on the image and button blocks.

Other Bindings

Setup

  1. Register a new custom field however you prefer, for example the following code in your theme's functions.php:
register_meta(
	'post',
	'text_custom_field',
	array(
		'show_in_rest' => true,
		'single'       => true,
		'type'         => 'string',
		'default'	   => 'This is the content of the text custom field',
	)
);
register_meta(
	'post',
	'url_custom_field',
	array(
		'show_in_rest' => true,
		'single'       => true,
		'type'         => 'string',
		'default'	   => 'https://wpmovies.dev/wp-content/uploads/2023/04/goncharov-poster-original-1-682x1024.jpeg',
	)
);
  1. Create a new post, open the code editor, and insert the following:

Note: This functionality appears to not be working for headings, images, and buttons on trunk at the moment, so I've omitted them from these testing instructions for now.

<!-- wp:paragraph {"metadata":{"bindings":{"content":{"source":"core/post-meta","args":{"key":"url_custom_field"}}}}} -->
<p>Overridable paragraph</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>Regular paragraph</p>
<!-- /wp:paragraph -->
  1. Open the visual editor and navigate to the page content using a screenreader
  2. Tab to a normal block and press Enter to use Edit Mode
  3. Press Escape twice to use Navigation Mode
  4. As you tab using Navigation Mode, notice that bound blocks are locked visually but screen readers don't announce them as non-editable
  5. Press Enter on a bound block to enter Edit Mode, then start typing
  6. See that the field is not announced as non-editable

Here's a video reviewing the overall UX when navigating with keyboard and screen reader across bound blocks. As mentioned, the heading, image, and button blocks appear to not be working consistently at the time of this writing so you may not be able to see that specifically, but we should still be able to get this discussion started:

accessibility-disabled-fields.mp4

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@artemiomorales artemiomorales added [Type] Bug An existing feature does not function as intended [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) Needs Accessibility Feedback Need input from accessibility [Feature] Block bindings [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). labels Feb 2, 2024
@artemiomorales
Copy link
Contributor Author

artemiomorales commented Feb 2, 2024

Note: Block Bindings appear to not be working for me consistently for headings, images, and buttons for now on trunk, so I've omitted them in the testing instructions for Other Block Bindings for now.

@artemiomorales
Copy link
Contributor Author

Pinging @joedolson @afercia @andrewhayward for your thoughts on best practices regarding accessibility here, and to add any context regarding navigating Gutenberg with keyboard and screen readers that I may be missing.

Do these proposals that I have above sound reasonable, or is there anything else we should be considering to improve the UX of keyboard + screen reader navigation?

@afercia
Copy link
Contributor

afercia commented Feb 2, 2024

@artemiomorales thank you for creating this issue, for the very detailed explanation (which I know takes a lot of time) and more importantly for caring about accessibility ❤️

It's a pretty complex issue to read through and test, it will take some time.

@artemiomorales
Copy link
Contributor Author

artemiomorales commented Feb 2, 2024

@afercia Of course, thanks to you for taking a look! Just let me know if you'd like any clarification or woudl like to discuss anything more in depth 😄

@annezazu
Copy link
Contributor

annezazu commented Feb 3, 2024

Want to ping @alexstine here as well in case he can dig in. Thanks for making this 🙌🏼

@afercia
Copy link
Contributor

afercia commented Feb 5, 2024

I started testing the bound block following instructions from WordPress/wordpress-develop#5888 and have some initial thoughts, limited to semantics so far. Keyboard interaction will need more testing.

contenteditable-based blocks

Blocks that are based on contenteditable, for example Paragraphs and Headings, should communicate they cannot be edited in the most possible native way.

I see that when a Paragraph or Heaeding take their content from a bound source, their contenteditable attribute gets a false value. So far, so good. An aria-readonly="true" attribute would semantically communicate the block cannot be edited.

However, we need to solve another, long standing, issue. Some of the contenteditable blocks have an ARIA role="document". That was changed from the original role="textbox" to solve an issue with the JAWS screen reader. That change has been discussed at length in another issue (can't find it right now) and there's no agreement about it. Personally, I'm not sure deviating from the specifications to solve very specific issues with very specific software is ideal. Ib my experiences, this kind of approach leads to more harm than good an over time, inevitably, turns to be incompatible with new features and needs. I think I expressed more in details why that change should be reverted in the other issue.

That said, we now need to communicate a contenteditable is read-only. To do it in an appropriate way, we need to first communicate the contenteditable is a textbox and not a document.

More holistically, I'd think that whenever a contenteditable value is false, the editor should also set an aria-readonly="true" attribute. The resulting relevant attributes for a bound contenteditable-based block that cannot be edited should be:

contenteditable="false" role="textbox" aria-readonly="true" with the addition of aria-multiline="true" when a block accepts text in multiple rows.

Other blocks

Other block that aren't based on contenteditable should do something different. The Image block, for example. I have no great ideas on how to communicate an image can't be edited or that its alt attribute can't be edited.

Suggestions welcome. I'd tend to think there should be some labeling or announcement at the block container level, to inform users the block, or some of its attributes, cannot be edited.

Visually

RIght now, there's no visual indication that a bound block cannot be edited. That can be inferred from the block toolbar, as some controls are not available for bound blocks. I don't think that's sufficient though. See the screenshot below where the first Paragraph is normally editable and the second Paragraph can't be edited. They look identical. I'd think there should be a clear visual indication, in addition to a semantic indication.

Adding the needs design feedback label.

Screenshot 2024-02-05 at 15 54 07

Cc @joedolson @alexstine @andrewhayward

@afercia afercia added the Needs Design Feedback Needs general design feedback. label Feb 5, 2024
@afercia
Copy link
Contributor

afercia commented Feb 5, 2024

Re: the visual feedback may better discussed in a separate ticket. I'd just like to add that I can't see anything in the UI that tells me, for example, a Paragraph content can't be edited besides the fact the block toolbar shows less controls. I can't see anything in the List View and anything in the Settings panel either. Images with a connected al attribute at least show a descriptive text 'Connected to a custom field'. Other blocks don't show anything.

Screenshot 2024-02-05 at 16 06 06

@artemiomorales
Copy link
Contributor Author

@afercia Great, thank you for taking a look at this and offering your recommendations so far 🙏

More holistically, I'd think that whenever a contenteditable value is false, the editor should also set an aria-readonly="true" attribute. The resulting relevant attributes for a bound contenteditable-based block that cannot be edited should be:

contenteditable="false" role="textbox" aria-readonly="true" with the addition of aria-multiline="true" when a block accepts text in multiple rows.

While it seems the role="textbox" will require more discussion, adding the aria-readonly attribute is an easy fix and I've done it in the following PR.

aria-multiline is already present on the blocks in question, so no change needed for that one as far as I can see 👍

@getdave
Copy link
Contributor

getdave commented Feb 28, 2024

@artemiomorales Please could I ask you to provide an update on this one? It seems that aria-readonly has been added to richtext for contenteditable blocks but I'm guessing we're still missing:

  • visual feedback that parts of a block are not editable - this feels doable right now with some support from design.
  • perceivable feedback for assistive tech that non-contenteditable blocks that support block bindings (e.g. image) are not editable - this feels like it needs specific guidance from the Core a11y team as there is no established pattern for this type of behaviour.

Are these points something we feel we will still be able to target for the 6.5 release?

Did I miss anything out or any work that has been complete I am unaware of?

I don't see the textbox vs document question being something we can solve in the 6.5 cycle, but it would be good to have a canonical Issue for this connecting back here so there is some additional context for that discussion.

@artemiomorales
Copy link
Contributor Author

artemiomorales commented Feb 28, 2024

@getdave

visual feedback that parts of a block are not editable - this feels doable right now with some support from design.

You're right that visual feedback is doable and we're adding some indicators here via input from design.

perceivable feedback for assistive tech that non-contenteditable blocks that support block bindings (e.g. image) are not editable - this feels like it needs specific guidance from the Core a11y team as there is no established pattern for this type of behaviour.

It looks like we likely won't be able to improve the accessibility with screen readers for 6.5, and that yes this is a larger discussion on how to improve the overall flow when editing documents, encountering noneditable fields, and switching between Navigation and Edit modes that will require more discussion.

@alexstine
Copy link
Contributor

There is a lot of discussion here.

#53364

The document role was introduced as I was trying to fix a very specific JAWS accessibility bug. Although, the role used before it, group, was also not correct. I accept responsibility for making the situation worse with the document role but never did figure out who added the group role and why. I think at this point, now that all the tests are playwright, changing to textbox is probably something to play with. Textbox role for text blocks, switch back to the group role for non-text blocks.

My point, there should have never been a specific role added to useBlockProps hook in the first place because accessibility roles are not one size fits all.

@getdave
Copy link
Contributor

getdave commented Feb 29, 2024

OK thanks for confirming folks.

@fabiankaegy @annezazu for your information re: WP 6.5:

@getdave
Copy link
Contributor

getdave commented Mar 5, 2024

I'm removing this from the 6.5 board as we are entering RC period for WP 6.5 the only immediately actionable task has been completed.

@joedolson
Copy link
Contributor

Can we get an update on what's remaining on this issue? It seems like it's been suspended to address the larger navigation issue, but I'm not totally clear what steps are being taken. If there isn't any new accessibility feedback needed on this issue, we could remove the Needs Accessibility Feedback label.

@afercia
Copy link
Contributor

afercia commented Apr 17, 2024

1
The RichText component ARIA role must be changed to role="textbox". It's good that aria-readonly was added in #58687 but that's not useful when the role is document.

There's already a long standing issue for this, see #42158.
There is some software archaeology on that issue to illustrate the role was originally textbox, than changed to group than changed to document, but only for some blocks.

2
Accessible feedback for non-contenteditable blocks. I have no great ideas on how to communicate some attrirbutes of a block can't be edited. For example. for the Image block the url, alt, and title may be not editable. In the future, for other blocks it could be anything.

3
Visual feedback. I'm not sure using only an icon is sufficient. To me, the visual representation of a bound block in the UI is very unclear and insufficient.

4
The 'connected' visual indicator added to the block toolbar is not OK. It's a focusable div element with an aria-label attribute, which is not OK and it's something that was already pointed out several times as a bad practice. Never use a focusable div unless it reproduces all the native semantics and behaviors of the corresponding HTML element that is supposed to replace.

@afercia
Copy link
Contributor

afercia commented Apr 17, 2024

Anyone willing to test the user interface with bound blocks can use the following snippets. As of April 2024 this will add 2 bound Paragraphs, one bound Image, and one bound Button. Note that the button appears to be a bit buggy.

Hoping this can help accessibility and usability specialists to test the UI Cc @joedolson @alexstine

Register a few post meta by adding this snippet to your theme's functions.php
add_action( 'init', 'test_block_bindings' );

function test_block_bindings() {
	register_meta(
		'post',
		'text_from_meta_ui',
		array(
			'show_in_rest'      => true,
			'single'            => true,
			'type'              => 'string',
			'sanitize_callback' => 'wp_strip_all_tags',
			'default'           => 'default text from registered meta',
		)
	);

	register_meta(
		'post',
		'text_from_meta_long',
		array(
			'show_in_rest'      => true,
			'single'            => true,
			'type'              => 'string',
			'sanitize_callback' => 'wp_strip_all_tags',
			'default'           => 'longer text from registered meta: A wonderful serenity has taken possession of my entire soul, like these sweet mornings of spring which I enjoy with my whole heart. I am alone, and feel the charm of existence in this spot, which was created for the bliss of souls like mine. I am so happy, my dear friend, so absorbed in the exquisite sense of mere tranquil existence, that I neglect my talents. I should be incapable of drawing a single stroke at the present moment; and yet I feel that I never was a greater artist than now.',
		)
	);

	register_meta(
		'post',
		'url_from_meta_value',
		array(
			'show_in_rest'      => true,
			'single'            => true,
			'type'              => 'string',
			'sanitize_callback' => 'esc_url_raw',
			'default'           => 'https://wpmovies.dev/wp-content/uploads/2023/04/goncharov-poster-original-1-682x1024.jpeg',
		)
	);

	register_meta(
		'post',
		'image_alt_from_meta_value',
		array(
			'show_in_rest'      => true,
			'single'            => true,
			'type'              => 'string',
			'sanitize_callback' => 'wp_strip_all_tags',
			'default'           => 'some meaningful alt text',
		)
	);

	register_meta(
		'post',
		'image_title_from_meta_value',
		array(
			'show_in_rest'      => true,
			'single'            => true,
			'type'              => 'string',
			'sanitize_callback' => 'wp_strip_all_tags',
			'default'           => 'some meaningful title text',
		)
	);

	register_meta(
		'post',
		'button_text_from_meta_value',
		array(
			'show_in_rest'      => true,
			'single'            => true,
			'type'              => 'string',
			'sanitize_callback' => 'wp_strip_all_tags',
			'default'           => 'This is the bound button text',
		)
	);

	register_meta(
		'post',
		'button_url_from_meta_value',
		array(
			'show_in_rest'      => true,
			'single'            => true,
			'type'              => 'string',
			'sanitize_callback' => 'esc_url_raw',
			'default'           => 'https://wordpress.org',
		)
	);

	register_meta(
		'post',
		'button_linkTarget_from_meta_value',
		array(
			'show_in_rest'      => true,
			'single'            => true,
			'type'              => 'string',
			'sanitize_callback' => 'wp_strip_all_tags',
			'default'           => 'hello',
		)
	);

	register_meta(
		'post',
		'button_rel_from_meta_value',
		array(
			'show_in_rest'      => true,
			'single'            => true,
			'type'              => 'string',
			'sanitize_callback' => 'wp_strip_all_tags',
			'default'           => 'lalala',
		)
	);
}
Blocks to add to your post content via the Code editor mode
<!-- wp:paragraph -->
<p>This is a normal paragraph.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph {"metadata":{"bindings":{"content":{"source":"core/post-meta","args":{"key":"text_from_meta_ui"}}}}} -->
<p></p>
<!-- /wp:paragraph -->

<!-- wp:paragraph {"metadata":{"bindings":{"content":{"source":"core/post-meta","args":{"key":"text_from_meta_long"}}}}} -->
<p></p>
<!-- /wp:paragraph -->

<!-- wp:image {"metadata":{"bindings":{"url":{"source":"core/post-meta","args":{"key":"url_from_meta_value"}},"alt":{"source":"core/post-meta","args":{"key":"image_alt_from_meta_value"}},"title":{"source":"core/post-meta","args":{"key":"image_title_from_meta_value"}}}}} -->
<figure class="wp-block-image"><img src="" alt="" title=""/></figure>
<!-- /wp:image -->

<!-- wp:buttons -->
<div class="wp-block-buttons"><!-- wp:button {"metadata":{"bindings":{"text":{"source":"core/post-meta","args":{"key":"button_text_from_meta_value"}},"url":{"source":"core/post-meta","args":{"key":"button_url_from_meta_value"}},"linkTarget":{"source":"core/post-meta","args":{"key":"button_linkTarget_from_meta_value"}},"rel":{"source":"core/post-meta","args":{"key":"button_rel_from_meta_value"}}}}} -->
<div class="wp-block-button"><a class="wp-block-button__link wp-element-button" href="" target="" rel="noopener">asd</a></div>
<!-- /wp:button --></div>
<!-- /wp:buttons -->

@artemiomorales
Copy link
Contributor Author

1
The RichText component ARIA role must be changed to role="textbox". It's good that aria-readonly was added in > #58687 but that's not useful when the role is document.

There's already a long standing issue for this, see #42158.
There is some software archaeology on that issue to illustrate the role was originally textbox, than changed to group than changed to document, but only for some blocks.

This seems beyond the scope of Block Bindings specifically, but maybe this project is the impetus that will drive a revision of these roles and the keyboard navigation overall. It seems like a big lift and I feel a bit stuck on the topic. I think we'd need to reach some agreement on the best path forward, then commit to gradual transition to ensure consistency across the editor, right?

2
Accessible feedback for non-contenteditable blocks. I have no great ideas on how to communicate some attrirbutes of a block can't be edited. For example. for the Image block the url, alt, and title may be not editable. In the future, for other blocks it could be anything.

Rather than simply disabling the fields, can we leave them enabled and focusable but introduce an aria label to better indicate to users their behavior, that they're disabled due to the binding but can be edited if the binding is removed?

3
Visual feedback. I'm not sure using only an icon is sufficient. To me, the visual representation of a bound block in the UI is very unclear and insufficient.

4
The 'connected' visual indicator added to the block toolbar is not OK. It's a focusable div element with an aria-label attribute, which is not OK and it's something that was already pointed out several times as a bad practice. Never use a focusable div unless it reproduces all the native semantics and behaviors of the corresponding HTML element that is supposed to replace.

The aim is to revisit this UI for 6.6, so we can take these ideas into consideration as we iterate on and improve the UX. Thank you 🙏 CC'ing @SantosGuillamot @SaxonF regarding this point on the UI.

@alexstine
Copy link
Contributor

@artemiomorales

  1. I am back to working on the PR that will hopefully solve this once and for all. We'll see. Waiting on others to test. Iframe: try role=application #53364
  2. I like that idea but you could use aria-disabled instead. We don't want to really use labels to communicate any type of non-editable state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block bindings [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback. [Type] Bug An existing feature does not function as intended
Projects
Status: No status
Development

No branches or pull requests

6 participants