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

Hiding tabs not screenreader friendly #14348

Closed
Willem-Siebe opened this issue Aug 7, 2014 · 11 comments · Fixed by #14444 or #14673
Closed

Hiding tabs not screenreader friendly #14348

Willem-Siebe opened this issue Aug 7, 2014 · 11 comments · Fixed by #14444 or #14673
Labels
Milestone

Comments

@Willem-Siebe
Copy link

I see that hidden tabs use display:none, but for the same reason why you have introduced .hidden for toggling content, this is not enough... So, in my opinion the hidden tabs should use the same CSS as .hidden.

@juthilo juthilo added the css label Aug 7, 2014
@cvrebert cvrebert added this to the v3.2.1 milestone Aug 7, 2014
@Willem-Siebe
Copy link
Author

Same for collapse.

@mdo
Copy link
Member

mdo commented Aug 28, 2014

Simply adding the CSS wasn't enough here. Need some JS perhaps to go with @fat?

@mdo mdo added the js label Aug 29, 2014
@fat
Copy link
Member

fat commented Sep 7, 2014

hm i don't follow – what do you want me to do, happy to make a change

@hnrch02
Copy link
Collaborator

hnrch02 commented Sep 7, 2014

Tab accessibility is provided by #14154 and collapse accessibility by #14147 / #14153.

@Willem-Siebe
Copy link
Author

Nice, but do you also apply the correct CSS in that changes now? With that, I mean the same CSS that is used for .hidden.

@hnrch02
Copy link
Collaborator

hnrch02 commented Sep 8, 2014

@Willem-Siebe dfa2759 did that but according to @mdo that broke the plugins.

@mdo
Copy link
Member

mdo commented Sep 8, 2014

Yeah, for some reason the transitions were borked when I added visibility into the mix. Maybe I just missed something and didn't add it to the transitions that change display/height?

mdo added a commit that referenced this issue Sep 23, 2014
Saranya-Raaj pushed a commit to Saranya-Raaj/bootstrap that referenced this issue Oct 9, 2014
stempler pushed a commit to stempler/bootstrap that referenced this issue Nov 4, 2014
stempler pushed a commit to stempler/bootstrap that referenced this issue Nov 4, 2014
@j-r-t
Copy link

j-r-t commented Nov 21, 2014

Why has visibility been added exactly? It has caused problems for me.

If one wants to animate the nav bar (eg fade in) and visibility has been set on this, it causes the element to be shown before the animation has even started.

@Willem-Siebe
Copy link
Author

@jessertaylor , to make it screenreader friendly, just like .hidden. You really have to dig into that issue to understand. Anyway.... this commit 7dd72d8 deleted the visibility: hidden !important; part from .hidden, so I'm not sure if the work being done here is still usefull, have to dig into the issue again and read the twitter discussion about why it is removed first. But I already write it down here in case somebody missed the commit!

@patrickhlauke
Copy link
Member

In all current screen reader/browser combinations (barring a few very edge-casey bugs), once something's display:none'd it's effectively removed from the DOM, as far as assistive technologies are concerned. so visibility:hidden is not necessary anymore. or...what issue was it that you were originally trying to address here?

@Willem-Siebe
Copy link
Author

@patrickhlauke That was the issue Patrick, and since it's deleted now from .hidden I thought let's be so friendly to post that here if somebody missed it so that adjustments for tabs and collapse can also be made. I haven't tested this myself yet and did not read the discussion about removing visibility either, but for the same reason I asked to add visibility here it now looks like this can be deleted again. So if the community agrees and knows only display:none is enough, I'm happy with that ofcourse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
8 participants