Skip to content

Colorscale opacity #422

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

Merged
merged 5 commits into from
Apr 15, 2016
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions src/components/colorbar/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
'use strict';

var d3 = require('d3');
var tc = require('tinycolor2');
Copy link
Contributor

Choose a reason for hiding this comment

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

🐄 we've been using var tinycolor = require('tinycolor2'); in other files. Let's stay consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahhh whoops - there's a few places with var tc = tinycolor('rgb(1,2,3)'); that I was head-referencing.

Copy link
Contributor

@etpinard etpinard Apr 15, 2016

Choose a reason for hiding this comment

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

Yeah, you're right:

image

var tinycolor = require('tinycolor2'); is the winner by a big margin though:"

image

Wanna change the other var tc too?

Copy link
Contributor

Choose a reason for hiding this comment

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

scratch that comment ⏫

those var tc refer to tinycolor output, not ref to the tinycolor module. No change require.


var Plotly = require('../../plotly');
var Plots = require('../../plots/plots');
Expand Down Expand Up @@ -356,13 +357,22 @@ module.exports = function draw(gd, id) {
if(i!==filllevels.length-1) {
z[1] += (z[1]>z[0]) ? 1 : -1;
}


// Tinycolor can't handle exponents and
// at this scale, removing it makes no difference.
var colorString = fillcolormap(d).replace('e-', ''),
opaqueColor = tc(colorString).toHexString();

// Colorbar cannot currently support opacities so we
// use an opaque fill even when alpha channels present
d3.select(this).attr({
x: xLeft,
width: Math.max(thickPx,2),
y: d3.min(z),
height: Math.max(d3.max(z)-d3.min(z),2)
})
.style('fill',fillcolormap(d));
height: Math.max(d3.max(z)-d3.min(z),2),
fill: opaqueColor
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a little hacky, but the alternative will require a substantial amount of work refactoring the colorbar component so it can use native svg gradients.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm cool with this.

But to make sure we don't forget about this hack, could you write an issue about refactoring colorbar/draw.js , possibly using SVG gradients.

});
});

var lines = container.select('.cblines')
Expand Down
10 changes: 6 additions & 4 deletions src/components/colorscale/make_scale_function.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,20 @@ module.exports = function makeScaleFunction(scl, cmin, cmax) {
for(var i = 0; i < N; i++) {
si = scl[i];
domain[i] = cmin + si[0] * (cmax - cmin);
range[i] = si[1];
range[i] = tinycolor(si[1]).toRgb();
Copy link
Contributor

Choose a reason for hiding this comment

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

brilliant ✨

}

var sclFunc = d3.scale.linear()
.domain(domain)
.interpolate(d3.interpolateRgb)
.interpolate(d3.interpolateObject)
.range(range);

return function(v) {
if(isNumeric(v)) {
var sclVal = Lib.constrain(v, cmin, cmax);
return sclFunc(sclVal);
var sclVal = Lib.constrain(v, cmin, cmax),
colorObj = sclFunc(sclVal);

return tinycolor(colorObj).toRgbString();
}
else if(tinycolor(v).isValid()) return v;
else return Color.defaultLine;
Expand Down
Binary file added test/image/baselines/colorscale_opacity.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
56 changes: 56 additions & 0 deletions test/image/mocks/colorscale_opacity.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
{
"data": [
{
"y": [
5,
5,
5,
5
],
"mode": "markers",
"marker": {
"size": 40,
"colorscale": [
[
0,
"rgb(255,0.0,0.0)"
],
[
1,
"rgba(0.0,0.0,255,0.5)"
]
],
"color": [
0,
1,
2,
3
],
"cmin": 0,
"cmax": 3
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious to see how the colorbar looks in this case.

Could you add a color bar to this mock? Adding marker.color.colorbar = {}; should do the trick.

},
"uid": "07bab4"
}
],
"layout": {
"title": "Scatter Plot with a Color Dimension",
"xaxis": {
"range": [
-0.271356783919598,
3.271356783919598
],
"autorange": true
},
"yaxis": {
"type": "linear",
"range": [
4,
6
],
"autorange": true
},
"height": 450,
"width": 1100,
"autosize": true
}
}
4 changes: 2 additions & 2 deletions test/jasmine/tests/colorscale_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -389,9 +389,9 @@ describe('Test colorscale:', function() {
color4 = scaleFunction(4);

expect(color1).toEqual(color2);
expect(color1).toEqual('#050aac');
expect(color1).toEqual('rgb(5, 10, 172)');
expect(color3).toEqual(color4);
expect(color4).toEqual('#b20a1c');
expect(color4).toEqual('rgb(178, 10, 28)');
});
});
});