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

Docs clean up - unnecessary role attributes, callout clarifications #15125

Merged
merged 3 commits into from Nov 17, 2014

Conversation

Projects
None yet
5 participants
@patrickhlauke
Member

patrickhlauke commented Nov 14, 2014

The removal of role="form" may be considered contentious. It seems that, compared to the clear-cut case of <nav> not needing role="navigation", <form> does not explicitly get a role of form (at least, it doesn't seem to be exposed in my quick testing). So, having role="form" can be useful, but only in cases where the author wants to make a form an explicit important landmark. For this reason (requires explicit author intent), I'd leave the attribute out from the general examples here.

Fixes #15109.

@hnrch02 hnrch02 added this to the v3.3.2 milestone Nov 14, 2014

@cvrebert

This comment has been minimized.

Member

cvrebert commented Nov 14, 2014

(Fixed Markdown formatting in your description by adding backticks around <tag>s so that they're visible.)

@patrickhlauke

This comment has been minimized.

Member

patrickhlauke commented Nov 14, 2014

btw my esteemed colleague and ARIA guru @stevefaulkner confirms that those role="form" can be removed without space-time collapsing for AT users...

@XhmikosR

This comment has been minimized.

Member

XhmikosR commented Nov 14, 2014

@patrickhlauke: don't forget a rebase and squash when you are ready.

patrickhlauke added some commits Nov 14, 2014

Redundant role="navigation" on <nav>s
plus one example that still used the old <div role="navigation"> and a
fix-up of the callout as per
#15109
@patrickhlauke

This comment has been minimized.

Member

patrickhlauke commented Nov 14, 2014

You forcing me to learn git yet again, eh? Done. To avoid my usual "mega-PR" syndrome, i'll leave this at that...

@XhmikosR

This comment has been minimized.

Member

XhmikosR commented Nov 14, 2014

Looks much better now, thanks!

PS. I'm against one commit PRs too. I just like having clean, separate patches.

@XhmikosR

This comment has been minimized.

Member

XhmikosR commented Nov 17, 2014

Where are we on this guys?

@patrickhlauke

This comment has been minimized.

Member

patrickhlauke commented Nov 17, 2014

good from my end. i've double-checked with a few more colleagues in the accessibility community, and the overall consensus is that those roles are redundant

@XhmikosR

This comment has been minimized.

Member

XhmikosR commented Nov 17, 2014

Cool, then. @cvrebert: push the button!

cvrebert added a commit that referenced this pull request Nov 17, 2014

Merge pull request #15125 from patrickhlauke/docs-clean-up
Docs clean up - unnecessary role attributes, callout clarifications

@cvrebert cvrebert merged commit 450a9ec into twbs:master Nov 17, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@juthilo juthilo referenced this pull request Nov 17, 2014

Closed

v3.3.2 ship list #15103

@rs459

This comment has been minimized.

rs459 commented on docs/_includes/components/navbar.html in bab3622 Feb 12, 2015

If you remove role="navigation" you lost the landmark inside assistive technology even with nav :
🆗 Jaws is ok
🚫 NVDA says nothing
🚫 Voice-over says nothing

This comment has been minimized.

Member

patrickhlauke replied Feb 12, 2015

Interesting. I was aware that there's a bug with IE/NVDA (but if you use Chrome or Firefox with NVDA, the <nav> is correctly identified as a landmark.

nav

As for VoiceOver (assuming OS X), it seems this is a recent change in behavior/bug, as it previously worked fine (see also http://stevefaulkner.github.io/html-mapping-tests/).

As in both cases this seems related to shortcomings/bugs in AT, and since (from experience and talking to AT users) landmark-based navigation is still not prevalent for these users (mainly because many sites don't use landmarks yet, so a chicken/egg situation), I may opt to leave this change as is...

However, thanks for making me go and recheck VO (and find out that it seems to have borked itself in recent updates)

[EDIT] It also seems that VO/iOS behaves correctly and recognises <nav> as a landmark, so it's definitely a new VO/OS X bug

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