hotfix(a11y): fix for aria-hidden not applying to slides properly#46
hotfix(a11y): fix for aria-hidden not applying to slides properly#46pimdewit wants to merge 2 commits intociampo:masterfrom
Conversation
Pull Request Test Coverage Report for Build 65
💛 - Coveralls |
ce29b99 to
a9cc297
Compare
a9cc297 to
444e6f5
Compare
ciampo
left a comment
There was a problem hiding this comment.
Using the inert property instead of the attribute seems like a good choice.
Just have a look at the comment I made, and then we can merge the PR
src/macro-carousel/macro-carousel.js
Outdated
| slideEl.setAttribute(ATTRS.STANDARD.TABINDEX, -1); | ||
| } else { | ||
| slideEl.setAttribute(ATTRS.STANDARD.INERT, ''); | ||
| slideEl.setAttribute(ATTRS.STANDARD.TABINDEX, '-1'); |
There was a problem hiding this comment.
Judging at the comment just before the if, it looks like I left a bug in those lines.
I think, if isSlideInView is true, the tabindex attribute should be removed (instead of being set to -1)
There was a problem hiding this comment.
I think its currently working as intended, we want to give tabindex -1 so we can programmatically focus the slide right?
There was a problem hiding this comment.
wicg-inert seems to do some magic with tabindex too.. 😕
95983ec to
1fea50d
Compare
|
I've updated the PR - we set the tabindex in So basically I have removed the aria-hidden and tabindex logic, since |
|
What happens now though is:
This logic conflicts with the slides having Do these changes affect at all the bit where the newly selected slide is programmatically focused? |

#43
Turns out that
inert-polyfill(used by the demos) has a side-effect if you apply it by callingsetAttribute('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