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

@W-12627152@ Improve refinement button ARIA labels #1607

Merged
merged 12 commits into from
Dec 13, 2023

Conversation

wjhsf
Copy link
Contributor

@wjhsf wjhsf commented Dec 7, 2023

Note: Built on #1605, so the changes in refinements.jsx and most of the changes in radio-refinements.jsx will disappear when that is merged.

This just adds "add/remove filter" to the ARIA labels for each refinement button.

Description

Types of Changes

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Documentation update
  • Breaking change (could cause existing functionality to not work as expected)
  • Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

  • Removing a public function or component or prop
  • Adding a required argument to a function
  • Changing the data type of a function parameter or return value
  • Adding a new peer dependency to package.json

Changes

  • (change1)

How to Test-Drive This PR

  • Start dev server
  • View a category or search page
  • Activate VoiceOver
  • Tab through each kind of refinement (radio, checkbox, size, color).
  • When not selected, the readout should be "Add filter: "
  • When selected, the readout should be "Remove filter: "
  • For color refinements, the readout should include the hit count
  • With selections made, tab into the "selected refinements" buttons at the top of the results
  • Tabbing through each button should say "Remove filter: "
  • The "clear all" button should be read out as "clear all refinements"

Checklists

General

  • Changes are covered by test cases
  • CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

  • There are no changes to UI

or...

Localization

  • Changes include a UI text update in the Retail React App (which requires translation)

@wjhsf wjhsf requested a review from a team as a code owner December 7, 2023 18:21
@wjhsf wjhsf enabled auto-merge (squash) December 7, 2023 21:33
vmarta
vmarta previously approved these changes Dec 7, 2023
Copy link
Contributor

@vmarta vmarta left a comment

Choose a reason for hiding this comment

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

Looks like this is not part of the original ticket's ACs... Cool, thanks for going the extra mile.

@vmarta vmarta disabled auto-merge December 7, 2023 21:38
@vmarta
Copy link
Contributor

vmarta commented Dec 7, 2023

FYI I've disabled the auto-merge for the current PR. Let's wait for the other PR #1605 to be merged first.

joeluong-sfcc
joeluong-sfcc previously approved these changes Dec 11, 2023
@wjhsf wjhsf dismissed stale reviews from joeluong-sfcc and vmarta via 4d821a1 December 11, 2023 19:35
@wjhsf wjhsf enabled auto-merge (squash) December 11, 2023 19:35
@wjhsf wjhsf requested a review from a team December 11, 2023 19:36
@wjhsf wjhsf merged commit de45add into develop Dec 13, 2023
22 checks passed
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