Fix Accessibility and Test for Accessibility Issues #426
Conversation
refactor: a11y improvements discovered by axe Revert "test: update build to ensure test accuracy" This reverts commit e59581e. refactor: a11y improvements
miles-grant-ibigroup
force-pushed
the
fix-a11y-issues
branch
from
770121f to
e797295
Compare
binh-dam-ibigroup
left a comment
binh-dam-ibigroup
left a comment
There was a problem hiding this comment.
I suppose there is more to uncover as we go but this looks like a good start.
| @@ -1,5 +1,5 @@ | |||
| <!DOCTYPE html> | |||
| <html> | |||
| <html lang="en"> | |||
There was a problem hiding this comment.
Add a FIXME for placing the correct language code when doing i18n. (I will also add it to the active i18n branch too.)
There was a problem hiding this comment.
I'd be interested to see how this applies to a website that can change languages, but I guess it's good enough to have the fixme there. Maybe just add a note that the lang attribute is there to make a11y happy.
| color: '#028602', | ||
| label: 'on time' | ||
| }, | ||
| SCHEDULED: { |
There was a problem hiding this comment.
What color are we using for scheduled departures? We may need to check that we are using the same as in https://github.com/ibi-group/otp-ui/pull/8/files for consistency.
There was a problem hiding this comment.
From what I can gather, it's inherited from otp-ui? Or am I misunderstanding what's going on here
There was a problem hiding this comment.
The color is #bbb and defined underneath and gets merged in the CSS by the Container component. Not a critical item, and I don't think the Lighthouse devtools plugin can catch that as is as these labels are not part of the initial render of stop viewers.
There was a problem hiding this comment.
If withBackground is false, isn't it inherited? Otherwise yes I agree. May have to do this on a project by project basis. For what it's worth, Firefox dev tools can do color contrast checking even on subsequent renders!
binh-dam-ibigroup
assigned
evansiroky and
miles-grant-ibigroup and unassigned
binh-dam-ibigroup and
evansiroky
|
Thank you for the feedback! All is resolved aside from a small question about the scheduled colors. From what I can tell based on looking at the inspector, if no color is set the color is inherited from the otp-ui css. |
Should be ok for now, so you can pass it to the next reviewer. |
miles-grant-ibigroup
assigned
evansiroky and
miles-grant-ibigroup and unassigned
binh-dam-ibigroup and
evansiroky
|
Assigning this back to myself after today's discussion, will add the puppeteer fixes first. |
| console.log('Built OTP-RR') | ||
|
|
||
| // Launch mock OTP server | ||
| const MOCK_SERVER_PORT = 9999 |
There was a problem hiding this comment.
This port won't work with the ports allowed with our localhost Mapbox token.
There was a problem hiding this comment.
Leaflet fails a11y tests and can't really be fixed from within this repo (the copyright at the bottom) so it should be alright?
evansiroky
left a comment
evansiroky
left a comment
There was a problem hiding this comment.
Needs an update so secrets and live endpoints are obtained from environment variables and not from plain-text files.
|
Have addressed these pressing items. Sorry again about writing that config to this repository. |
evansiroky
left a comment
evansiroky
left a comment
There was a problem hiding this comment.
Looks good, just needs those updated otp-ui versions.
|
Assigned to @miles-grant-ibigroup for integrating new otp-ui releases. |
|
🎉 This PR is included in version 3.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
In tandem with opentripplanner/otp-ui#283 makes colors within OTP-RR conform with WCAG.
A few notes: