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

Radio Group: Update coding of current examples to use APG coding practices #1878

Merged
merged 14 commits into from Sep 14, 2021

Conversation

jongund
Copy link
Contributor

@jongund jongund commented Apr 26, 2021

Update the radio button examples to:

  1. Use APG coding practices.
  2. Simplify CSS by using inline SVG images in the content property.
  3. Updated accessibility documentation to include using currentColor and forced-color-adjust with SVG to support high contrast modes of operating systems.

Issues:

  • High contrast support on Firefox

Preview radio button example
Preview radio button example using aria-activedescendant

@jongund jongund changed the title Radio button coding updates Radio Group: Update coding of current examples to use APG coding practices May 7, 2021
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

These work well in Edge, Chrome, and Firefox in both HCM Black and HCM White.
Cool that Chrome now supports WHCM out of the box!

Thanks, @jongund !

Note that if you are having HCM issues with Firefox, you have to close Firefox completely and then reopen it with HCM mode already on. Firefox needs to do some HCM initialization, so it always needs to be restarted, even when just switching from HCM White to HCM Black.

@carmacleod
Copy link
Contributor

Confirmed that Codepen works also.

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.

@jongund Thanks for adding visual clue(affordance) for hover!

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.

@jongund
We have two different wordings for the explanation of SVG coloring in high contrast mode. It looks to me like the code is the same in both examples though.

In Radio Group Example Using aria-activedescendant, accessibility features say:

To ensure the data:image/svg+xml SVG graphics in the CSS have sufficient contrast with the background when high contrast settings invert colors, the CSS currentColor value for the stroke and fill properties to synchronize the colors with text content. If specific colors were used to specify the stroke and fill properties, the color of these elements would remain the same in high contrast mode, which could lead to insufficient contrast between them and their background or even make them invisible if their color were to match the high contrast mode background.
NOTE: The data:image/svg+xml SVG elements need to set the CSS forced-color-adjust property to auto for some browsers to support the currentColor value.

In Radio Group Example Using Roving tabindex, accessibility features say:

To ensure the borders of the inline SVG radio button graphics in the CSS 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 radio button borders are set to match the foreground color of high contrast mode text by specifying the CSS currentColor value for the stroke property of the circle elements used to draw the radio button. To make the background of the circle elements 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.

The second version used for the roving tabindex example is more clear. However, this phrase needs to be changed as it is talking about slider elements, not radio elements:

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.

@jongund
Copy link
Contributor Author

jongund commented Aug 25, 2021

@mcking65
Since it has been awhile since I worked on this update I did some more testing of high contrast mode and found that for at least the radio group examples we do not need to use currentColor for the fill and 'strokeattributes, we only need to useforce-color-adjustset toautoon thesvg` element. This provides authors a little more stylistic freedom on the colors of the SVG graphic and still support high contrast modes. I also updated the documentation to describe what is necessary and should now be consistent for the two examples.

@mcking65
Copy link
Contributor

@jesdaigle , please review code and tests on his.

@a11ydoer, we don't need any additional screen reader testing for accessibility, but a double check of high contrast would be good. I don't think we need any other accessibility review on this.

@a11ydoer
Copy link
Contributor

@mcking65 sounds good. I will work on it this evening and finish the review.

@a11ydoer
Copy link
Contributor

@mcking65 I checked both examples with high contrast in firebox(noted as the issue in the begining) and chrome in Window 10. Both look great.

@mcking65 mcking65 merged commit 17ae4f2 into main Sep 14, 2021
@mcking65 mcking65 deleted the radio-coding-update branch September 14, 2021 17:29
@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 Sep 14, 2021
@mcking65 mcking65 added this to the 1.2 Release 1 milestone Sep 14, 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.

None yet

5 participants