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

RE: [a11y] Links to read reviews are confusing for screen reader/keyboard users #10945

Merged
merged 4 commits into from Oct 20, 2021

Conversation

xlisachan
Copy link
Contributor

Fixes #8860

This pull request refactors the multiple links per rating by star to a single link (an approach mentioned in #9254).

In addition, it updates the link titles to include the number of reviews per rating by star (e.g., 'Read all 7 five-star reviews'). Conditions were also added in case there is only one or no reviews available (e.g. 'Read the four-star review', 'No three-star reviews yet').

Screen Shot 2021-10-01 at 1 45 10 PM

Please review when you have a moment. Thank you!

@codecov
Copy link

codecov bot commented Oct 1, 2021

Codecov Report

Merging #10945 (3038165) into master (be26ea1) will decrease coverage by 0.00%.
The diff coverage is 95.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10945      +/-   ##
==========================================
- Coverage   98.39%   98.38%   -0.01%     
==========================================
  Files         258      258              
  Lines        7538     7556      +18     
  Branches     1366     1372       +6     
==========================================
+ Hits         7417     7434      +17     
- Misses        113      114       +1     
  Partials        8        8              
Impacted Files Coverage Δ
src/amo/components/RatingsByStar/index.js 97.14% <95.23%> (-2.86%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be26ea1...3038165. Read the comment docs.

@bobsilverberg bobsilverberg requested review from a team and diox and removed request for a team October 5, 2021 11:58
src/amo/components/RatingsByStar/index.js Outdated Show resolved Hide resolved
@xlisachan
Copy link
Contributor Author

Thanks for the feedback, @diox ! The latest pull request removes the template strings and utilizes ngettext to handle the plural forms. Please review when you have a moment. Thank you!

@xlisachan xlisachan requested a review from diox October 11, 2021 14:54
src/amo/components/RatingsByStar/index.js Outdated Show resolved Hide resolved
@xlisachan
Copy link
Contributor Author

Thanks again for the feedback, @diox! The latest pull request has ngettext for each star rating as suggested. Please review when you have a moment. Thank you!

@xlisachan xlisachan requested a review from diox October 18, 2021 14:45
@xlisachan
Copy link
Contributor Author

Thanks for the feedback, @diox! The latest push reflects an updated stylesheet that removes !important and instead increases the specificity as advised. Please review when you have a moment. Thank you!

@xlisachan xlisachan requested a review from diox October 20, 2021 13:14
Copy link
Member

@diox diox left a comment

Choose a reason for hiding this comment

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

Looks good now, thanks @xlisachan !

@diox diox merged commit f27c2a2 into mozilla:master Oct 20, 2021
@eviljeff
Copy link
Member

@xlisachan if you comment in the issue we can assign it to you

@xlisachan xlisachan deleted the ratingsbystar_accessibility branch October 21, 2021 13:17
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.

[a11y] Links to read reviews are confusing for screen reader/keyboard users
3 participants