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: Temperature Selector Slider Example #1864

Merged
merged 19 commits into from
May 23, 2021
Merged

Slider: Temperature Selector Slider Example #1864

merged 19 commits into from
May 23, 2021

Conversation

jongund
Copy link
Contributor

@jongund jongund commented Apr 21, 2021

I have created a new temperature selector example based on the discussion at the 20 April 2021 meeting to include the only the temperature selector slider to illustrate the use of aria-valuetext and aria-orientation.

Features:

  • Appends temperature unit to aria-valuetext.
  • Uses SVG graphics elements
  • Increased size of clickable area on rail to change value with pointing device
  • Has high contrast support
  • Code to use APG coding practices
  • Uses event.key
  • Uses pointer events
  • Uses the none role on SVG element
  • Sets the CSS property forced-color-adjust to auto on the SVG elements.
  • 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 temperature slider example

Review checklist


Preview | Diff

@charmarkk charmarkk self-requested a review April 27, 2021 18:35
@mcking65
Copy link
Contributor

mcking65 commented May 4, 2021

@jongund

The meaning of this is not clear to me:

For people with visual impairments the current value of the slider is visible next to the thumb when it has focus or is being dragged.

What problem does this solve for people with visual impairments in particular?

@mcking65
Copy link
Contributor

mcking65 commented May 4, 2021

@jongund,

In valuetext, I wonder if using the degrees symbol instead of writing out the word "degrees" could impact announcement with some screen readers. We advise content designers to avoid using symbols in label strings at Facebook. However, the driver there might be translation. I'm not sure if there is generally accepted best practice around that.

Similarly, I am wondering if we want to use the letter "C" or write out celsius.

@jongund
Copy link
Contributor Author

jongund commented May 4, 2021

@jongund

The meaning of this is not clear to me:

For people with visual impairments the current value of the slider is visible next to the thumb when it has focus or is being dragged.

What problem does this solve for people with visual impairments in particular?

@mcking65
Updated the accessibility documentation to clarify the benefit.

@jongund
Copy link
Contributor Author

jongund commented May 4, 2021

@jongund,

In valuetext, I wonder if using the degrees symbol instead of writing out the word "degrees" could impact announcement with some screen readers. We advise content designers to avoid using symbols in label strings at Facebook. However, the driver there might be translation. I'm not sure if there is generally accepted best practice around that.

Similarly, I am wondering if we want to use the letter "C" or write out Celsius.

@mcking65
I don't have a strong opinion on the changing the aira-valuetext to use °C, it's an easy change if you want me to make it.

@jongund jongund changed the title Temperature Selector Slider Example Slider: Temperature Selector Slider Example May 7, 2021
@a11ydoer a11ydoer requested a review from carmacleod May 11, 2021 18:15
@mcking65
Copy link
Contributor

@jongund

I think I now understand the meaning of:

The current value of the slider is visible next to the thumb, so when the thumb value is changed using a pointing device by a person with the visual impairment they can immediately see the change in value without have to move their visual attention to another part of the page.

However, that does not make it clear that the value display moves with the thumb. Is the following revision accurate?

The display of the slider's current value remains adjacent to the thumb as the thumb is moved, so people with a small field of view (e.g., due to magnification) can easily see the value while focusing on the thumb as they move it.

@mcking65
Copy link
Contributor

@jongund
The last sentence of the high contrast documentation is:

In addition, some browsers the SVG rect elements may need to adjust their stroke-opacity and fill-opacity values in place of using the transparent value for setting the fill and stroke colors in high contrast modes.

The grammar is not correct. But, I can't correct it because I don't know what it means.

Who would need to adjust the opacity instead of using the transparent value? The author? Does our example adjust opacity or use the transparent value?

</section>

<section>
<h2 id="kbd_label" style="clear: both;">Keyboard Support</h2>
Copy link
Contributor

Choose a reason for hiding this comment

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

@jongund

What does the styling on this heading do? We don't have it on the kb heading in other examples? Should it be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mcking65
I think some older examples may use the CSS float property, so this is probably a ruminant from some of those early examples to make sure the heading was on a new line. It can be removed.

@jongund
Copy link
Contributor Author

jongund commented May 12, 2021

@mcking65
I have also updated the description of using fill-opacity and removing references to transparent value. Please review whenyou get a chance.

@mcking65 mcking65 requested review from a11ydoer, smhigley and carmacleod and removed request for carmacleod May 13, 2021 05:46
@mcking65
Copy link
Contributor

@jongund

Thank you for the help with the HCM documentation. I revised the focus ring sentences slightly to try to simplify and clarify a bit more. Here is where I landed. Is this wording clear and accurate?

To ensure the borders of the slider rail, thumb and focus ring have sufficient contrast with the background when high contrast settings invert colors, the color of the borders are synchronized with the color of the text content. For example, the color of the focus ring border is set to match the foreground color of high contrast mode text by specifying the CSS currentColor value for the stroke property of the inline SVG rect element used to draw the focus ring border. To make the background of the rect match the high contrast background color, the fill-opacity attribute of the rect is set to zero. If specific colors were instead used to specify the stroke and fill properties, those colors would remain the same in high contrast mode, which could lead to insufficient contrast between the rail and the thumb or even make them invisible if their color matched the high contrast mode background.
Note: The SVG element needs to have the CSS forced-color-adjust property set to auto for the currentColor value to be updated in high contrast mode. Some browsers do not use auto for the default value.

@jongund
Copy link
Contributor Author

jongund commented May 14, 2021

@mcking65
The changes to HCM documentation look great!

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 revisions and review complete. Looking great! Thank you, thank you @jongund for the fabulous work on this.

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.

I had to zoom in the example to activate and interact with slider thumb. Once I activate it, I could adjust the temperature. The exisiting problem is that the change is still annoucned as percentage, not as celsius degree. I have attached two screen shots, one is 22.4 celsicus as 44 percent, the other one is 34.1 celsius as 84 percent.

All the editorial changes look good to me.
talkback slider value as percent1
talkback slider value as percent

Copy link
Contributor

@jesdaigle jesdaigle left a comment

Choose a reason for hiding this comment

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

Everything looks good. There are still a few test failures but they're unrelated to this PR and are part of another effort to get APG into a "green" state.

Copy link
Contributor

@charmarkk charmarkk left a comment

Choose a reason for hiding this comment

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

Functions appropriately with Firefox/Chrome and NVDA/JAWS latest. HCM Black and White look good. Rock on!

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.

visual design looks good :)

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.

This is an approving Accessibility Review for VO + Safari/Chrome on macOS

Fn+up/down arrows work for PageUp/PageDown in VoiceOver the same as with VO off (i.e. temperature goes up/down by 2 degrees instead of 1/10 degree). (Also works with PageUp/PageDown using a Windows keyboard).

Fn+left/right arrows for Home/End work oddly in VoiceOver: they do change the slider value to min/max as expected, but then they move the VO cursor to unexpected locations at the beginning and end of the Thermostat group. This seems like a VO bug and not anything in the example code. (Also tried Home/End using a Windows keyboard, with the same odd results).

So, other than the weirdness with Home/End, this example works well with VoiceOver on macOS.

@mcking65 mcking65 merged commit 1379b72 into main May 23, 2021
@mcking65 mcking65 deleted the slider-temperature branch May 23, 2021 22:31
@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.

7 participants