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

Detect duplicate colours #4183

Merged
merged 2 commits into from
Jul 6, 2018
Merged

Detect duplicate colours #4183

merged 2 commits into from
Jul 6, 2018

Conversation

lildude
Copy link
Member

@lildude lildude commented Jul 5, 2018

Description

As discovered in #4182, our current testing allows the reuse of colours as our test wasn't catering for the fact that in the event the two colours are the same, the result from past_threshold?() would fail and include the same language twice, one for the newly added language and one for the duplicate.

This PR addresses that.

With the change to the test I discovered we have four duplicates:

$ bundle exec ruby test/test_color_proximity.rb
Run options: --seed 55279

# Running:

F

Finished in 0.932629s, 1.0722 runs/s, 1.0722 assertions/s.

  1) Failure:
TestColorProximity#test_color_proximity [test/test_color_proximity.rb:21]:
The following 8 languages have failing color thresholds. Please modify the hex color.
- Glyph (#e4cc98) is too close to ["e4cc98", "e4cc98"]
- Harbour (#0e60e3) is too close to ["0e60e3", "0e60e3"]
- KRL (#28431f) is too close to ["28431f", "28431f"]
- NCL (#28431f) is too close to ["28431f", "28431f"]
- Perl (#0298c3) is too close to ["0298c3", "0298c3"]
- Ring (#0e60e3) is too close to ["0e60e3", "0e60e3"]
- Tcl (#e4cc98) is too close to ["e4cc98", "e4cc98"]
- VCL (#0298c3) is too close to ["0298c3", "0298c3"]

1 runs, 1 assertions, 1 failures, 0 errors, 0 skips
$

I've adjusted those slightly by picking the language that had its colour set most recently and then:

  • loaded that colour into Google's colour picker,
  • added or subtracted at least 16 (the minimum acceptable distance) from the colour,
  • verified it looked similar to the original colour,
  • updated languages.yml with the colour,
  • ran just the proximity tests: bundle exec ruby test/test_color_proximity.rb,
  • rinsed and repeated, adjusting the difference by a few here and there, until I got a non-clashing colour.

When that didn't work or was taking too long, I dragged the colour cursor ever-so-slightly and did the same thing again.

The results:

Glyph: #e4cc98 -> #c1ac7f
KRL: #28431f -> #28430A
Ring: #0e60e3 -> #2D54CB
VCL: #0298c3 -> #148AA8

I think those are close enough to the originals and certainly meet our proximity requirements.

Checklist:

  • I am adding new or changing current functionality fixing a test
    • [N/A] I have added or updated the tests for the new or changed functionality.

Copy link
Collaborator

@Alhadis Alhadis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little surprised that this remained under our noses for this long, haha.

LGTM! 👍

@pchaigno
Copy link
Contributor

pchaigno commented Jul 5, 2018

I'm a little surprised that this remained under our noses for this long, haha.

I mean, what's the chance two contributors will pick the exact same color? unless they copy paste...

@lildude
Copy link
Member Author

lildude commented Jul 6, 2018

I mean, what's the chance two contributors will pick the exact same color? unless they copy paste...

I’d normally say low, but Ring is a case of a very recent example and #4182 is an example of doing so just to keep the tests happy 😆

@lildude lildude merged commit 98e0a5d into master Jul 6, 2018
@lildude lildude deleted the lildude/detect-dup-colours branch July 6, 2018 07:24
@aaronfranke
Copy link
Contributor

aaronfranke commented Jul 8, 2018

You mentioned using Google's color picker to find alternative colors. Do you know if there's a tool for finding a "nearby" suitable/non-conflicting color? As you mentioned, blue is a fairly popular color, so this might not be easy just guess-and-checking unless there's a tool.

EDIT: I found a suitable color, but such a tool may be useful for others.

@lildude
Copy link
Member Author

lildude commented Jul 9, 2018

Do you know if there's a tool for finding a "nearby" suitable/non-conflicting color?

Unfortunately not. 😞

@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants