metafizzy / flickity

:leaves: Touch, responsive, flickable carousels
https://flickity.metafizzy.co
7.53k stars 602 forks source link

Not accessible when used as a content slider #776

Open runnickrun opened 6 years ago

runnickrun commented 6 years ago

Hello David, I've tried to use flickity as a content slider (e.g. where slides have actual content, links, etc.) and seems to be not accessible at all.

First of all, I understand there is not your fault because there are no stable conventions on that. But within current setup you can focus the slides that are not in the viewport which seems to be very confusing. I also created a question on StackExchange: https://ux.stackexchange.com/questions/118607/content-slider-carousel-accessibility-best-practices

Slide out of the view

Test case: https://codepen.io/runnick/pen/MXKqLY E.g. try moving slide 1 off the screen and then focus: it will focus on the link but since slide is invisible there is no cue it's focused. One workaround is to only allow focus slides that are currently on the screen.

asNavFor

Test case: https://codepen.io/runnick/pen/QxyZwL

This one also doesn't work because even if you add tabindex=0 to the navigation slide to make it focusable, Enter does nothing, and to get back to the active slide you need to tab back through all previous nav item (but that seems to be a behavior for all sliders with the navigation).

Carousel (groupCells)

I'm still researching here, but if someone has suggestions on how it should work, please do so.

Do you think you're interested in working on the solution? If not, I understand, but maybe you could provide some suggestion on what will be the best way to make it better. Thanks!

desandro commented 6 years ago

Thanks for reporting this issue.

In short, if you absolutely must have focus related UI behaviors (like focusing on an input brings that input into view), then Flickity may not be the best solution. You are likely better off with a simple scrollable container with overflow-x: scroll CSS.

Slides themselves are design designed to be focusable, but you can focus the carousel itself and press ← or →. This works for the asNavFor example.

runnickrun commented 6 years ago

My concern is rather to not to bring bring focused slide into the view but make only visible slides to be focusable. Is it something achievable?

Concerning asNavFor, while you can move it with the arrows you can't select the actual slide with the keyboard. Or even if you could it starts from the 1st one that can be out of the view. So again, making only visible slides focusable can solve that issue.

runnickrun commented 6 years ago

Found another issue concerning the prev/next navigation via tabbing. Probably should be addressed as controlling slider with ←→ not easily discoverable. Again, this is for when a slide content can be tabbed. Same for the carousel with mutliple slides (think Related Products in Amazon).

With the default setup current tab order is (as HTML order): slide ← →

So if let's say I change the slide to e.g. 2nd, and then want to focus on it, I need to go → ← slide (as slide is higher in HTML than previous button)

But should go → slide ←

Not sure if it makes sense, but desired functionality can be seen on e.g. Guardian: https://www.theguardian.com/uk#video

But this can be solved with custom controls I believe.

Enchiridion commented 6 years ago

I ran into the same issue where I'm using flickity as a content carousel and only wanted focus to work on the current slide. This is the solution I came up with: https://codepen.io/enchiridion/pen/MXpRLo

It listens for keydown and focusout events to watch where focus will be going and steers it towards elements within the current slide when needed.

With some more work it could be made into something more reusable. I only needed it to work on a single page so I haven't gone that far yet. It doesn't work correctly if the nav arrows are visible. For my case, I disabled the global arrows as all the prev/next links are within the content itself. There's helper functions so links like #next/#previous work for navigation.

desandro commented 6 years ago

Piggybacking on @Enchiridion's idea. If you would like to only enable links and inputs on the selected cell element, you can do so with some CSS using Flickity's is-selected class:

/* disable links and inputs in all cells */
.carousel-cell a,
.carousel-cell input {
  display: none;
}

/* enable links and inputs in selected cell */
.carousel-cell.is-selected a,
.carousel-cell.is-selected input {
  display: inline;
}
runnickrun commented 6 years ago

Seems to work fine so far, except cases where content has animations on it. But I think it can be fixed with some additional styles.

@Enchiridion is there a reason for moving prev/next in the slide vs global prev/next arrows?

Enchiridion commented 6 years ago

@runnickrun No, only that the original design mockups I made this for called for them to be inside rather than outside. It's being used with some parallax'd background images with the content slides floating on top and arrows on the sides would of spoiled the look.

nikolowry commented 6 years ago

A little aside for this issue -- the current use of aria-selected is invalid. It can only be used on elements that have a role of gridcell, option, row or tab.

If flickity did start implementing tabpanel roles for slides and tabs for the dots/asNavFor, you'd then be required to generate unique ids (if not already present) for elements needing to use the aria-labbelledby and aria-controls attributes.

If that is too much work or can't be implemented right away, it might be better to switch out aria-selected w aria-hidden -- but that might not be ideal due to the current tabbing logic

This SO thread seems relevant to the discussion https://stackoverflow.com/questions/50884582/aria-role-tabs-with-arrow-buttons

theenoahmason commented 6 years ago

Joing this conversation, it is possible to set the tabindex attribute of your focusable elements (or a slide itself) to -1 on all slides, then set the tabindex to 0 when the slide is focused or active.

We also show the arrows on focus when they are hidden. This allows for accessibility when you want hidden arrows.

vadim-on-github commented 5 years ago

Yes, definitely would love to see this [inablity to tab to out-of-viewport cells' contents] in a future release. For now, here's a workaround (in addition to @Enchiridion's approach) which is exactly what @theenoahmason suggested and what I've been resorting to myself:

var makeOnlyVisibleCellsFocusable: function(theSlider, cellSelector){
    var focuseableElemsSelector = 'a, button, input, textarea, [tabindex=0], select';
    theSlider.find(cellSelector).each(function(i, elem){
      jQuery(elem).find(focuseableElemsSelector).attr('tabindex', '-1');
    });
    var flickitySelElems = theSlider.data('flickity').selectedElements;
    flickitySelElems.forEach(function(elem){
      jQuery(elem).find(focuseableElemsSelector).removeAttr('tabindex');
    });
}

Then run this funciton when the slider is ready:

theSlider.on('ready.flickity', function(){
  setTimeout(function(){
    slider.makeOnlyVisibleCellsFocusable(theSlider, cellSelector);
  }, 100);
});

and every time the slider slides:

theSlider.on('change.flickity', function(){
  slider.makeOnlyVisibleCellsFocusable(theSlider, cellSelector);
});
jbascue commented 2 years ago

I was suffering from this condition of elements being focusable and not visible as well.

I arrived at a CSS-only approach where any focusable elements like buttons or links get display: none; until the parent has a is-selected class. My PostCSS looks like this:

.carousel__cell {
  .link { display: none; }

  .is-selected & {
    .link {
      .link { display: block; }
    }
  }
}
legendofmi commented 1 year ago

I was suffering from this condition of elements being focusable and not visible as well.

I arrived at a CSS-only approach where any focusable elements like buttons or links get display: none; until the parent has a is-selected class. My PostCSS looks like this:

.carousel__cell {
  .link { display: none; }

  .is-selected & {
    .link {
      .link { display: block; }
    }
  }
}

Good solution! However, the link (eg. an image inside a link) is only shown once the animation (left to right or right to left) is done. This means you won’t have the slide animation. It would be good to have an "official" solution.

drywall commented 1 month ago

FWIW, hiding in CSS wasn't appropriate for my use case, so I implement JS like so:

on: {
        change: function(index) {
          self.cleanupA11y(index);
        },
        ready: function() {
          self.cleanupA11y(0);
        }
      }

and

  // <a> elements in inactive slides should not be focusable because inactive slides are aria-hidden="true"
  // See https://dequeuniversity.com/rules/axe/4.9/aria-hidden-focus for more information
  cleanupA11y = (index) => {
    this.videos.forEach((video, i) => {
      var videoLink = video.querySelector('a');
      if (videoLink) {
        if (i !== index) {
          videoLink.setAttribute('tabindex', '-1');
        } else {
          videoLink.removeAttribute('tabindex');
        }
      }
    });
  }