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

Use of named colours in scatterGL is not case-insenstive #6685

Closed
dsmmcken opened this issue Jul 25, 2023 · 5 comments · Fixed by #6928
Closed

Use of named colours in scatterGL is not case-insenstive #6685

dsmmcken opened this issue Jul 25, 2023 · 5 comments · Fixed by #6928
Labels
bug something broken

Comments

@dsmmcken
Copy link

dsmmcken commented Jul 25, 2023

When using CSS colour names for a marker color, they should be case insensitive. I expected LemonChiffon to works as it is valid CSS3 colour, and works in svg, however a scatterGL trace renders it as black.

As usual for CSS-defined idents, all of these [color] keywords are ASCII case-insensitive.

A simple fix would be to apply a lowercase before setting colours in the scattergl code, though I am not entirely sure why this isn't working currently.

Codepen:
https://codepen.io/dsmmcken/pen/JjeambK

image

@28raining
Copy link
Contributor

The issue is inside this package:
https://www.npmjs.com/package/color-parse

which utilizes this package:
https://www.npmjs.com/package/color-name

where we can see all the string -> color code translation is in lower case

Created this issue -> colorjs/color-parse#5

@28raining
Copy link
Contributor

@archmoj looks like you're a maintainer of that repo. If you agree with this analysis, let's create a new version

@28raining
Copy link
Contributor

note that the package dependency path is

ScatterGl -> plot.js -> regl-line2d -> color-normalize -> color-rgba -> color-parse

@archmoj
Copy link
Contributor

archmoj commented Sep 15, 2023

@archmoj looks like you're a maintainer of that repo. If you agree with this analysis, let's create a new version

@28raining Yes please feel free to submit a PR. Thank you!

@28raining
Copy link
Contributor

Added to PR #6725

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants