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

Some blacks are not correctly ordered when ordering by color #66

Closed
1 of 4 tasks
ericcornelissen opened this issue May 16, 2021 · 18 comments · Fixed by #110
Closed
1 of 4 tasks

Some blacks are not correctly ordered when ordering by color #66

ericcornelissen opened this issue May 16, 2021 · 18 comments · Fixed by #110
Assignees
Labels
bug Something isn't working

Comments

@ericcornelissen
Copy link
Contributor

ericcornelissen commented May 16, 2021

Kind of issue

  • Feature
  • Bug
  • Design
  • Other, namely:

Description

When ordering by colour the ordering is a bit off as some black colors are displayed at the top, before the reds, while most blacks are displayed all the way at the bottom, following the whites.

Ideally, we find some sort of solution for this ordering problem. Unfortunately, in general there isn't necessarily a good solution for sorting colors. There will always be some cutoff point and it will probably never be perfect.

However, with some effort we may be able to improve the current situation. Two (possible) goals:

  • Group all "blacks" and "whites" at the end.
  • Have at least the ordering above the fold look "as expected".

Anyone looking to work on this issue, share any suggestions, research, or previews you can 🙂

Example

Below is an example of what ordering by colour looks like - using v4.8.0 of simple-icons. As you can see "GitHub" and "DS Autombiles", which have dark grey colours, appear before "The Spriters Resource" which is red, i.e.

#181717 #1D1717 #BE3939

image

@ericcornelissen ericcornelissen added the bug Something isn't working label May 16, 2021
@adamrusted
Copy link
Member

Might this be something we can calculate ourselves by turning the Hex into HSL, and ordering primarily by Hue but then by either Saturation / Brightness to suit our needs? I imagine that's what a lot of color-sorter is, but may allow us greater flexibility if it's built straight into our own code?

@ericcornelissen
Copy link
Contributor Author

Feel free to play around with it, you can inspect the source code of color-sorter (the dependency we currently use for sorting), try other color sorters, or implement something yourself. All you need to do to test this is update the value that is assigned to the sortedHexes constant in webpack.config.js.

Honestly, as a temporary fix I would be okay if we hard-code moving #181717 and #1D1717 to the end for now, if you can't find something else that works feel free to submit a Pull Request for that 😄

@ericcornelissen ericcornelissen added the good first issue Issues we believe are simple and good for newcomers label Jul 4, 2021
@jorgeamadosoria
Copy link
Contributor

jorgeamadosoria commented Aug 30, 2021

So, I've been reading about this, and this is a problem without a clear answer.
There is no way to order colors without some jumps happening from time to time.
Also, the library used even has some reference images that illustrate color jumps such as the one seen.
image

This is not so much a bug as an undesired behavior.
I don't think the library can be leveraged for something different. We would have to sort manually, use a different library, or live with this.

As I was reading, apparently this is a problem without hard solutions. Every color sorting algorithm encounters breaks in color continuity, and the only option is to choose where those are going to happen, not if.

@ericcornelissen
Copy link
Contributor Author

I agree that there isn't a "best" solution, but that doesn't mean there necessarily isn't anything we can do. I think the goal of this issue to find what we, collectively, think is the "best" solution for our use case. To get to that I would suggest anyone working on this tries to play with the configuration of our current color sorter library, and possibly try some others libraries. That way we can see what "feels" right, and perhaps narrow down on aspects we find important (e.g., the order should look "as expected" above the fold - which it currently doesn't).

I will update the issue description for clarity

@jorgeamadosoria
Copy link
Contributor

I am scouring the web for alternative algorithms. They all have pretty visible trade offs, I'm afraid.

The most promising approach I've found is hue clustering: order the colors in buckets by hue, order each bucket by luminosity, and concatenate the buckets.
It looks like a series of up and downs from white to black, except each segment is a different color.

Can someone explaon why are we sorting by color, anyways? Is this a highly used feature, or does it provide an importanr benefit I'm missing?

@ericcornelissen
Copy link
Contributor Author

I am scouring the web for alternative algorithms. They all have pretty visible trade offs, I'm afraid.

That's awesome, though I don't think we have to (or even should) go beyond using anything prebuilt that's available on npm. If you've come to the conclusion that there's really no suitable solution for the problem/example of the issue description than please do close this 🙂

The most promising approach I've found is hue clustering: order the colors in buckets by hue, order each bucket by luminosity, and concatenate the buckets.
It looks like a series of up and downs from white to black, except each segment is a different color.

I would personally prefer it the way we have now where it is (mostly) granular - with "pure" blacks and whites at the end.

Can someone explaon why are we sorting by color, anyways? Is this a highly used feature, or does it provide an importanr benefit I'm missing?

The project website originally started out that way. Following #38 we decides to switch the default to alphabetical ordering. I personally still use the colored ordering as neither colored or alphabetical seems "usefull" to me, and colored ordering is more aesthetically pleasing. If there's consensus I guess we could remove it but, at least to me, it doesn't seem like too much trouble to keep around as long as we already have it.

