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

Collect feedback on auto-rotating image carousel example introduced in APG 1.1 Release 3 (January 2019) #971

Closed
1 task done
mcking65 opened this issue Jan 15, 2019 · 18 comments
Labels
Example Page Related to a page containing an example implementation of a pattern Feedback Issue raised by or for collecting input from people outside APG task force

Comments

@mcking65
Copy link
Contributor

mcking65 commented Jan 15, 2019

Collect feedback on the Feb 7, 2019 version of the Auto-Rotating Image Carousel Example
and make updates to the editors draft of the example for the July 2019 publication of APG 1.1.

Feedback Issues

Based on discussion in this issue, the following issues requesting changes to the example have been raised:

@mcking65 mcking65 added Example Page Related to a page containing an example implementation of a pattern Feedback Issue raised by or for collecting input from people outside APG task force labels Jan 15, 2019
@mcking65 mcking65 added this to Next Steps in Carousel Pattern and Examples Development Project via automation Jan 15, 2019
@mcking65 mcking65 added this to the 1.1 APG Release 4 milestone Jan 15, 2019
@shirsha
Copy link

shirsha commented Jan 31, 2019

The content over the image, and previous/next button are difficult to see due to the busyness of the image behind it. Add a background color to the buttons and insert CSS color block between the text and the image so that they can be visible even when the background image is busy.
Note: aria-roledescription is not yet supported across the browser and screen reader combination

@ghost
Copy link

ghost commented Feb 28, 2019

I think that the button to stop the automatic slide show should be visible by default. Actually, if you have an attention-deficit disorder, you're not necessarily using keyboard to navigate in websites. And you can't guess there is a button to stop the animation.

@DavidMacDonald
Copy link

DavidMacDonald commented Mar 1, 2019

At first I was intrigued by the idea of pause on hover/focus, but I think we need a visible by default pause button in addition to that functionality. The main reason for pause (WCAG 2.2.2) is so the user with a cognitive disability is not distracted when they are elsewhere on the page. The current pause functionality requires mouse or keyboard focus to remain on the component, which could be frustrating for someone who wants to do stuff elsewhere on the page without the rotating carousel distracting.

@jnurthen
Copy link
Member

jnurthen commented Mar 1, 2019

At first I was intrigued by the idea of pause on hover/focus, but I think we need a visible by default pause button in addition to that functionality. The main reason for pause (WCAG 2.2.2) is so the user with a cognitive disability is not distracted when they are elsewhere on the page. The current pause functionality requires mouse or keyboard focus to remain on the component, which could be frustrating for someone who wants to do stuff elsewhere on the page without the rotating carousel distracting.

+1
I love the Microsoft carousel. Can we use that as inspiration?

@ArnaudDelafosse
Copy link

...I think we need a visible by default pause button in addition to that functionality...

Also, I would think this is an issue for a user navigating by voice command if she/he can't see the Pause button.

@okeul
Copy link

okeul commented Mar 13, 2019

Two points :

  1. Why use a link that replicates the behavior of a button instead of using a button directly?
  2. Why use aria-label instead of a sr-only class that seems more robust?

Current code:

<a class="next carousel-control" role="button" tabindex="0" aria-controls="myCarousel-items" aria-label="Next Slide">
  <svg width="30" height="30" version="1.1" xmlns="http://www.w3.org/2000/svg">
    <polygon points="0 0 30 15 0 30" stroke="white" fill="white" stroke-width="2"></polygon>
  </svg>
</a>

Fixed code:

<button type="button" class="next carousel-control" aria-controls="myCarousel-items">
  <span class="sr-only">Next Slide</span>
  <svg width="30" height="30" version="1.1" xmlns="http://www.w3.org/2000/svg">
    <polygon points="0 0 30 15 0 30" stroke="white" fill="white" stroke-width="2"></polygon>
  </svg>
</button>

@jongund
Copy link
Contributor

jongund commented Mar 13, 2019

@okeul @jnurthen @DavidMacDonald
I can start working on adding visible pause Icon and changing the A elements to Button elements.

@scottaohara
Copy link
Member

Note that the visually hidden stop button doesn't work on iOS with VoiceOver on. you can hear the announcement, double tap to activate, but nothing happens. (believe this is a bug with iOS voiceover not activating elements hidden from view or something).

should be fixed when the pause icon becomes visible, i'd imagine. but just wanted to make mention of it...

@jhausler1266
Copy link

I know this is older, but Salesforce has a Carousel that's worth looking at for inspiration. It's based on a tabset semantic.
https://www.lightningdesignsystem.com/components/carousel/

@a11ydoer
Copy link
Contributor

a11ydoer commented May 7, 2019

@carmacleod
Copy link
Contributor

carmacleod commented May 7, 2019

