feat: add `aria-required` attributes to WC form fields

[frosso]

trafficstars

Submission Review Guidelines:

• I have followed the WooCommerce Contributing Guidelines and the WordPress Coding Standards.
• I have checked to ensure there aren't other open Pull Requests for the same update/change.
• I have reviewed my code for security best practices.
• Following the above guidelines will result in quick merges and clear and detailed feedback when appropriate.

Changes proposed in this Pull Request:

Adding the aria-required="true" attribute to required form inputs. I chose to add the aria-required attribute rather than the required attribute to avoid backwards-compatibility issues. When the required attribute is added, the form submission is intercepted and prevented by the browser. By adding aria-required, instead`, we add information to Screen Reader users without affecting the form's behavior.

I ensured that the aria-required attribute was added to all input types, excluding hidden. The aria-required attribute is added to all inputs of type radio, per HTML specs:

To avoid confusion as to whether a radio button group is required or not, authors are encouraged to specify the attribute on all the radio buttons in a group.

Closes https://github.com/woocommerce/woocommerce/issues/44663

How to test the changes in this Pull Request:

• (optional) Add the following snippet to your installation to test the radio inputs(you can use the Code Snippets plugin, for example)

add_action( 'woocommerce_after_checkout_billing_form', 'checkout_test_radio_options' );
function checkout_test_radio_options() {
	echo "<div>";
	echo sprintf( '<p>%s</p>', __( "Test radio input" ) );
    woocommerce_form_field( 'test_radio',  array(
		'required'  => true,
        'type'      => 'radio',
        'class'     => array( 'form-row-wide' ),
        'options'   => array(
            '1' => 'Option 1',
            '2' => 'Option 2',
        ),
    ), '1' );
	echo "</div>";
}

• Add a product to the cart
• Go to the shortcode checkout page
• The checkout page should display a wide range of inputs. All required inputs (excluding the "hidden" ones) should have an aria-required="true" attribute

Changelog entry

• [x] Automatically create a changelog entry from the details below.

• [ ] This Pull Request does not require a changelog entry. (Comment required below)

Changelog Entry Details

Significance

• [x] Patch
• [ ] Minor
• [ ] Major

Type

• [ ] Fix - Fixes an existing bug
• [ ] Add - Adds functionality
• [x] Update - Update existing functionality
• [ ] Dev - Development related task
• [ ] Tweak - A minor adjustment to the codebase
• [ ] Performance - Address performance issues
• [ ] Enhancement - Improvement to existing functionality

Message

feat: add aria-required attributes to WC form fields

Changelog Entry Comment

Comment

[github-actions[bot]]

Test using WordPress Playground

The changes in this pull request can be previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Test this pull request with WordPress Playground.

Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit.

[github-actions[bot]]

Hi @naman03malhotra,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like: https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

[github-actions[bot]]

Hi @alexflorisca,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like: https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

[frosso]

Assigned to Rubik because the change affects checkout form fields on classic checkout. Feel free to reassign if incorrect!

[frosso]

Thank you @alexflorisca ! I fixed the linting issues & added a new test for checkboxes (ensuring the aria-attribute is added to the input). Thanks again for your time!

[frosso]

Thank you for the feedback, @opr !

The changes you made here won't apply to country or state select fields as these are converted into select2 instances. For these you would need to update selectWoo.full.js.

https://github.com/woocommerce/woocommerce/commit/bbabe8a8efab6a74633062b6955f634c74c559f1 shows how this would look, though you need to change the country-select.js code to check for aria-required instead of required

With this approach, were you able to make the Screen Reader (SR) software announce the field as "required"? With the changes implemented (as a note - slightly differently because the $this.attr('required') doesn't return a boolean but a string & applying the required attribute on the role="combobox" element), I am still not able to have any software announce the field as required. I tried with VoiceOver on iOS/Mac OS, Talkback on Android Chrome & Brave, Screen Reader in Chrome.

Also in 718bd3c I made it so the asterisk is not read by screen readers. What do you think about applying this change too?

I'd like to hear your thoughts on the issue I mentioned above above, first. Since some browsers & SR software combinations don't announce required fields (e.g.: Talkback on Android w/ Google Chrome), I think the "required" abbr (albeit redundant in some other cases) provides a useful hint to users that otherwise wouldn't get the hint.

If we removed the required abbr, do you think we should also remove the (optional) hint on optional fields at checkout?

FWIW, I also noticed that to be really consistent, we'd need to add a aria-hidden attribute on address-i18n.js as well.

Since both issues go hand-in-hand, I was instead wondering if we should keep the "required" abbr tag to ensure that required select2 fields can be announced by SR software.

[opr]

@frosso

With this approach, were you able to make the Screen Reader (SR) software announce the field as "required"? With the changes implemented (as a note - slightly differently because the $this.attr('required') doesn't return a boolean but a string & applying the required attribute on the role="combobox" element), I am still not able to have any software announce the field as required. I tried with VoiceOver on iOS/Mac OS, Talkback on Android Chrome & Brave, Screen Reader in Chrome.

Yes, I was, I just implemented my changes on your branch. I tried with VoiceOver and Screen reader in Chrome and both worked for me! Can you check out the origin/try/country-screen-reader branch and see if it works there?

Since some browsers & SR software combinations don't announce required fields (e.g.: Talkback on Android w/ Google Chrome), I think the "required" abbr (albeit redundant in some other cases) provides a useful hint to users that otherwise wouldn't get the hint.

I didn't realise that some wouldn't announce the field as required! I agree the hints are probably useful then, let's leave them!

[frosso]

Yes, I was, I just implemented my changes on your branch. I tried with VoiceOver and Screen reader in Chrome and both worked for me! Can you check out the origin/try/country-screen-reader branch and see if it works there?

I applied those changes on my local environment. I also added a required select on checkout with label "Test required input".

I couldn't record Talkback on Android, due to some permissions issue. However, this is the transcribed result on Brave browser:

Brave Browser transcription

"Test required input" label:

Test required input

"Test required input" input:

Collapsed, required, test required input, menu popup button, double tap to activate

"First name" label:

First name

"First name" input:

Required, edit box, first name, double tap to edit text

"Country / region" label

Country / region

"Country / region" selectWoo input

Collapsed, United States US, dropdown list, country / region, double tap to change

On Android with Talkback & Google Chrome, all inputs (and labels) are not announced as required.

Mac OS w/ VoiceOver on Safari - VoiceOver doesn't seem to announce the "selectWoo" inputs as required. It's skipping the "required" abbr tag and announcing other native fields as required.

https://github.com/woocommerce/woocommerce/assets/273592/00292fa4-dd3b-48a2-931c-3070e9fa5f40

So with that branch's changes the "Country / Region" was never announced as required during my testing.

[opr]

@frosso very strange! It looks like that select isn't styled correctly , I'm wondering if it really is being picked up by select2 on your site?

Does it have aria-required="true" on it?

When I inspect it I see this, what do you see?

[frosso]

Does it have aria-required="true" on it? When I inspect it I see this, what do you see?

I have a similar output

[opr]

Ok, so it is being picked up by select2, and the select2-selection--single span has aria-required="true". Not sure why your screen reader isn't voicing this while mine is. If you update the aria-label on that span, does the screen reader correctly read the updated text? Trying to narrow down whether this is the element it's reading from, or if it's looking at something else.

[frosso]

If you update the aria-label on that span, does the screen reader correctly read the updated text? Trying to narrow down whether this is the element it's reading from, or if it's looking at something else.

Still no luck for my scenario, using VoiceOver on Safari. https://github.com/woocommerce/woocommerce/assets/273592/fc51b96d-242e-4ac4-91e3-d43bcb0c2aee

I also have another recording with the devtools open on Safari, in case that helps. It's just a bit messier :D

https://github.com/woocommerce/woocommerce/assets/273592/a7df26e6-f6f4-4854-ab11-6413d8a36faa

[github-actions[bot]]

Size Change: 0 B

Total Size: 2.31 MB

_{compressed-size-action}

[frosso]

@opr thank you for the time you dedicated today. As we discussed, I made the following changes:

• Added the aria-required to the combobox/selectWoo
• Added aria-hidden on the required abbr

[frosso]

@opr 🤦 I had it modified locally and didn't push it

Do you happen to know if I also need to modify plugins/woocommerce/client/legacy/js/selectWoo/selectWoo.js somehow? Or if I need to implement the changes in https://github.com/woocommerce/selectWoo?

[opr]

@opr 🤦 I had it modified locally and didn't push it

Do you happen to know if I also need to modify plugins/woocommerce/client/legacy/js/selectWoo/selectWoo.js somehow? Or if I need to implement the changes in woocommerce/selectWoo?

I couldn't see any code path that loaded plugins/woocommerce/client/legacy/js/selectWoo/selectWoo.js but maybe you want to do it just in case, with that said I didn't know about that selectWoo repository, makes sense that we should edit there as well. @barryhughes could you shed some light on the repository @frosso linked, is this the actual source for selectWoo and should we be editing the files there instead?

[barryhughes]

@barryhughes could you shed some light on the repository @frosso linked, is this the actual source for selectWoo and should we be editing the files there instead?

Yes, in theory that is the actual source of SelectWoo. Unfortunately, I believe there may have been some divergence over time, with some edits being made directly to the copy here, but not always being applied over there (and possibly vice versa).

If we can also raise a PR making the same change over there, though, I think that would be beneficial. cc @naman03malhotra for any other thoughts, as you've recently been looking at Select2/Woo.

[frosso]

@barryhughes @naman03malhotra I just took a quick look - after all the moving around, these are the latest changes to selectWoo in this repository. Notice the last commit matches the last commit in the selectWoo repository 3 years ago.

I created this PR on the selectWoo repository to basically mirror these changes: https://github.com/woocommerce/selectWoo/pull/53 I'm not sure who to assign it to 👀

[barryhughes]

Thanks, @frosso: @naman03malhotra I assigned to you (the usual round-robin doesn't seem to work in that repo, and I have low availability next week so couldn't grab it myself).

[frosso]

The addition of aria-hidden on the abbr would cause regressions across multiple repositories using e2e tests that run on multiple WC core versions. We decided to defer this as a future enhancement: https://github.com/woocommerce/woocommerce/issues/48948

[barryhughes]

@frosso should https://github.com/woocommerce/selectWoo/pull/53 also be put on hold?

[frosso]

@frosso should https://github.com/woocommerce/selectWoo/pull/53 also be put on hold?

@barryhughes sorry I missed your comment - I don't think there's anything that should hold it

[frosso]

@frosso I am still able to see the changed made to select2 files, in this PR https://github.com/woocommerce/selectWoo/pull/53 Since we have now https://github.com/woocommerce/woocommerce/pull/48731 Select2 altother from core, probably we can also remove it from the SelectWoo repo to avoid confusion.

@naman03malhotra as it stands, the selectWoo repository still contains Select2. How would you prefer to proceed? I have a few options:

1. First remove Select2 from the SelectWoo repository, then update https://github.com/woocommerce/selectWoo/pull/53 accordingly
2. First merge https://github.com/woocommerce/selectWoo/pull/53 , then update the SelectWoo repository to remove select2

I made the changes to the selectWoo repository just as a mirror of the changes in the main WC Core repository.

Do you think this PR is currently on hold because select2 is not currently removed from the selectWoo repository?