#25054 closed defect (bug) (fixed)
Twenty Fourteen Accessibility fixes
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | 3.8 | Priority: | normal |
Severity: | normal | Version: | 3.8 |
Component: | Bundled Theme | Keywords: | reporter-feedback |
Focuses: | Cc: |
Description (last modified by )
Twenty Fourteen has a few accessibility issues that I've found:
- Remove title attributes on featured image permalinks and entry meta. There may be others as well.
- A few color contrast issues (our goal is a contrast ratio of 4.5:1 in text below 18px):
- gray on white (meta, blockquotes) is 3.36:1. recommended: replace with 767676
- green on white (links) is 3.13 recommended: replace with 24890d
- dark gray on black (site description & colophon) 3.7:1; change the alpha value to 0.5 for a 5.2:1 ratio
- The search button in toolbar can't be focused with the keyboard
- Dropdown menus aren't keyboard accessible (note that this is true of nearly all themes)
- There's no visible focus on tag links. They should match the focus style.
This is a first pass; please comment with any additional issues.
Attachments (14)
Change History (62)
#2
follow-up:
↓ 6
@
12 years ago
Added three patches. Note, these don't iterate on each other -- each addresses a separate problem from the list.
- 25054.titleattr.diff removes the title attributes from links.
- 25054.taglinks.diff adds focus styles to match the hover style on tag links.
- 25054.contrast.diff fixes the color contrast ratio in link text and a few other places (notably hovers and focus outlines). It does not address the same color where it's used in the search and social toggles in the top nav, since I want to get design feedback on that one.
#6
in reply to:
↑ 2
;
follow-up:
↓ 7
@
11 years ago
Replying to sabreuse:
Awesome, thank you.
- 25054.titleattr.diff removes the title attributes from links.
This one I have a question on ... shouldn't it only be removed when the anchor text is repeated in the title text? But shouldn't it be left in place when they are different?
#7
in reply to:
↑ 6
@
11 years ago
Replying to lancewillett:
Replying to sabreuse:
Awesome, thank you.
- 25054.titleattr.diff removes the title attributes from links.
This one I have a question on ... shouldn't it only be removed when the anchor text is repeated in the title text? But shouldn't it be left in place when they are different?
There's some related discussion in #24766 - the current consensus seems to be that title attributes are at best pointless and at worst somewhat harmful to accessibility. Add to that the fact that device support is all over the map, which means we can't rely on them for the information that really needs to be there.
Throughout WP, we have mix of title attributes -- those that just repeat the link text are uncontroversial. Those that very nearly repeat it (e.g. Link text: "Trackback URL". Title: "Trackback URL for your post") should be just as uncontroversial. The only real problem is with title attributes that serve as help text -- and if it's that important, the information shouldn't be in a title attribute at all, but we should consider whether the information is really needed and find a better solution.
IMO, we don't have any of that class in the theme.
#8
@
11 years ago
Do you think we could send Twenty Fourteen through an a11y review by the group? Like they did with _s
?
#9
@
11 years ago
I alerted the a11y group to this ticket in yesterday's meeting and asked for further trouble spots to be added in comments here -- if anyone has the bandwidth for a more formal audit, that would be wonderful to have!
#11
@
11 years ago
I was starting work on a keyboard focus patch for the stylesheet, but I think that the issues with that are a bit more complicated. There are many hover states that are themselves insufficient difference to pass accessibility standards. I'll need to do the a11y review before I can prep that patch, so I have a better sense of what needs to happen.
#13
follow-up:
↓ 48
@
11 years ago
Text in the sidebar appears to have low contrast, and the font is too small, so the titles, dates, and the site description are barely readable. The screenshot is from Firefox 23 (Windows).
#15
follow-ups:
↓ 17
↓ 31
@
11 years ago
All right, I've reviewed the theme extensively, and have some notes. I'll work on patches for these, of course - but if anybody else wants to pick one up, here are my notes:
These are just notes, so they may be hard to make sense of, but I can clarify if needed.
Color Contrast
values with alpha transparency that may be problematic:
- .site-description, #secondary, .site-footer, .site-footer a - color: rgba(255, 255, 255, 0.55) / #000; ( ratio + transparency: 6.27:1 (PASS, but low given very small font size...) )
- .ephemera .entry-meta - color: rgba(0, 0, 0, 0.2); (in stylesheet, but can't locate an example? ratio + transparency, if #fff background: 1.61:1 (FAIL) (#fff gives the max contrast possible for rgba value.) )
- .comment-navigation - color: rgba(0, 0, 0, 0.2); (appears to apply to text that is also hidden. regardless, as above, fails on any background.)
border-bottom on abbr/acronym very hard to see at .1 transparency; not critical, but would be nice to be able to see it - at least a little bit...
Empty link:
.attachment-featured-thumbnail has no content if no featured image assigned? Remove link if no featured image?
:hover/:focus states with insufficient contrast
many, particularly post meta links: author, permalink, comment link). Links should have :hover/:focus states that are easily discernable from their default states; contrast is one way to do this, but adding or removing text-decoration is actually better.
Repetitive text:
Leave a Comment text: remove title attribute, replace with .screen-reader-text
ditto 'Continue Reading', add title via screen-reader-text.
Form focus visibility
form :focus states - none; apply :hover and :focus states on fields, not just buttons.
Keyboard Accessibility
dropdown menus - not keyboard accessible; needs patch.
Skip link: not keyboard accessible, needs to be moved ahead of other focusable elements.
#17
in reply to:
↑ 15
;
follow-up:
↓ 18
@
11 years ago
Replying to joedolson:
All right, I've reviewed the theme extensively, and have some notes. I'll work on patches for these, of course - but if anybody else wants to pick one up, here are my notes:
Joe, I'd like to have a crack at what I perceive to be the 'easiest" one....
:hover/:focus states with insufficient contrast
many, particularly post meta links: author, permalink, comment link). Links should have :hover/:focus states that are easily discernable from their default states; contrast is one way to do this, but adding or removing text-decoration is actually better.
This should be able to be accomplished via strict CSS, right? I think this would be a gentle way for me to make a first contribution. Let me know if you think it involves more than CSS
#18
in reply to:
↑ 17
@
11 years ago
Replying to sharonaustin:
This should be able to be accomplished via strict CSS, right? I think this would be a gentle way for me to make a first contribution. Let me know if you think it involves more than CSS
You're right; that shouldn't involve anything but CSS. Have fun!
#19
@
11 years ago
No movement in a while. Do we have a consensus on the proposed changes? Do the patches need testing? Are any of them commit ready?
#20
@
11 years ago
I don't think there's been any conflict on the changes -- nobody has expressed any qualms about them, at any rate. There are more patches yet to be done, but as far as this thread represents, all the proposed changes are agreed to.
TheI don't know the conditions for testing patches; is there anything special that needs to be done for that? These are all pretty basic & discrete; should be readily testable.
#21
@
11 years ago
At the last accessibility chat, there were a couple of newer devs who were interested in learning to contrib to core; some of the remaining fixes look like they should be pretty do-able CSS stuff for learners.
#22
@
11 years ago
25054.searchfocus.2.diff is an update of joedolson's searchfocus diff -- it could no longer be applied after the changes in the last few weeks. As a quick housekeeping note, patches should be relative to the WordPress root directory, not the theme directory.
Social icon integration was removed at r25214, so the Social Icon focus diff isn't relevant any more.
Thanks for those two patches, Joe!
#23
@
11 years ago
25054.searchfocus.3.diff removes an unnecessary .genericons class from the css and includes a couple of code style fixes at obenland's suggestion.
#25
@
11 years ago
I think the only open change from these patches is the title attribute changes. @sabreuse if you could please refresh that one, I can get it in so we can close this ticket.
#26
@
11 years ago
Just added a new patch to this ticket that provides keyboard support for the primary navigation. Edits to style.css & theme.js
#27
follow-up:
↓ 28
@
11 years ago
Oh, heck. And that patch also included a minor spelling correction in a comment that I completely forgot that I'd made...and shouldn't be on this patch. Uploading update.
#28
in reply to:
↑ 27
@
11 years ago
Replying to joedolson:
Oh, heck. And that patch also included a minor spelling correction in a comment that I completely forgot that I'd made...and shouldn't be on this patch. Uploading update.
Thanks Joe, it tests out and works well. My only nitpicks are with core WP code style. :) I'll fix it up -- but in CSS we use one selector per line, and in JS spaces around braces and parentheses, similar to PHP code style.
#31
in reply to:
↑ 15
;
follow-up:
↓ 33
@
11 years ago
Replying to joedolson:
Color Contrast
values with alpha transparency that may be problematic:
- .ephemera .entry-meta - color: rgba(0, 0, 0, 0.2); (in stylesheet, but can't locate an example? ratio + transparency, if #fff background: 1.61:1 (FAIL) (#fff gives the max contrast possible for rgba value.) )
I think it's fine to leave this one, because the only text with that color is a pipe |
between words. If it's invisible it doesn't impact the usability on readability of any real words.
- .comment-navigation - color: rgba(0, 0, 0, 0.2); (appears to apply to text that is also hidden. regardless, as above, fails on any background.)
This color rule can be removed from the CSS, the only text this styles is hidden as .screen-reader-text
and if that's going to be shown, black is preferred (as much contrast as possible, I think).
border-bottom on abbr/acronym very hard to see at .1 transparency; not critical, but would be nice to be able to see it - at least a little bit...
It's fine to change it to be darker. #2b2b2b
should work.
Empty link:
.attachment-featured-thumbnail has no content if no featured image assigned? Remove link if no featured image?
We do want the background pattern to be there so it might require an anchor with no content (it does link to the single post, though, right? )... can you re-test now that r25735 changes are in? There should be a difference between index and single views -- only index gets the link, single does not.
In order to show the background pattern we'll need the empty element, either div
or a
.
:hover/:focus states with insufficient contrast
many, particularly post meta links: author, permalink, comment link). Links should have :hover/:focus states that are easily discernable from their default states; contrast is one way to do this, but adding or removing text-decoration is actually better.
Can you make a patch to demonstrate this?
Repetitive text:
Leave a Comment text: remove title attribute, replace with .screen-reader-text
ditto 'Continue Reading', add title via screen-reader-text.
These are both in core WP (not controlled in the theme).
Form focus visibility
form :focus states - none; apply :hover and :focus states on fields, not just buttons.
Can you give an example? I see focus states on input
elements already.
Keyboard Accessibility
Skip link: not keyboard accessible, needs to be moved ahead of other focusable elements.
Yes, this is because a div
can't be focused, it needs to be an a
element instead (props obenland for reminding me). Will fix that soon.
#33
in reply to:
↑ 31
@
11 years ago
Replying to lancewillett:
Replying to joedolson:
Color Contrast
- .comment-navigation - color: rgba(0, 0, 0, 0.2); (appears to apply to text that is also hidden. regardless, as above, fails on any background.)
Removed in r25740.
border-bottom on abbr/acronym very hard to see at .1 transparency; not critical, but would be nice to be able to see it - at least a little bit...
Fixed in r25740.
Empty link:
.attachment-featured-thumbnail has no content if no featured image assigned? Remove link if no featured image?
More notes on this here: http://core.trac.wordpress.org/ticket/25325#comment:17
I think we can add screen reader text to the empty elements.
Keyboard Accessibility
Skip link: not keyboard accessible, needs to be moved ahead of other focusable elements.
Yes, this is because a
div
can't be focused, it needs to be ana
element instead (props obenland for reminding me). Will fix that soon.
Fixed in r25742.
#35
@
11 years ago
I think we're really close to closing this one. Just a few things that need a reply from joedolson, and any other changes should be new tickets, please.
#40
follow-up:
↓ 43
@
11 years ago
- Keywords reporter-feedback added; has-patch needs-testing removed
Everything mentioned above has been addressed except:
- :hover/:focus states with insufficient contrast -- needs reporter feedback
- Repetitive text: needs a core ticket + patch
#41
@
11 years ago
@joedolson Can you also comment on http://core.trac.wordpress.org/ticket/25325#comment:25 please?
- What effect does an empty element have on a11y?
- If the element has text, but isn't focusable, is that a problem?
The latest patch there adds placeholder text so the div
or a
is never empty.
#42
follow-up:
↓ 47
@
11 years ago
Facing alot of lagging while scrolling on mozilla firefox 24.0 (Mac OS X 10.8.5).
#43
in reply to:
↑ 40
@
11 years ago
For contrast on hover/focus, I'd support toggling the underline on focus, they way we do with .more-link -- it's a clear visual transition, and probably more noticeable, especially for colorblind users, than tweaking the color contrast more.
I'll follow up with a patch.
Replying to lancewillett:
Everything mentioned above has been addressed except:
- :hover/:focus states with insufficient contrast -- needs reporter feedback
- Repetitive text: needs a core ticket + patch
#44
@
11 years ago
25054-linkdecoration.diff gives the hover state on entry text the same treatment as read more links -- underlined by default for better link recognition, remove the underline on hover to make the difference more noticeable.
#46
@
11 years ago
- Resolution set to fixed
- Status changed from new to closed
Closing this ticket, please open new issues in new tickets. Feel free to refer here for any general accessibility comment, though.
related: #24766 deals with title attributes in core functions. Patches on this ticket should only deal with title attributes that are added in theme files.