Skip to content

Conversation

@mkfreeman
Copy link
Contributor

The lighter color of the boolean color scales is difficult to see against a white background. This branch updates the default scale. See this notebook to compare results.

Screen Shot 2022-01-25 at 5 29 37 PM

@zanarmstrong
Copy link

+1 to this feature!

(how light the hue was came up in data viz dev pairing this morning, and @mkfreeman ran with it!)

@mkfreeman
Copy link
Contributor Author

mkfreeman commented Jan 25, 2022

I see that I need to update the test (2) plot colorSchemesOrdinal:), looking into that now.

mkfreeman and others added 2 commits January 25, 2022 18:04
Co-authored-by: Mike Bostock <mbostock@gmail.com>
@mkfreeman mkfreeman merged commit 180ad4e into main Jan 25, 2022
@mkfreeman mkfreeman deleted the mkfreeman/binary-colors branch January 25, 2022 23:20
@mbostock
Copy link
Member

This PR looks good. I’ve also now enabled branch restrictions for this repo to make it explicit that approval is expected before merging. 🙏

@mkfreeman
Copy link
Contributor Author

Thanks! I wasn’t sure if the review above was explicit approval, but was recently told that the Author of the PR should merge (so when I saw the button enabled, I thought it was on me).

@mbostock
Copy link
Member

No, that was a comment. An approval looks like this:

Screen Shot 2022-01-25 at 3 49 56 PM

You should merge, but only after it’s approved. (I’ve now made that restriction explicit.)

@zanarmstrong
Copy link

zanarmstrong commented Jan 26, 2022

Just curious about the motivation for the switch from:
if (n === 2) return [scheme[5][1], scheme[5][3]];
to
if (n === 2) return [scheme[3][1], scheme[3][2]]
?

The most important goal (to me) was to avoid the lightest end of the spectrum, which is now solved for*.

But, sometimes the dark end is pretty dark too. So, when we were testing things, I thought that [5][1] / [5][3] was a nice balance between getting a lot of intensity difference between the two boolean colors while also avoiding the darkest darks and lightest lights.

All in all, I'm super happy with this change and am 100% fine leaving it as if (n === 2) return [scheme[3][1], scheme[3][2]].

That said, I am curious to understand why you prefered [3][1]/[3][2] to [5][1]/[5][3] :). Thank you!

* except the Magma / Viridis / Plasma family which go from dark-to-light by default instead of light-to-dark like "greys", etc.

@mkfreeman
Copy link
Contributor Author

This was @mbostock's suggestion, but to me it [3][1] - [3][2] feels less arbitrary: [5][1] - [5][3] felt like a guess (I still wasn't sure that the colors were dark enough).

@Fil
Copy link
Contributor

Fil commented Jan 26, 2022

The general hand-wavy argument is that if a scheme is good for 3 colors, then a pair extracted from that scheme should be good for 2 colors?

@zanarmstrong
Copy link

sounds good - thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants