aria-current in NavLink #4707 #4708
Conversation
Line 32 has to be `aria-current` instead of `ariaCurrent`, otherwise React will strip it out.
|
This seems sensible to me, but I'm not an a11y expert. Maybe @ryanflorence would know more? |
|
It would be great to have this in 🙂 Maybe I can help with some references: Using the aria-current attribute - by Léonie Watson, January 14th 2017 Linked on that post: Examples and support as of January 2017 (actually, I see now they updated the support matrix) With the release of NVDA 2017.2 a few days ago, three of the major screen readers, in combination with different browsers, have full support for |
| } | ||
|
|
||
| NavLink.defaultProps = { | ||
| activeClassName: 'active' | ||
| activeClassName: 'active', | ||
| ariaCurrent: 'true' |
There was a problem hiding this comment.
Should this be page by default? That seems to be the most common use case within a routing library.
There was a problem hiding this comment.
I suggested true because it's the least specific (and therefore the most widely applicable) value, but you could be right. My thought was that in some cases, it's not a page that's active like in the case of a nested route (think an onboarding flow that might have multiple steps in one similar view, ergo step).
Totally open to some more informed opinions of course, I'm just speculating.
|
Testing with Safari + VoiceOver, here's the difference between
|
|
Well, if people don't like it, it can be overridden pretty easily. Thanks! |
* Update packages to react 16 * Upgrade Flow to v0.61.0 * Fragment ALL the things * Use ariaCurrent and refactor navlink props - See: remix-run/react-router#4708 and https://tink.uk/using-the-aria-current-attribute/ * Update more dependencies * Extract dom mock code to test utils * Remove window.requestAnimationFrame from mocks * Fix lint
Line 32 has to be
aria-currentinstead ofariaCurrent, otherwise React will strip it out.