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

fix: (CXSPA-8033) add aria hidden attribute to 'required' asterisks #19327

Conversation

StanislavSukhanov
Copy link
Contributor

@StanislavSukhanov StanislavSukhanov requested review from a team as code owners October 3, 2024 09:29
@github-actions github-actions bot marked this pull request as draft October 3, 2024 09:29
@StanislavSukhanov StanislavSukhanov marked this pull request as ready for review October 3, 2024 10:55
@StanislavSukhanov StanislavSukhanov self-assigned this Oct 3, 2024
Copy link

cypress bot commented Oct 3, 2024

spartacus    Run #45378

Run Properties:  status check passed Passed #45378  •  git commit 41ac514605 ℹ️: Merge 7f855f4a204d4dca2e786f38d138bbbc478084e3 into cdc23a2c36ae6c3157f8dcf64bf8...
Project spartacus
Run status status check passed Passed #45378
Run duration 04m 28s
Commit git commit 41ac514605 ℹ️: Merge 7f855f4a204d4dca2e786f38d138bbbc478084e3 into cdc23a2c36ae6c3157f8dcf64bf8...
Committer Stanislav Sukhanov
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 3
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 125

Copy link
Contributor

@Pio-Bar Pio-Bar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just add the attribute to the abbr, otherwise you can enter the empty group in read mode. We need to make sure the information about the field being required is still included somewhere. 🤔

Comment on lines 73 to 81

<ng-template #requiredAsterisk>
<abbr
*cxFeature="'a11yRequiredAsterisks'"
class="text-decoration-none"
title="{{ 'common.required' | cxTranslate }}"
>*</abbr
>
</ng-template>
Copy link
Contributor

@Pio-Bar Pio-Bar Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good on you for cleaning it up, somebody must have left it here while removing the asterisks from the login page. 🤷

@github-actions github-actions bot marked this pull request as draft October 7, 2024 09:58
@StanislavSukhanov
Copy link
Contributor Author

@Pio-Bar fixed. The 'required' is still being narrated. It's being picked up by an inputs required attribute.

@StanislavSukhanov StanislavSukhanov marked this pull request as ready for review October 7, 2024 10:00
Pio-Bar
Pio-Bar previously approved these changes Oct 7, 2024
Zeyber
Zeyber previously approved these changes Oct 7, 2024
@github-actions github-actions bot marked this pull request as draft October 10, 2024 17:23
@developpeurweb
Copy link
Contributor

@StanislavSukhanov your solution works well on JAWS. Now, let's complete the auditor's full request, so please bring back the Login and Reset Password asterisk removed on CXSPA-7418 and apply your solution there as well. I have left a comment on that ticket explaining why we can't remove audited content.

@StanislavSukhanov StanislavSukhanov dismissed stale reviews from Pio-Bar and Zeyber via a691380 October 15, 2024 08:41
@StanislavSukhanov StanislavSukhanov force-pushed the feature/CXSPA-8033-add-aria-hidden-to-required-asterisks branch from 9361c7f to a691380 Compare October 15, 2024 08:41
@StanislavSukhanov StanislavSukhanov marked this pull request as ready for review October 15, 2024 08:42
@StanislavSukhanov
Copy link
Contributor Author

@developpeurweb It's done. Please have a look on that once again 🙏

@github-actions github-actions bot marked this pull request as draft October 17, 2024 10:03
@StanislavSukhanov StanislavSukhanov marked this pull request as ready for review October 17, 2024 10:05
@github-actions github-actions bot marked this pull request as draft October 17, 2024 10:45
@StanislavSukhanov StanislavSukhanov marked this pull request as ready for review October 17, 2024 10:46
@github-actions github-actions bot marked this pull request as draft October 17, 2024 11:36
@StanislavSukhanov StanislavSukhanov marked this pull request as ready for review October 17, 2024 12:06
@github-actions github-actions bot marked this pull request as draft October 17, 2024 14:05
@StanislavSukhanov StanislavSukhanov marked this pull request as ready for review October 17, 2024 14:28
@StanislavSukhanov StanislavSukhanov merged commit 6d20b3b into develop Oct 17, 2024
28 checks passed
@StanislavSukhanov StanislavSukhanov deleted the feature/CXSPA-8033-add-aria-hidden-to-required-asterisks branch October 17, 2024 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants