macro-carousel icon indicating copy to clipboard operation
macro-carousel copied to clipboard

hotfix(a11y): fix for aria-hidden not applying to slides properly

Open pimdewit opened this issue 5 years ago • 3 comments

https://github.com/ciampo/macro-carousel/issues/43 Turns out that inert-polyfill (used by the demos) has a side-effect if you apply it by calling setAttribute('inert', true). It takes a frame before aria-hidden is applied. You can omit this by setting inert the "native" way; element.inert = flag.

This is not a bug on our end, but it looks like this fix would not hurt

pimdewit avatar Sep 17 '19 11:09 pimdewit

Pull Request Test Coverage Report for Build 65

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 27 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.8%) to 90.114%

Files with Coverage Reduction New Missed Lines %
dist/macro-carousel-test.js 27 90.11%
<!-- Total: 27
Totals Coverage Status
Change from base Build 62: -0.8%
Covered Lines: 534
Relevant Lines: 573

💛 - Coveralls

coveralls avatar Sep 17 '19 11:09 coveralls

I've updated the PR - we set the tabindex in _onSlidesSlotChange to -1 to ensure we can programmatically focus the slides. If inert=true, the element will not be focusable regardless of the tabindex value. It will also set the semantic properties under the hood, including aria-hidden (and this is reflected through an attribute when using the wicg-inert polyfill).

So basically I have removed the aria-hidden and tabindex logic, since inert takes care of that. What do you think @ciampo?

pimdewit avatar Oct 08 '19 17:10 pimdewit

What happens now though is:

  • setting the inert property to true:
    • adds the inert and aria-hidden="true" attributes
    • removes the tabindex attribute
  • setting the inert property to false:
    • removes the inert and aria-hidden attributes
    • adds the tabindex="-1" attribute

This logic conflicts with the slides having tabindex="-1" set in the _onSlidesSlotChange function — we should probably remove this bit of code and handle this accessibility aspect all through the inert property?

Do these changes affect at all the bit where the newly selected slide is programmatically focused?

ciampo avatar Oct 09 '19 00:10 ciampo