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

Color Blindness Contrast #43

Closed
wants to merge 2 commits into from
Closed

Color Blindness Contrast #43

wants to merge 2 commits into from

Conversation

@jcdickinson
Copy link

@jcdickinson jcdickinson commented Jul 20, 2015

Implementing color blindness contrast that simply piggy-backs off of the existing contrast code. Only one color blindness item is shown per-element because if e.g. the trichromat ("normal") validation fails
all of the others will obviously fail: it's redundant.

  • Adding color-blind package.
  • Updating contrast templates to support information about color blindness.
  • Updating contrast plugin code to assert color blindness.

Experimenting With The Changes

To get an idea of the type of output this plugin provides, simply uncomment the blindness key component in runOneType - you will then see the redundant messages per blindness type.

Screenshot

colorblind

Limitations

  • The color suggestions will have a chance of not actually resolving the assertion.
  • The way I use the color-blind package works around that the interface uses strings for params, which would cause a needless string roundtrip in a loop.
  • Anomalous trichromacy is not checked as it is merely a less severe form of dichromacy.
Jonathan Dickinson
Implementing color blindness contrast that simply piggy-backs off of the
existing contrast code. Only one color blindness item is shown
per-element because if e.g. the trichromat ("normal") validation fails
all of the others will obviously fail: it's redundant.

- Adding `color-blind` package.
- Updating contrast templates to support information about color
blindness.
- Updating contrast plugin code to assert color blindness.
@jcdickinson
Copy link
Author

@jcdickinson jcdickinson commented Jul 20, 2015

Relevant to #36 (but does not resolve it).

@jdan
Copy link
Contributor

@jdan jdan commented Jul 20, 2015

Oh wow, this is incredible - nice work! Love the icons in the description.

I've been working on a few (well, two) plugins that I'd like to classify as "experimental" for a number of reasons. Not because they're fragile or anything like that, but because they're more "out there" when compared to simple/trivial things like checking alt text. I think this fits nicely with that.

How would you feel about working this into a plugin of its own, rather than the Contrast plugin? I'll worry about the details as far as representation on the menu goes (we'll likely just have a list of plugins, a separator that says "EXPERIMENTAL" and another list). No worries about dupe code or anything at this time either.

@@ -6,6 +6,7 @@
let $ = require("jquery");
let Plugin = require("../base");
let annotate = require("../shared/annotate")("labels");
let blinder = require("../../node_modules/color-blind/lib/blind").Blind;

This comment has been minimized.

@jdan

jdan Jul 20, 2015
Contributor

You just call require("color-blind").Blind here and webpack will know where to look :)

This comment has been minimized.

@jcdickinson

jcdickinson Jul 20, 2015
Author

Fancy! Updated.

I can't use only "color-blind" as I don't use the index js from the package, but I updated it to "color-blind/lib/blind".

This comment has been minimized.

@jdan

jdan Jul 20, 2015
Contributor

Oh, you're absolutely right! I was looking at the "main" field in package.json, but that points to lib/color-blind. Anyway - thanks.

@jcdickinson
Copy link
Author

@jcdickinson jcdickinson commented Jul 20, 2015

Oh wow, this is incredible - nice work! Love the icons in the description.

Thanks :).

How would you feel about working this into a plugin of its own

No problem, should be trivial. I'll send a PR tomorrow night SAST for that - I have to head to bed.

@jcdickinson
Copy link
Author

@jcdickinson jcdickinson commented Jul 21, 2015

Closing this PR off as I'll be sending a new one.

@jdan
Copy link
Contributor

@jdan jdan commented Jul 21, 2015

Cool, thanks @jcdickinson!

FWIW I'm happy to keep this one open if you'd prefer to just push changes here. Your call, though.

@jcdickinson
Copy link
Author

@jcdickinson jcdickinson commented Jul 21, 2015

thanks

With your enthusiasm about contributions it's really no problem.

I'm happy to keep this one open if you'd prefer to just push changes here.

Would rather keep the changelog short. Besides, I got to learn some Git dark arts in the process.

This was referenced Jul 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants