Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

[Bug 750268] The content inside of #tabzilla-contents should not recieve focus when tabzilla is closed #23

Merged
merged 18 commits into from May 18, 2012

Conversation

Projects
None yet
3 participants
Contributor

icaaq commented May 11, 2012

This pull request hides the content inside of #tabzilla-contents, it will make it much easier for keyboard users to navigate the page when the tabzilla is used, when the page has loaded and the user tab once, then the focus will be targeted on the visible tabzilla-button. Not on 15 links + 1 input field that are "hidden" for these users.

If the user clicks the tabzilla-button, the tabzilla section opens and the taborder will begin at the first link in that section. If the user wants to close the section again you can click enter again (before you have tabbed) or press the esc key whenever.

I've also added title and some aria-attributes to make the open/close states more clear for screen reader users. These titles are up for debate maybe ;)

Last but not least I've added a off screen positioned label for the search field so screenreader users understans what they are supposed to type in that input field.

For "regular" mouse users the behavior will be exactly as before.

It all can be tested here http://icaaq.com/sandbox/mozilla/tabzilla.htm

Contributor

gauthierm commented May 12, 2012

Does it work properly for the CSS-transition alternative to jQuery animations?

Does it fail gracefully when jQuery is not available?

Contributor

icaaq commented May 12, 2012

@gauthierm:

Does it work properly for the CSS-transition alternative to jQuery animations?

Yes

Does it fail gracefully when jQuery is not available?

Yes

Contributor

icaaq commented May 12, 2012

@gauthierm: regarding your question on "Does it fail gracefully when jQuery is not available". The following original code will not work in that case:

if (Tabzilla.hasCSSTransitions) {
    Tabzilla.$panel.addClass('tabzilla-opened');
    Tabzilla.$link.addClass('tabzilla-opened');
    Tabzilla.$panel.removeClass('tabzilla-closed');
    Tabzilla.$link.removeClass('tabzilla-closed');
} else {
Contributor

gauthierm commented May 12, 2012

I forgot that we're loading jQuery dynamically if it's not already loaded, so it shouldn't matter if jQuery is available.

The changeset looks great. I've tried out your demo and the improvements are excellent. I'll merge it after confirming with Steven Garrity.

The link title text will likely need to be tweaked, but that can be done after merging. Thanks for your work!

Contributor

icaaq commented May 13, 2012

@gauthierm @sgarrity Wouldn't it be better if it were more consistent? like always use jquery eventhandlers etc. or the other way around, always use native JS?

Contributor

gauthierm commented May 14, 2012

The first version was written so if you didn't have jQuery it still worked as some special Mozilla pages didn't include jQuery for performance reasons.

Soon, it became apparent that jQuery was needed and we added dynamic loading of jQuery if it was not already loaded. The event handling in Tabzilla was written before that change and can safely be updated to always use jQuery (except for the DOMContentLoaded event in run()).

Contributor

icaaq commented May 15, 2012

I reverted back to a more jqueryfied version ;)

@sgarrity sgarrity added a commit that referenced this pull request May 18, 2012

@sgarrity sgarrity Merge pull request #23 from icaaq/a11y-2
[Bug 750268] The content inside of #tabzilla-contents should not recieve focus when tabzilla is closed
4175bf1

@sgarrity sgarrity merged commit 4175bf1 into mozilla:master May 18, 2012

Contributor

icaaq commented May 18, 2012

a-w-e-s-o-m-e!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment