Skip to content
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

Accessibility review for logged-in state #109

Closed
boonebgorges opened this issue Dec 14, 2017 · 14 comments
Closed

Accessibility review for logged-in state #109

boonebgorges opened this issue Dec 14, 2017 · 14 comments
Milestone

Comments

@boonebgorges
Copy link
Collaborator

Previously: #108

We should do a WAVE review while logged in.

Special attention should be paid to dynamic areas, which will probably need aria-live labels. I'm not sure WAVE is smart enough to catch this.

@boonebgorges boonebgorges added this to the Fall 2018 milestone Dec 14, 2017
boonebgorges added a commit that referenced this issue Apr 19, 2018
boonebgorges added a commit that referenced this issue May 1, 2018
boonebgorges added a commit that referenced this issue May 1, 2018
boonebgorges added a commit that referenced this issue May 1, 2018
boonebgorges added a commit that referenced this issue May 1, 2018
boonebgorges added a commit that referenced this issue May 1, 2018
boonebgorges added a commit that referenced this issue May 1, 2018
boonebgorges added a commit that referenced this issue May 1, 2018
boonebgorges added a commit to openlab-at-city-tech/openlab that referenced this issue May 1, 2018
boonebgorges added a commit that referenced this issue May 2, 2018
boonebgorges added a commit to openlab-at-city-tech/openlab that referenced this issue May 2, 2018
@boonebgorges
Copy link
Collaborator Author

I've made a pass through the logged-in states and made a number of improvements.

  • I've added aria-live and related regions to the dynamic parts of the application (aria-busy, aria-atomic). I did some light testing using VoiceOver and this appears to be working as expected.
  • The aXe scanner does a better job of scanning for form labels in JS-powered form elements, like our select boxes, than the WAVE tool did in my initial scans. It identified a number of places where we either needed aria-label attributes, or aria-labelledby that point to the regular HTML label element corresponding to the original form element.
  • Fixed a number of duplicate element IDs.
  • Added navigation, banner, application, and complementary landmarks. This covers about 90% of the elements on the page.
  • Added an h1 for screen-readers to the theme.

The aXe scanner is showing a couple of remaining items that cannot be easily fixed at the moment. A summary:
a. The sidebar filters ("Select Course" etc) have an invalid aria-owns attribute; they're empty, when they ought to be excluded. This is a problem with the upstream library, and is being tracked by them. JedWatson/react-select#1667 I think we should not touch this for the time being.

b. It's reporting that the question-course-data element contrast is insufficient. See screenshot. It just barely misses the contrast ratio - @jennaspevack maybe you could have a look and provide a quick bit of guidance on a new color for this.
screenshot_2018-05-01_20-23-14
screenshot_2018-05-01_20-30-52

c. There are some best-practices failures in the sitewide footer (lack of ARIA role, etc). This is low-priority - not a violation, but a best-practice thing - and should be addressed at the OpenLab level.

d. The tool is reporting a couple issues with the WordPress toolbar (tabindex on the 'Skip to toolbar' link, some minor contrast issues). We should not address this, as it's minor and it's a WP issue.

Once we have addressed (b), I think we can pass things along to Bree and/or EA for some review (in conjunction with the changes in #114).

@jennaspevack
Copy link
Collaborator

@boonebgorges I think 555555 would be good for question-course-data and for .ww-subtitle section, which seems to pass WAVE checker, but not the actual contrast ratio. Maybe that has to do with the font size.

Also noticing that the link color on the Help and About pages is still light green, which doesn't pass. That should be the same blue 0044af as the home page.

boonebgorges added a commit that referenced this issue May 4, 2018
boonebgorges added a commit to openlab-at-city-tech/openlab that referenced this issue May 4, 2018
boonebgorges added a commit to openlab-at-city-tech/openlab that referenced this issue May 4, 2018
@boonebgorges
Copy link
Collaborator Author

Thanks, @jennaspevack - I've made the #555 changes, and also changed the selectors so that the link color should be consistent throughout.

@jennaspevack
Copy link
Collaborator

Thanks, @boonebgorges - looks like we need to change the hover state for consistency too. It's just black.

@boonebgorges
Copy link
Collaborator Author

@jennaspevack Could you please clarify which element you mean? I'm looking around and I see a couple of things that are black on hover, but I'm not sure which of them needs to be changed, and what they need to be changed to. Maybe a class name so that I could look into it?

@jennaspevack
Copy link
Collaborator

Sorry, @boonebgorges I was referring to the links on About and Help. You had changed the link color from light green to 0044af blue above, but the hover associated with those links is still teal. I believe the color is 1abc9c. It should be changed to black.
screenshot 2018-05-08 11 43 26

boonebgorges added a commit to openlab-at-city-tech/openlab that referenced this issue May 8, 2018
boonebgorges added a commit to openlab-at-city-tech/openlab that referenced this issue May 8, 2018
@boonebgorges
Copy link
Collaborator Author

Ah yes, thank you!! Fixed.

@moui72
Copy link

moui72 commented May 27, 2018

I'm getting 0 contrast errors now.

There are mark-up errors associated with images imported from webwork not having alt-text. See e.g., http://openlabdev.org/webwork-playground/#:problemId=local/CoordinatePlaneTrig/identify-quadrant.pg:questionId=11900

<img src="http://openlabdev.org/wp-content/uploads/2018/05/b3bcce7f-f929-3580-8529-dde333f50a25___df0de1a3-83f3-3909-9471-a462a70e44ce.png" style="outline: red dashed 2px;">

Could we perhaps add an alt tag that reads something like "Image for [problem]" where [problem] is replaced with something like the title of the view at the given URL "Problem: CoordinatePlaneTrig"?

I am not seeing any other accessibility issues at this point

@boonebgorges
Copy link
Collaborator Author

Could we perhaps add an alt tag that reads something like "Image for [problem]" where [problem] is replaced with something like the title of the view at the given URL "Problem: CoordinatePlaneTrig"?

We've leaned against doing this elsewhere on the OpenLab because, while it might fool the WAVE scanner, it doesn't actually provide any accessibility benefits. Any text that we can build programatically is likely to contain no info that would be useful to someone using a screen reader. @bree-z and @jennaspevack Does that sound correct?

@bree-z
Copy link
Collaborator

bree-z commented Jun 1, 2018

Yes, that's how I remember these discussions. @jennaspevack does that make sense to you? Thanks!

@jennaspevack
Copy link
Collaborator

Yep, that's correct.

@boonebgorges
Copy link
Collaborator Author

Awesome - I think we are ready to close this one, then. (alt should be less of an issue moving forward, thanks to #118)

@jennaspevack
Copy link
Collaborator

sorry @boonebgorges
Just found this:
Looks like the mobile > hamburger dropdown link color is blue. I think it should be white, like the main nav. (And the nav hover color doesn't seem to have any styling, but maybe we'll leave that for now unless there is something simple that makes sense to you? Perhaps just an underline?)
dropdownlinkcolor

boonebgorges added a commit to openlab-at-city-tech/openlab that referenced this issue Jun 1, 2018
boonebgorges added a commit to openlab-at-city-tech/openlab that referenced this issue Jun 1, 2018
@boonebgorges
Copy link
Collaborator Author

Thanks, @jennaspevack. I've made two changes:

  • on mobile (hamburger), links are #fff
  • on non-mobile, menu links are underlined on hover

I'll leave the ticket closed, but please reopen or report elsewhere if things need more adjustment.

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

No branches or pull requests

4 participants