report: Tweak colors so that we are WCAG2AA valid. #1686
Conversation
|
The red was fine, I'll check the rest when I'm back.
…On Feb 10, 2017 21:00, "Eric Bidelman" ***@***.***> wrote:
***@***.**** commented on this pull request.
Seems good. How are the red and orange values for a11y?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1686 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAVVtXbIOEjd3q7Fmn4AXO_YwrPOYdmrks5rbLPRgaJpZM4L9uUj>
.
|
|
@ebidel: the other colors had issues too with WCAG2AA. Please verify because I only tested this in theory since I don't have URLs that use |
|
Fixed one more issue with poor color. Now the patch should be final. |
XhmikosR
changed the title
|
Personally I haven't had the need to use HSL yet, but from a quick look it does seem easier to interpret indeed. This PR just makes sure the report is WCAG2AA valid, nothing more or less :) |
|
@XhmikosR gotcha, sounds good. in that case, let me defer to @robdodson on this stuff |
|
Is this for color contrast? This idea might sound like a joke but isn't: maybe in CI we should run lighthouse then do a smokehouse run against the generated report. That would let us assert color contrast, etc |
|
Yeah this is for color contrast in order to comply with WCAG2AA. I didn't go for WGAG2AAA because I thought "one step at a time". |
|
IMO we should also run HTML validation on CI because in the past it has caught important things, see #1575. |
|
Just started reviewing :) |
Sent from
|
small nit: the version number under the word "Lighthouse" is still low contrast. I think if you set it to |
|
It is valid in this branch since I've already changed it. How did you check
it?
|
|
Can someone report this guy? Doesn't seem right that one can randomly spam
PRs like that :/
|
|
@XhmikosR we've taken care of it. |
|
Doesn't seem so :p
|
|
just kidding but really now it should be taken care of. |
|
I blame the black header. |
|
Hehe. I reported him. Alternatively I guess you could lock the PRs but I won't be able to reply (I think). |
|
Now about @robdodson's comment above, https://github.com/GoogleChrome/lighthouse/pull/1686/files#diff-7c4405f5b4874ecc329c5331597912e1R616 There's the change to the footer's color. And there's no opacity in footer. |
|
Sorry, I was referring to this version number in the top left
|
|
@robdodson: I see now. But still, I don't get any errors for that chunk at all. I can change it to |
|
I think I was testing in aXe and the chrome devtools extension. What're u
using to test?
…On Tue, Feb 14, 2017, 8:58 AM XhmikosR ***@***.***> wrote:
@robdodson <https://github.com/robdodson>: I see now. But still, I don't
get any errors for that chunk at all.
I can change it to #aab3ed and remove opacity if you prefer.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1686 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABBFDfxJ-fiEt8HfAQqNufwQAumZoEBWks5rcd1DgaJpZM4L9uUj>
.
|
|
I'm using http://squizlabs.github.io/HTML_CodeSniffer/. No big deal, your change makes also sense. Just give me the final OK and I'll include it in this branch, |
|
In fact your solution won't pass WCAG2AAA (3A) if there are plans to do so in the future. |
|
I can make changes in another PR so that we are WCAG2AAA valid. The current changes are for WCAG2AA. |
|
WCAG2AA should be fine. If you want to make the change I can sign off on the PR |
|
I'd say better merge this for now and we can revisit it later. There are a
few improvements we could make later.
…On Feb 14, 2017 22:57, "Rob Dodson" ***@***.***> wrote:
WCAG2AA should be fine. If you want to make the change I can sign off on
the PR
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1686 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAVVte7mYCMlvYJfGcNmBouecV1DI15Nks5rchUvgaJpZM4L9uUj>
.
|
|
On a side note @paulirish we hit this failure on Windows quite often. Not sure what's causing it, but it seems the process can't be terminated for some reason. Otherwise this should be merged so that we can move forward with more changes. |
|
The version number being used is pretty important information. I'd like to still change it as mentioned in my prior comment before merging this PR |
|
All right I'll change it. Just keep in mind it won't pass 3a and we'll need
to change it again then. Also I personally don't see it failing.
…On Feb 15, 2017 02:08, "Rob Dodson" ***@***.***> wrote:
The version number being used is pretty important information. I'd like to
still change it as mentioned in my prior comment before merging this PR
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1686 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAVVtRYRy1eOv7PRCptnN8XHUkwAU11cks5rckICgaJpZM4L9uUj>
.
|
|
I used the color picker and Lea Verou's contrast tool to confirm it's failing We don't need to shoot for WCAG2AAA, my understanding is WCAG2AA is the generally accepted standard and AAA is for advanced cases when you know your audience might contain a larger percentage of folks who need very high contrast for example. |
|
Rebased and also fixed one new issue with |
before
after
Now the report should be WCAG2AA valid.