refactor: Misc accessibility improvements #1124
Conversation
rschristian
changed the title
| class="logo" | ||
| aria-label={title} |
There was a problem hiding this comment.
.logo isn't used.
As for aria-label, it replaces a spread title prop (<svg title="...") which does nothing to my knowledge, and is used instead of <title> for two reasons:
- On hover, most browsers will show the content of
<title>as a tooltip. We don't really need or want this in the places we use this component. - FireFox's accessibility tools show that
<title>does not count as heading text in the scenario of<h1><svg><title>..., whilearia-labeldoes. This is important as we use this structure on our home page, where the main heading has no text in it, just our logo. With<title>, that heading is completely empty.
| if (to.href == '/' || to.path == '/') { | ||
| e.preventDefault(); | ||
| location.route('/branding'); | ||
| } | ||
| e.preventDefault(); | ||
| location.route('/branding'); | ||
| } | ||
| const homeProps = to.href == '/' || to.path == '/' | ||
| ? { onContextMenu: BrandingRedirect, 'aria-label': 'Home' } | ||
| : {}; |
There was a problem hiding this comment.
Better to only attach the handler to the one route it's used, and the 'aria-label': 'Home' I think is more useful of a description, even if users are probably familiar with the concept that brand/sitename == "home"
rschristian
changed the title
| <nav tabIndex={0} onFocus={this.open}> | ||
| <nav onFocus={this.open}> |
There was a problem hiding this comment.
There's no real reason a user would need to tab onto the nav container, there's nothing for them to do there. They can still tab onto the links within the container though.
| <nav | ||
| tabIndex={0} | ||
| class={cx(style.toc, !(items && items.length > 1) && style.disabled)} | ||
| > | ||
| <nav class={cx(style.toc, !(items && items.length > 1) && style.disabled)}> |
There was a problem hiding this comment.
Same here, tabbing onto the side bar has no use.
| @@ -1,27 +0,0 @@ | |||
| import marked from 'marked'; | |||
There was a problem hiding this comment.
Sorry, unrelated, but long unused.
| ? [<a href={'#' + props.id} />, props.children] | ||
| ? [ | ||
| <a href={'#' + props.id}> | ||
| <svg width="16" height="16" viewBox="0 0 24 24" aria-label={`Link to: ${props.children} (#${props.id})`}> |
There was a problem hiding this comment.
This implementation (e.g., Link to: Setting up JSX (#setting-up-jsx)) came from Vue's docs which I quite like, makes it quite clear why there's a link right alongside the heading content.
rschristian
force-pushed
the
refactor/logo-accessibility
branch
5 times, most recently
from
ecc50f2
to
4b65af0
Compare
| color: #4b5363; | ||
| color: #8b93a3; |
There was a problem hiding this comment.
Was failing contrast checks
Been trying to learn more accessibility stuff, figured I'd start here.