Adds aria-label to the search button, as accessibility enhancement #38136

Sisanu · 2022-01-21T13:33:34Z

Description

Adds aria-label to the search button, as accessibility enhancement.

How has this been tested?

Tested with Lighthouse and the accessibility score went up.

Screenshots

Types of changes

Enhancement

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
I've tested my changes with keyboard and screen readers.
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 (please manually search all *.native.js files for terms that need renaming or removal).
I've updated related schemas if appropriate.

github-actions · 2022-01-21T13:33:56Z

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @Sisanu! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

aristath · 2022-01-24T10:23:13Z

I can see that before this PR, the <button> contained just the <svg> without a label.
Using aria-label fixes the issue, however, I'm not certain if this would be the recommended method. 🤔
I think we have 2 choices here: Either use the aria-label attribute, OR add in the contents of the button element something like <span class="screen-reader-text">%s</span>.

Adding the a11y tag since this will need to be reviewed 👍

Sisanu · 2022-01-24T10:27:19Z

Thank you @aristath for the feedback! Anything is better than no accessibility for the button.

carolinan · 2022-01-24T10:46:21Z

I do not know if using a screen reader text or aria-label would be best.
~~Since this reuses the $attributes['label'] it might also make sense to use aria-labelledby?~~ The label does not have a unique id.

Sisanu · 2022-01-25T10:11:24Z

~~Since this reuses the $attributes['label'] it might also make sense to use aria-labelledby?~~ The label does not have a unique id.

Maybe that's why aria-label is the way to go for now, without changing too much the output of the block.

alexstine · 2022-02-09T06:08:35Z

I think you're only other option is VisuallyHidden from wordpress/components. Not sure this would actually be better than just a simple aria-label in this case.

alexstine · 2022-02-09T06:14:03Z

https://accessible360.com/accessible360-blog/use-aria-label-screen-reader-text/

Looks like screen reader text is a better option and I've always opted for using screen reader text anyway. For this, it really is trivial as both should have the same effect.

@Sisanu I'll let you make the call on if you want to do the extra work with VisuallyHidden.

Sisanu · 2022-02-10T06:37:12Z

Thank you @alexstine! I would keep it as simple as possible, and use just the area-label for now.

packages/block-library/src/search/index.php

ryelle · 2022-02-15T21:56:11Z

I was giving this one last test when I realized the aria-label was applied to every button, regardless of icon or text. The buttons that have text don't need the label, as IIRC aria-label will override the custom text someone may have set. I made a change in 0ef7f75 to only apply the aria-label when it's an icon button (and also rebased the PR).

alexstine

A thought to add.

alexstine · 2022-02-16T07:14:41Z

packages/block-library/src/search/index.php

@@ -81,17 +82,19 @@ function render_block_core_search( $attributes ) {
 				$button_internal_markup = wp_kses_post( $attributes['buttonText'] );
 			}
 		} else {
+			$aria_label             = sprintf( 'aria-label="%s"', esc_attr( wp_strip_all_tags( $attributes['label'] ) ) );


@ryelle I am not sure using aria-label in this context should be done. Honestly, I think this whole file should be refactored to include $attributes['button_aria_label'] that way users wouldn't be tempted to go out of their way to break accessibility. My problem with your approach is this.

$attributes['buttonText'] = '<span class="icon-class"></span>';

Now, the button is back to being inaccessible. Technically, it's not an icon button, but it still allows HTML right? Hints the need to strip tags for aria-label?

As I said, I would rather see an approach where accessibility related attributes are given and then we won't have to try to guess if users could make it inaccessible. Of course users can make it inaccessible, but the difference would be passing $attributes['buttonText'] vs. $attributes['button_aria_label'].

Thoughts? Does this make sense?

So, create a new attribute to pass in a button label? That does make sense, but I think it would be a good discussion to have in another issue. Overall I think getting this solution in place would improve things now, and it wouldn't prevent iterating on the label by introducing a new attribute.

Sure, I'm good with that. I'll make a follow-up PR once this is in.

Sounds good, though I'd suggest making an issue first - it'll need design input for the aria label setting UI.

github-actions · 2022-02-16T17:31:46Z

Congratulations on your first merged pull request, @Sisanu! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

carolinan · 2022-02-18T22:09:14Z

Please note that there is an open trac ticket that can be closed once this is added to core https://core.trac.wordpress.org/ticket/55196

talldan · 2022-02-21T04:17:23Z

@Sisanu Thanks for tackling this! I also made an issue about the lack of label when I noticed it recently - #38647.

Looking at your PR, it looks like the problem has now been solved for the front-end output of the block.

In the editor there's some separate code for the search block's icon only button that's also missing a label:

gutenberg/packages/block-library/src/search/edit.js

Lines 255 to 263 in b44f513

    
           { buttonUseIcon && ( 
        
           	<button 
        
           		type="button" 
        
           		className={ buttonClasses } 
        
           		style={ buttonStyles } 
        
           	> 
        
           		<Icon icon={ search } /> 
        
           	</button> 
        
           ) }

Do you think a label should be added for this too? If so, is that something you'd be willing to work on?

Sisanu · 2022-02-21T06:20:37Z

Hi @talldan! Sure, I can take a look. Please assign me the issue.

alexstine · 2022-02-21T18:03:21Z

@Sisanu Looks like I can't assign you to the Issue, but here is the link.

#38647

Sisanu · 2022-02-21T20:17:37Z

@talldan, @alexstine, it's ready for review #38966

talldan · 2022-02-22T07:14:53Z

@Sisanu If you write something like 'Fixes #38647' or 'Closes #38647' in the PR description, there's a bot that will assign you to the issue and github will automatically close the issue when the PR is merged.

…38136) * Adds aria-label to the search button, as accessibility enhancement * Strip all tags for the aria label attribute * Add aria label only when the button is an icon Co-authored-by: Kelly Dwan <[email protected]>

