feat: Switch color processing library#7536
Conversation
|
I created a couple of PRs (#78 and #81) in a dependency used by color that will add some new features for parsing color strings that would bring it closer to parity with TinyColor. The maintainer recently merged one of these and will (hopefully) be merging the other soon. Once he publishes a new release, I'll be able to use it in plotly.js. |
fc905c7 to
6e83d8a
Compare
|
@camdecoster Looks good overall, but lightened/darkened colors seem much less distinct after this change, is there any way to adjust that? Maybe increase the degree of lightening/darkening?
(codepen) |
| var c = tc.toRgb(); | ||
| return 'rgb(' + Math.round(c.r) + ', ' + | ||
| Math.round(c.g) + ', ' + Math.round(c.b) + ')'; | ||
| const _color = require('color').default; |
There was a problem hiding this comment.
| const _color = require('color').default; | |
| 'use strict'; | |
| const _color = require('color').default; |
| }; | ||
|
|
||
| color.rgb = function(cstr) { return color.tinyRGB(tinycolor(cstr)); }; | ||
| const rgb = (cstr) => { |
There was a problem hiding this comment.
Can you add a comment explaining that this is for returning a string which doesn't contain alpha? (Or whatever is the purpose of this function, I'm assuming it's that because of the comment in the old file.)
| // and convert them to rgb(0-255 values) | ||
| color.clean = function(container) { | ||
| if(!container || typeof container !== 'object') return; | ||
| const clean = (container) => { |
There was a problem hiding this comment.
I'm pretty sure we can just remove this function and its usages entirely, since it looks we deprecated the rgb(fractions) format 11(!) years ago.
|
|
||
| function cleanOne(val) { | ||
| if(isNumeric(val) || typeof val !== 'string') return val; | ||
| const cleanOne = (val) => { |
There was a problem hiding this comment.
Likewise for this function, can remove
| "- hex (e.g. '#d3d3d3', '#d3d3d3aa')", | ||
| "- rgb (e.g. 'rgb(255, 0, 0)', 'rgb(255 0 0)')", | ||
| "- rgba (e.g. 'rgba(255, 0, 0, 0.5)', 'rgba(255 0 0 / 0.5)')", | ||
| "- hsl (e.g. 'hsl(0, 100%, 50%)')", | ||
| "- hsla (e.g. 'hsla(0, 100%, 50%, 0.5)')", | ||
| "- hwb (e.g. 'hwb(0, 0%, 0%)')", |
There was a problem hiding this comment.
Weirdly, I can't find this info in the docs anywhere, although I could be missing it. We should make sure the supported color formats are documented with the next release.
| }); | ||
|
|
||
| var gridColor = coerce2('gridcolor', addOpacity(dfltColor, 0.3)); | ||
| var gridColor = coerce('gridcolor', addOpacity(dfltColor, 0.3)); |
There was a problem hiding this comment.
@camdecoster Why this change, is it intentional?
|
|
||
| color = tinycolor(color); | ||
| if(!color.isValid()) return false; | ||
| const newColor = Color.addOpacity(color, Color.opacity(color)); |
There was a problem hiding this comment.
Couldn't this just be Color.color(color).rgb().string()?
|
|
||
| for(i = 0; i < colorList.length; i++) { | ||
| colors.push(tinycolor(colorList[i]).lighten(20).toHexString()); | ||
| colors.push(Color.color(colorList[i]).lighten(0.2).hex()); |
There was a problem hiding this comment.
Re: my comment about lightened/darkened colors being too similar, I assume you could just increase this value here to fix.


Description
Switch color processing library from TinyColor to color.
Closes #7523.
Changes
rgb()/rgba()strings with decimal 0–1 fractions are now normalized to integers viaColor.clean()on inputTesting
npm run test-jasmineand verify that all tests passnpm run test-imageand verify that all image-baseline diffs match the regenerated baselines in this PRnpm run schemaproduces no diff againsttest/plot-schema.jsonScreenshots
Color.contrast,Color.brighten, andColor.addOpacityproduce slightly different RGB output. Here are some example plots that can be used to see the difference betweentinycolorandcolorwhen pasted into Plotly devtools:Auto-contrast text on a heatmap
Auto inside-text color on default colorway
Pie with extended colorway (lighten/darken)
Notes