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

Tab index issue with aria-hidden vs. hidden #27

Open
fThues opened this issue Apr 24, 2019 · 10 comments

Comments

Projects
None yet
3 participants
@fThues
Copy link

commented Apr 24, 2019

I see you recently changed the hidden attribute to aria-hidden. Would it be possible to render both attributes? I'm using your component in dynamic forms and it messes up the tab index. hidden will instruct the browser to not include the input when tabbing through the form, aria-hidden won't.

@danieldiekmeier

This comment has been minimized.

Copy link
Owner

commented Apr 24, 2019

Hey! We only used hidden very briefly, it quickly became clear that it messed up some edge cases if the closed element should still be partially visible. It also required additional fine tuning to make sure that the attribute was only added after the animation ran.

If you absolutely need it, you could add it yourself. This basically works:

<vue-slide-up-down :active="active" :hidden="!active">
  <input placeholder="Only tabbable if active!" />
</vue-slide-up-down>

Alternatively, if you don't want to have to fix the animation, you could

<vue-slide-up-down :active="active">
  <input placeholder="Only tabbable if active!" :tabindex="active ? 0 : -1" />
</vue-slide-up-down>
@fThues

This comment has been minimized.

Copy link
Author

commented Apr 24, 2019

That makes a lot of sense. Thanks for the detailed response and the great work!

@fThues fThues closed this Apr 24, 2019

@danieldiekmeier

This comment has been minimized.

Copy link
Owner

commented Apr 24, 2019

No problem! Grüße an die Elbe!

@martyf

This comment has been minimized.

Copy link
Contributor

commented Jun 24, 2019

Rather than create a new issue, thought I'd query this issue. I encountered the same issue as @fThues with the use of form fields and tab index.

It does go further than just visible tabbing, but also users who rely on assistive devices (such as screen readers) would use their standard keyboard navigation techniques, which encounters the same issue - while tabbing for visual users is odd, tabbing for screen reader users is totally confusing.

I do feel there is valid argument for including "hidden", and while I appreciate the workaround, on a larger scale project I'm on at the moment, does make for quite a lot of additional logic needing to be added.

I've rolled back a version to the one that uses "hidden" (as I haven't encountered any of the edge case issues that mentioned, so works for me) and tweaked to also use aria-hidden, but just wondering @danieldiekmeier can provide any more details on those edge cases where issues were occurring? It may still be worth exploring this one to create a more accessible result.

@danieldiekmeier

This comment has been minimized.

Copy link
Owner

commented Jun 24, 2019

@martyf Sure!

The edge cases are

  • hidden will immediately hide the whole element, so you won't see the whole animation
  • hidden will always hide the whole element, even if the content should be still partially visible in the active === false state (if you set min-height to something larger than 0)

I also feel the general need of adding hidden, but I'm not an expert for accessibility in any way, so I'd like to rely on external contributors, if anybody wants to find a way that does not break backwards compatibility.

Also, I haven't needed this component since 2017, so I'm not really eager to sink time into this.

If the rolled back version works for you, that's great! You could also copy the component into your code base (it's really just one file with a few lines) and make any changes for your specific use case!

Also, you could write a wrapper so you could simplify your code:

<template>
  <vue-slide-up-down :active="active" :hidden="!active">
    <slot/>
  </vue-slide-up-down>
</template>

<script>
export default {
  name: 'MySlideUpDown',

  props: {
    active: Boolean
  }
}
</script>
@martyf

This comment has been minimized.

Copy link
Contributor

commented Jun 24, 2019

Excellent that's great thank you @danieldiekmeier - I totally get where you're coming from for your needs too. I will have a play with the latest version and see if I can iron out those kinks to find the best solution for everyone. Just on deadline at the moment, but will do after that, as I find this component absolutely invaluable.

@martyf

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

I've made a branch that fixes the first issue above @danieldiekmeier, so any fields/controls within the hidden area are not accessible when it is "closed", but also has complete animations too.

The second one is more challenging though. Basically visible elements can always be access via the keyboard except if the specific element has a tabindex of -1. This means that when it is not active, every child element of the inner content needs to have tabindex of -1 applied - but also means that we need to be able to undo it too - so starts to get really complicated.

For me, I question its usefulness - if I want an element hidden, I want it hidden, not partially visible (i.e. with min height). So feel that having it partially visible does fall in to a specific use case.

Maybe the balance of the two is adding a new flag that allows the developer to disable the "hidden" attribute - with the impact of keyboard focus of internal content. It could (would) still cause accessibility issues for keyboard users, but given the element itself is visible anyway, I think falls in to a different category.

Happy to do a PR with that if you'd like - the first edge case is fixed regardless. It's the tabindex issue that stops the second from really getting bulletproof fixed.

@danieldiekmeier

This comment has been minimized.

Copy link
Owner

commented Jul 4, 2019

@martyf Thanks! I've been thinking about it and I think I agree with you. The most important use case is completely hiding the element (and then adding a hidden attribute), and the partial hiding can be an opt-in thing. I'd probably add a section to the docs about that and bump the major version.

Can you prepare a PR?

@martyf

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2019

Cool bananas - will polish up today and get it back to you.

@martyf

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2019

PR made... evolved the demo a bit to show off the different props too... wasn't in the scope, I know, but is useful to see things change in realtime. Hopefully you agree too 😇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.