carousel Example with buttons for slide control: Improve High Contrast Support and implementation of APG Programming Practices

[jongund]

This pull request updates the current carousel example with previous and next buttons:

1. Provide view options similar to the tabbed carousel example
2. Improved color contrast support
3. Improved keyboard focus styling
4. Updated JS to new example design patterns
5. Updated documentation

Preview Link

Preview updated carousel in compare branch

Review checklist

• Editorial review by @mcking65
• Functional review by @carmacleod
• Visual design review by @a11ydoer
• Accessibility review by @charmarkk
• Code review by @spectranaut (NOTE: there is a new requirement in the code review process)
• Test review by @spectranaut

[a11ydoer]

Visual design review - When we make another version of the slider next time, I would like to recommend to change the design of the arrows, buttons for previous and next slide, to simpler ones. It would also help to prevent the strecthy look of current buttons.

We may also put pause button in the middle between previous and next so that all the controls are in one place, not pause button far left or previous and next buttons on far right.

[charmarkk]

A11y QA

• JAWS 2020.2006.12 (Note: tested on Windows 10 VM)

  • Firefox 80 OK
  • Chrome 84.0.4147.125 OK
  • Edge 84.0.522.59 OK
  • Notes:
    • For Chromium browsers, checking the Display Controls checkbox causes JAWS to announce the content of the carousel after checking the box. This does not appear to happen with remaining two checkboxes.
    • JAWS does not announce change in play/pause carousel button's name when pressed as NVDA does.

• NVDA 2020.2

  • Firefox 80 OK
  • Chrome 84.0.4147.125 OK
  • Edge 84.0.522.59 OK

• Voiceover 10

  • Safari 13.1.1 OK
    • Note: Start/stop button name updates (unlike JAWS/Chromium), but the slide names do not announce when changing slides. Otherwise, announcements and functionality work as expected.

• Windows HCM (default settings - High Contrast Black) and Color Contrast

  • Firefox 80 OK - Buttons have clear focus indication; white arrow on blue background has 7.1:1 CCR.
  • Chrome 84.0.4147.125 w/ HCM extension set to Inverted Color OK (see note)
    • Note: Buttons do appear contrasty but the images are super bright and washed out, likely due to how the extension functions. Considered OK considering Chrome's current lack of OS HCM integration.
  • Edge 84.0.522.59 OK - Buttons have clear focus indication; white arrow on blue background has 7.1:1 CCR.

Test env: Windows 10 v2004 b19041.450 and MacOS Catalina 10.15.5.

[carmacleod]

Sorry, @jongund - I meant to create a PR against your branch, but I messed up and pushed directly to your branch.
It was only editorial nits, like a space before a comma, and a space between "be come" instead of "become" (plus a few more nits).
I just recently changed from having my own fork of APG to working directly from a clone of the repo, so I guess I need to figure out how to create a PR for a branch. Sorry again!

[carmacleod]

Two of the points about the rotation control button in the Accessibility Features section may need a bit of revising to match behavior.

• If a user has activated the button to stop the show, the rotation will only restart if the button is activated. Moving keyboard focus or the mouse out of the carousel will not restart rotation.

This is correct, but we may want to add a new point describing the opposite behavior, because it might not be intuitive:

• If a user has activated the button to start the show, the rotation will only stop if the button is activated again. Moving keyboard focus or the mouse into the carousel will not stop rotation.

---

• If keyboard focus is inside the carousel, or if the mouse is hovering over the carousel, the button is disabled; it cannot be used to start rotation.

This point doesn't seem to match the current behavior. The button isn't really disabled, because it still changes from pause to play, and as soon as the mouse is moved off of the carousel, rotation starts up again.

[carmacleod]

Tested Windows HCM High Contrast White, to compliment @charmarkk's High Contrast Black test. Everything is visible in white also.

[carmacleod]

This example looks good!

I only have the questions in my previous comment about the behavior of those 2 rotation control button points.

I don't know why the build is failing on the treeview_treeview-2b regression test - I didn't touch that.

[jongund]

@carmacleod
Thanks for catching problems in the documentation. Made some edits to hopefully resolve the issue. No problem merging your edits in this branch.

[carmacleod]

@jongund Thanks for deleting the point about the disabled button - makes sense now.

Regarding adding a new point after:

• If a user has activated the button to stop the rotation, the rotation will only restart if the button is activated. Moving keyboard focus or the mouse out of the carousel will not restart rotation when the button is in the stopped state.

Here's what I had in mind for the new point:

• If a user has activated the button to start the rotation, the rotation will only stop if the button is activated again. Moving keyboard focus or the mouse into the carousel will not stop rotation when the button has re-entered the playing state.

To see this, try the following steps:

• go to https://raw.githack.com/w3c/aria-practices/update-carousel-previous-next/examples/carousel/carousel-1-prev-next.html
• make sure none of the options are checked
• refresh the page, to clear any previous state
• with the mouse, click the pause button (the button takes focus, and the rotation stops)
• hover the mouse over the slide and leave it there
• with the keyboard, type enter or space to activate the play button
  • the rotation starts, and keeps going, even though the mouse is still over the slide
• type tab to move focus to one of the other buttons or to the slide itself
  • the rotation continues, even though focus is within the carousel

I believe we did this on purpose, because if I recall correctly, we said "if the user intentionally presses the play button again after they pressed pause, then they obviously want the carousel to rotate regardless of hover or focus". But we should explain it with the new point, because it might not be completely clear what is going on there.

[carmacleod]

No problem merging your edits in this branch.

Phew - thanks, @jongund !
I figured out that all I need to do to create a pull request on your branch is to push to a new branch... I should've realized that!
Always learning! ;)

[carmacleod]

+1
The new rotation button point looks good, @jongund. Thanks!

[mcking65]

@jongund
Valerie will be able to re-review this on Thursday. Otherwise, this is ready to go. So, please do not make further changes. Assuming your responses to Valerie's review are sufficient, I will merge this on Thursday afternoon.

[spectranaut]

Code review for previously requested changes: Approved! Looks great!! Thanks so much for the readability changes @jongund !