@jnurthen The Microsoft carousel pause button would not pass WCAG 2.1 contrast. You can see this on the second panel. The pause button is barely discernible to the right of the dots (which are also somewhat difficult to see).

Microsoft carousel with hard-to-see pause button

@carmacleod
Copy link
Contributor

Regarding adding a 3px black border around the left and right triangles and the pause button so that they pass WCAG 2.1's Non-text Contrast SC 1.4.11, this is similar to the advice for infographics in the Understanding document for the SC.

Regarding the 3px value itself, I was remembering earlier draft versions of the SC, where they were originally thinking of requiring a 4.5:1 contrast for graphics and UI components, unless they were "thicker" - which was defined as "at least 3 CSS pixels" - then they were allowed to be 3:1.

@carmacleod
Copy link
Contributor

@jongund Some comments on the Carousel from yesterday's APG call:

  • color contrast is good now

  • general agreement that the current mouse cursor changes are "wonky" :)

    • cursor changes to pointing hand when over image, and arrow when over caption div or buttons
    • this is probably due to the image being inside a <a href="#"> link, because the user agent default cursor for links is the pointing hand (cursor: pointer), and for buttons and other things, UA's default to arrow (cursor: default)
  • so, please remove the <a href="#"> image parent from all of the images to fix the cursor confusion, and because:

    • the link is focusable, so it's confusing for both mouse click and keyboard tab traversal (i.e. why does this take focus? Can I do anything? Activating just scrolls to top of page because href="#", etc)
    • note that the "Great Britain Vote" page doesn't have the link, so it shows how we'd like it with respect to cursor changes and the image not taking focus
  • Please increase the size of the svg to 44 x 44 because of WCAG 2.1 SC 2.5.5

    • ok to keep the rect at 34 x 20
  • Nice that it stops rotating when hovered - please keep that

@jnurthen
Copy link
Member

jnurthen commented May 15, 2019

Please increase the size of the svg to 44 x 44 because of WCAG 2.1 SC 2.5.5

Note the svg size does not have to be made bigger. It is fine to have a hit area that is bigger than the actual button size. Adding padding to the button increases the hit area without making the button bigger. IMO this is fine in this case.

@carmacleod
Copy link
Contributor

@jnurthen Please note that increasing the size of the svg in this particular code has the same effect as adding padding. It doesn't make the visible button (the rect) bigger. So I think we are saying the same thing. :)

@jamesedjac
Copy link

jamesedjac commented Jul 27, 2019

I noticed that the example does not follow the authoring practices documentation for keyboard focus

The Authoring practices state that elements in the carousel receiving keyboard focus permanently pauses the carousel unless the user manually restarts it, but the example resumes the auto rotation when focus leaves the carousel:
• Authoring Practices: “If the carousel has an auto-rotate feature, automatic slide rotation stops when any element in the carousel receives keyboard focus. It does not resume unless the user activates the rotation control.”
• Auto-Rotating Image Carousel Example: “Moving keyboard focus to any of the carousel content, including the next and previous slide elements, pauses automatic rotation. Automatic rotation resumes when keyboard focus moves out of the carousel content unless another condition, such as mouse hover, that prevents rotation has been triggered.”

@jamesedjac
Copy link

I pointed this out on the issue tracker for the design pattern itself (#970), but wanted to mention it here as well and second the comment by @juliemoynat-tanaguru regarding how the hidden unless focused on pause button impacts users with ADHD.

The current example doesn't proved a way for users with ADHD who do not navigate using a keyboard to pause the content. Making the pause button visible would be a good change, but I think in addition the example should support permanently pausing the rotation if the user changes the slide using any of the carousel controls.

Pausing carousels for this reason is the most commonly supported method for pausing carousels on the web (see Amazon.com and Microsoft.com as examples). I have ADHD myself and because this method is the most common, its the first method I try when I see a carousel on the web. In addition, pause buttons can be hard to find especially because they are so rare and not consistently positioned, and because the disruption caused by the rotating slides can make this even more difficult, its important that we support expected methods that are easy to execute (next and previous buttons are consistently placed for example).

For myself, there have definitely been times where a page had a pause button, but did not pause when the slide was rotated by the user, and the slide rotation was so disruptive I could not find the pause button before needing to take a break from using the web site or application. In the case of this example, the button would be easy to find if it were visible without focus, but this example and the design pattern should still support user expected behavior here, as they set examples for others to follow that might not create such highly visible pause buttons

@mcking65
Copy link
Contributor Author

I believe we have addressed all feedback in the latest editor's draft of the carousel. If not, please raise a new issue with any bugs or suggestions.

Thank you everyone for all the wonderful input!

Carousel Pattern and Examples Development Project automation moved this from In Progress to Complete Aug 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Example Page Related to a page containing an example implementation of a pattern Feedback Issue raised by or for collecting input from people outside APG task force
Development

No branches or pull requests