Image Carousel Example: Use buttons for controls and make rotation control always visible

[jongund]

Resolves issue #1007 by making the stop/start rotation button always visible and uses button elements instead of links for the next and previous slide elements.

Preview Link

The revised carousel can be seen in the issue1007-visible-pause-carousel branch.

---

Preview | Diff

[css-meeting-bot]

The ARIA Authoring Practices (APG) Task Force just discussed Reviewers needed for revised carousel.

The full IRC log of that discussion

<AnnAbbott> TOPIC: Reviewers needed for revised carousel
<AnnAbbott> PR 1018: https://github.com//pull/1018
<zcorpan> GitHub: https://github.com//pull/1018
<AnnAbbott> mck: issues for Windows, Mac & iOS
<jongund> https://raw.githack.com/w3c/aria-practices/issue1007-visible-pause-carousel/examples/carousel/carousel-1/carousel-1.html
<AnnAbbott> mck: goal is fully reviewed by May 7 and ready to merge
<MarkMcCarthy> I cannot volunteer at that this time, we have several all hands projects tying me up. Sorry!
<AnnAbbott> Jemma volunteered
<AnnAbbott> jn volunteered
<AnnAbbott> mck: other issues should go into 971

[carmacleod]

Please note this issue comment re: size: #971 (comment)

[spectranaut]

I left some commons in a test file! If it's ok, I'm going to follow up with a commit to fix it to help out.

[spectranaut]

hey @jongund! I added a commit to fix up the existing attribute tests. I also added a couple tests and have two observations:

I noticed a bug with aria-live. The table description does not match the behavoir. aria-live is set to off on page load, but after interacting with the widget in any way (hovering mouse, moving focus), aria-level is set to polite and it remains that way after focus leaves the widget and the automatic transitions begin again. Unless you click pause, in which case it changes to off (which seems to be incorrect according to the table, which states The live region is off when the carousel is automatically rotating.). The test for aria-live is failing as it test what is described in the table.

You also might notice the test I wrote for aria-disabled is failing as well. Using selenium test driver, I'm having trouble replicating moving the focus from the "previous" button to the "pause" button (which should remove aria-disabled according to the table) -- I'm not sure if this is a bug in the example, but I can look into it tomorrow morning. Writing this test made me think that the use of aria-disabled here could be confusing, as the button will always work if you navigate to it or click it. Specifically, the button will turn off the automatic rotation off once you leave the carousel region regardless of whether the automatic transition is currently happen. Maybe there is something more to "aria-disabled" than I realize, but is it really necessary for this widget?

[jongund]

@spectranaut I will look into the aria-live issue, it should be pretty easy to fix

The aria-disabled is used instead of disable property so the button stays in tab order for screen reader users. When focus is in the carousel the carousel automatically pauses, so the pause button is essentially disabled.

[spectranaut]

Ok I added more tests for keyboard interaction, and I notice the Keyboard Interaction table needs to be updated now that the "pause" button no longer hides. Other than that it looks good :)

[mcking65]

@jongund wrote:

@spectranaut I will look into the aria-live issue, it should be pretty easy to fix

Excellent, thank you! Since we never want it live when auto-rotating, can the change of aria-live be triggered based on changes to the auto-rotation state regardless of which user event causes rotation to stop or start?

The aria-disabled is used instead of disable property so the button stays in tab order for screen reader users. When focus is in the carousel the carousel automatically pauses, so the pause button is essentially disabled.

After discussing with Valerie and Jemma, we concluded that we can remove aria-disabled because, in practice, the button is never disabled. It is still operable with the mouse, screen reader reading cursor, or touch cursor even when rotation is paused automatically. And, it is useful to be able to click or tap it even when automatically paused; that causes the carousel to stay stopped.

[jongund]

@spectranaut Thank you for adding the regression tests I will make the changes for disabled button and the live region

[jongund]

@spectranaut @mcking65 @a11ydoer I made the changes to the code to fix the live region issue, so it is ready for review and hopefully merging and I also removed the aria-disabled attribute to the "Pause" button.

[mcking65]

@jongund, thank you; this is looking really good. I fixed some editorial issues. It's ready to go now.

There is one thing we can address later: the view options are not accessible; we need to put aria-current on the link that is currently active. We don't need to hold this up for that though.