Sisanu requested a review from ajitbohra as a code owner January 21, 2022 13:33

github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Jan 21, 2022

aristath added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Jan 24, 2022

alexstine approved these changes Feb 9, 2022

View reviewed changes

alexstine mentioned this pull request Feb 9, 2022

Search block icon-only button has no aria-label #38647

Closed

alexstine reviewed Feb 10, 2022

View reviewed changes

packages/block-library/src/search/index.php Show resolved Hide resolved

ryelle added the [Block] Search Affects the Search Block - used to display a search field label Feb 11, 2022

ryelle self-requested a review February 11, 2022 21:52

ryelle reviewed Feb 11, 2022

View reviewed changes

packages/block-library/src/search/index.php Outdated Show resolved Hide resolved

ryelle mentioned this pull request Feb 11, 2022

Search: Use search block with icon button WordPress/wporg-mu-plugins#155

Merged

Sisanu and others added 3 commits February 15, 2022 16:28

Adds aria-label to the search button, as accessibility enhancement

4c222d8

Strip all tags for the aria label attribute

0c0b84a

Add aria label only when the button is an icon

0ef7f75

ryelle force-pushed the enhancement/search-block/accessibility-attr branch from 72cc2bd to 0ef7f75 Compare February 15, 2022 21:53

alexstine reviewed Feb 16, 2022

View reviewed changes

ryelle merged commit 16f6a5b into WordPress:trunk Feb 16, 2022

github-actions bot added this to the Gutenberg 12.7 milestone Feb 16, 2022

ryelle mentioned this pull request Feb 16, 2022

No accessible name on search submit button WordPress/wporg-mu-plugins#163

Closed

carolinan added the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Feb 18, 2022

talldan mentioned this pull request Feb 22, 2022

Added aria label to the button block with icon #38966

Merged

8 tasks

Mamaduka removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Mar 29, 2022

manolobevia mentioned this pull request Apr 7, 2022

Create upstream release v1.9.4 cul-it/uls#1088

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds aria-label to the search button, as accessibility enhancement #38136

Adds aria-label to the search button, as accessibility enhancement #38136

Sisanu commented Jan 21, 2022

github-actions bot commented Jan 21, 2022

aristath commented Jan 24, 2022

Sisanu commented Jan 24, 2022

carolinan commented Jan 24, 2022 •

edited

Loading

Sisanu commented Jan 25, 2022

alexstine commented Feb 9, 2022

alexstine commented Feb 9, 2022

Sisanu commented Feb 10, 2022

ryelle commented Feb 15, 2022

alexstine left a comment

alexstine Feb 16, 2022

ryelle Feb 16, 2022

alexstine Feb 16, 2022

ryelle Feb 16, 2022 •

edited

Loading

github-actions bot commented Feb 16, 2022

carolinan commented Feb 18, 2022

talldan commented Feb 21, 2022

Sisanu commented Feb 21, 2022

alexstine commented Feb 21, 2022

Sisanu commented Feb 21, 2022

talldan commented Feb 22, 2022

Adds aria-label to the search button, as accessibility enhancement #38136

Adds aria-label to the search button, as accessibility enhancement #38136

Conversation

Sisanu commented Jan 21, 2022

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

github-actions bot commented Jan 21, 2022

aristath commented Jan 24, 2022

Sisanu commented Jan 24, 2022

carolinan commented Jan 24, 2022 • edited Loading

Sisanu commented Jan 25, 2022

alexstine commented Feb 9, 2022

alexstine commented Feb 9, 2022

Sisanu commented Feb 10, 2022

ryelle commented Feb 15, 2022

alexstine left a comment

Choose a reason for hiding this comment

alexstine Feb 16, 2022

Choose a reason for hiding this comment

ryelle Feb 16, 2022

Choose a reason for hiding this comment

alexstine Feb 16, 2022

Choose a reason for hiding this comment

ryelle Feb 16, 2022 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Feb 16, 2022

carolinan commented Feb 18, 2022

talldan commented Feb 21, 2022

Sisanu commented Feb 21, 2022

alexstine commented Feb 21, 2022

Sisanu commented Feb 21, 2022

talldan commented Feb 22, 2022

carolinan commented Jan 24, 2022 •

edited

Loading

ryelle Feb 16, 2022 •

edited

Loading