-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Colorscale drawing fix #1112
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
Colorscale drawing fix #1112
Conversation
- that made color array containing identical items not draw properly.
max = -Infinity; | ||
colorArray.forEach(function(color) { | ||
if(isNumeric(color)) { | ||
if(min > color) min = +color; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block here used to override the Colorscale.calc
computations in the case where marker.cauto
(or marker.line.cauto
) was true.
🔪 🔪 🔪
Luckily, for most cases, this block here gave the same results as Colorscale.calc
. One exception coming when the computated cmin
and cmax
where equal. In that case, Colorscale.calc
spreads them apart by -0.5 and +0.5 respectively here whereas the removed block left them as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds right, do you want to test it? Could just be adding a second scale to the mock you already edited...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good call!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you want to test it?
Test what exactly? Just to be sure.
The equal cmin and cmax case is now tested via 860ccee
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test the colorscale we draw in this case, so it's clear what cmin and cmax end up being. That mock tests that the right markers show up (and I guess it's a bipolar colorscale so the center color is the same gray you would have gotten from a one-sided scale if this routine had chosen the starting color)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh ok. You mean show the colorbar. Good call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha yeah, colorscale, colorbar... you figured out what I meant :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in 0539e28
Nice. 💃 |
fixes #1109
while 🔪 a bunch of useless code 🎉