@jorgeamadosoria
Copy link
Contributor

http://jorgeamadosoria.info/colorsorting/

a sample of ways to order by colors.
I don't think there is a lot of improvement over color-sorter.
Maybe a library can show improvement.

@jorgeamadosoria

This comment has been minimized.

@ericcornelissen

This comment has been minimized.

@jorgeamadosoria

This comment has been minimized.

@ericcornelissen

This comment has been minimized.

@jorgeamadosoria

This comment has been minimized.

@ericcornelissen
Copy link
Contributor Author

I was thinking that categories is a good grouping for icons. Of course, they would have to be defined: Programming languages, Tech companies, crypto currencies, retailers, etc. It's an idea, but it's not something all that important. Feel free to disregard that.

If you think categories are a good idea, open an issue!

It doesn't matter, it's a bag, not an array

I understand where you're coming from from a technical perspective, but that doesn't translate clearly to displaying things on a website.

Now, going back to color sorting, is any of the alternatives in my sample better than the current alternative?

Forgot about that comment 😅 Nice work on that one! (here's a preview in case you ever decide to remove the webpage)

  • Both "Default sort implementation" and "Sorted by color distance" mix colors more than the current implementation does (which tends to mix in whites and blacks instead).
  • "Sorted by hue only" looks surprisingly good I think.
  • Both "Sorted by hue,saturation, light" and "Sorted by hue,saturation, light" looks very close to what we currently have, except with the blacks and whites moved to the front.
  • "Sorted by saturation,hue,light", "Sorted by saturation,light,hue", "Sorted by light,hue,saturation", and "Sorted by light,saturation,hue" all look pretty random I would say.
  • While "Distance by buckets every 30 degrees with in-bucket sort by lightness" is pretty ordered, I don't think the repeated dark-to-light sequences look good overall.
  • Both "Distance by buckets every 30 degrees" and "Distance by buckets every 60 degrees (primary and secondary colors buckets)" look pretty good, but not better than other options and having the blacks and whites in the middle doesn't look good I think.

All in all, I don't think any of the alternatives is better than what we have currently. Thanks a lot for putting this together though!

The only other avenue I can think of is trying to extract blacks and whites and sort the "colors" separately from the "grays" and then just append the "grays" after the "colors". I believe the previous version of our website used to do something like this based on the saturation(?). My knowledge of color theory is very limited, @jorgeamadosoria do you think you could investigate further in this direction?

@jorgeamadosoria
Copy link
Contributor

jorgeamadosoria commented Sep 3, 2021

The only other avenue I can think of is trying to extract blacks and whites and sort the "colors" separately from the "grays" and then just append the "grays" after the "colors". I believe the previous version of our website used to do something like this based on the saturation(?). My knowledge of color theory is very limited, @jorgeamadosoria do you think you could investigate further in this direction?

Yeah, I also don't think any of these is better than the current one. Maybe the buckets, but it has the issue of alternating dark to light per bucket, as you pointed out.

Now, regarding blacks, whites and grey, I guess that it is possible to mdoify the bucket algorithm to add a black, a grey and a white buckets at the end of the algorithm.
That would (in theory, depending on implementation) move all black, grey and whites, in that order, to the end of the sorted array. It would keep the dark jumps for every bucket, but would not show a very dark hard stop, rather a color would give way to the next in a softer (but not really soft!) transition.
I will work on that algorithm, it could be interesting. If anything else, it could be its own npm package :)

@jorgeamadosoria
Copy link
Contributor

jorgeamadosoria commented Sep 4, 2021

I've added a new approach: http://jorgeamadosoria.info/colorsorting/
image

Slightly better with the blacks and whites, but not perfect. Hey, at least the leading blacks are out of that position, so that's a win, I guess.
I honestly don't know how to make it better. Suggestions?

@ericcornelissen
Copy link
Contributor Author

ericcornelissen commented Sep 4, 2021

I think this is heading in the right direction, if you reverse the black and whites (so the colors are followed by whites first, then blacks) it would be satisfactory for this issue. We could play around with the exact cutoffs for blacks and whites but this is a good start. Do you think you can replace the current implementation by yours?

@jorgeamadosoria
Copy link
Contributor

I think this is heading in the right direction, if you reverse the black and whites (so the colors are followed by whites first, then blacks) it would be satisfactory for this issue. We could play around with the exact cutoffs for blacks and whites but this is a good start. Do you think you can replace the current implementation by yours?

It should be easy to invert blacks and whites. they are in different buckets so it would just be a switch.
Yes, the cutoffs for black and white do change the distribution of colors, the problem is that if you go too much in one direction, you end up with some dark colors that are not really black (think of, say, a red with a black overlay, that will be classified as black because of the luminosity). There is wiggle room, but unfortunately is not that much.

I can try and substitute the implementation and make a PR we can test live, let's see how that looks like on the web site.

@jorgeamadosoria
Copy link
Contributor

image

I've added my implementation. It looks as the image above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants