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

Slider: Rating Slider Example #1867

Merged
merged 26 commits into from
May 18, 2021
Merged

Slider: Rating Slider Example #1867

merged 26 commits into from
May 18, 2021

Conversation

jongund
Copy link
Contributor

@jongund jongund commented Apr 22, 2021

I have created a new rating slider example based on the discussion at the 6 April 2021 meeting.

Features:

  • Uses SVG graphics elements
  • Has high contrast support
  • Code to use APG coding practices
  • Uses pointer events
  • Uses aria-hidden=true on SVG element
  • Sets the CSS property forced-color-adjust to auto on the SVG element.
  • Using stroke-opacity and fill-opacity instead of transparent values for setting stroke and fill colors for the SVG rect used for focus ring for high contrast mode.

Preview rating slider example

Review checklist

@a11ydoer a11ydoer requested review from jesdaigle and mcking65 April 27, 2021 18:49
@jongund
Copy link
Contributor Author

jongund commented May 4, 2021

Works on iOS using touch interface and also works with VoiceOver which announces the current and changes the number of stars.

@a11ydoer
Copy link
Contributor

a11ydoer commented May 4, 2021

Works on iOS using touch interface and also works with VoiceOver which announces the current and changes the number of stars.

Yes, I had a problem with iOS device for the example. Thanks so much, Jon.
I am also in the process of looking into/asking around WCAG touch target rule.

@a11ydoer
Copy link
Contributor

a11ydoer commented May 4, 2021

New double A WCAG 2.2 target size success criteria.

https://w3c.github.io/wcag/understanding/target-size-minimum.html

"Targets that allow for values to be selected spatially based on position within the target are considered one target for the purpose of the success criterion. Examples include sliders with granular values, color pickers displaying a gradient of colors, or editable areas where you position the cursor."

Therefore, the rating slider example does not have a conflict with WCAG target size rule.

My big thanks to Alastair Campbell, @alastc for this info.

@mcking65 mcking65 requested review from a11ydoer, shirsha and smhigley May 4, 2021 15:48
Copy link
Contributor

@smhigley smhigley left a comment

Choose a reason for hiding this comment

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

Visuals + a quick skim of the code look good to me.

Potentially in the future we could include a lighter shadow-fill effect on hover, but it's definitely not necessary before merging now 👍

Copy link
Contributor

@carmacleod carmacleod left a comment

Choose a reason for hiding this comment

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

+1
Works well with VoiceOver in both Safari and Chrome on macOS.
5 stars! :)

@alastc
Copy link

alastc commented May 4, 2021

Be careful with a lighter fill-effect, that can be an issue with non-text contrast, we even have that example in the docs.

@shirsha
Copy link

shirsha commented May 4, 2021

My only concern is when screenreader user navigates in browser mode there is no way for them to know that there are five stars.
When no stars are selected it will narrate it as "slider no stars"

The aria-valuetext changes only when the user tabs to it in focus mode ( aria-valuetext="none of the five stars" ), otherwise it will be "two stars", no stars etc.

@jongund
Copy link
Contributor Author

jongund commented May 4, 2021

Be careful with a lighter fill-effect, that can be an issue with non-text contrast, we even have that example in the docs.

Thank you for the reference, we are using a solid fill.

@jongund jongund changed the title Rating Slider Example Slider: Rating Slider Example May 7, 2021
@jscholes
Copy link
Contributor

@jongund This is looking good; thanks for your work on it. My only two concerns revolve around the wording of the default/zero value, and the changes that happen on focus/blur:

  1. By default, the slider is set to 0, and aria-valuetext reads: "none of the five stars". This wording feels a bit awkward, specifically the inclusion of the word "the", and the inconsistency between "none" and "0". "0 of 5 stars" would be fine IMHO.
  2. I don't know that the change to aria-valuetext on focus/blur is really necessary. If aria-valuetext was always to include the max, it would reduce the complexity of the implementation and provide a uniform experience regardless of how users navigate to/encounter the slider. I'm thinking "0 of 5 stars", "3 and a half of 5 stars", etc., at all times. Thoughts?

@mcking65
Copy link
Contributor

@jscholes wrote:

  1. By default, the slider is set to 0, and aria-valuetext reads: "none of the five stars". This wording feels a bit awkward, specifically the inclusion of the word "the", and the inconsistency between "none" and "0". "0 of 5 stars" would be fine IMHO.

I agree with this.

@jscholes wrote:

  1. I don't know that the change to aria-valuetext on focus/blur is really necessary. If aria-valuetext was always to include the max, it would reduce the complexity of the implementation and provide a uniform experience regardless of how users navigate to/encounter the slider. I'm thinking "0 of 5 stars", "3 and a half of 5 stars", etc., at all times. Thoughts?

We made this change to be consistent with the seek slider where I believe we have general agreement that hearing the maximum value repeated every time the value changes creates excess and even distracting verbosity, making cognitive focus on the task of setting a new value more difficult. In the case of a seek slider, we should also keep in mind that excess verbosity detracts from media that might be playing while users are performing the seek task.

So, the behavior we are proposing as ideal is to ensure that valuetext communicates the maximum when the page is loaded and any time the focus moves into the slider. To be sure that a reading cursor picks up the correct valuetext after focus moves away, the max is added to valuetext on blur.

While this approach to minimizing distracting verbosity when communicating the maximum value via valuetext is a bit more complex than necessary for the rating slider, I think there is some value in consistency in guidance. Trying to explain when this small bit of fine tuning is really valuable verses a nice-to-have is more likely to create confusion.

jongund added 3 commits May 12, 2021 13:34
updated `aria-valuetext` values for zero stars
Fixed spelling errors
@jongund
Copy link
Contributor Author

jongund commented May 12, 2021

@jscholes
Updated aria-valuetext labels when no stars are selected.

Copy link
Contributor

@mcking65 mcking65 left a comment

Choose a reason for hiding this comment

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

Editorial changes and review complete.

Copy link
Contributor

@a11ydoer a11ydoer left a comment

Choose a reason for hiding this comment

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

  • It still has a lag to activate the rating slider("tap to activate")
  • Rating was read as percent like other sliders in Android, Talkback. This is not necessarily incorrect but it would be more correct to read this is "star" rating reflecting the visual design.
    star rating slider-one start is 20percent

@jesdaigle
Copy link
Contributor

Code and test review look good.

@mcking65 mcking65 merged commit 35bff94 into main May 18, 2021
@mcking65 mcking65 deleted the slider-rating branch May 18, 2021 17:47
@mcking65 mcking65 added Code Quality Non-functional code changes to satisfy APG code style guidelines and linters enhancement Any addition or improvement that doesn't fix a code bug or prose inaccuracy Example Page Related to a page containing an example implementation of a pattern labels May 24, 2021
@mcking65 mcking65 added this to the 1.2 Release 1 milestone May 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Quality Non-functional code changes to satisfy APG code style guidelines and linters enhancement Any addition or improvement that doesn't fix a code bug or prose inaccuracy Example Page Related to a page containing an example implementation of a pattern
Development

Successfully merging this pull request may close these issues.

9 participants