Add pattern fill for histogram, bar and barpolar traces #5520
Conversation
Wow, awesome For reference, here is what Bokeh supports, including their little one-character abbreviations, for things we could add later: https://docs.bokeh.org/en/latest/docs/user_guide/styling.html#hatch-properties |
@s417-lama very nice work! I've made a few comments but I think you're off to a great start, and this will be a popular feature! |
Co-authored-by: Alex Johnson <[email protected]>
Thanks for the updates @s417-lama - this looks ready to go from my side! The |
radius = Math.sqrt(solidity * size * size / Math.PI); | ||
} else { | ||
// linear interpolation | ||
var linearFn = function(x, x0, x1, y0, y1) { |
archmoj
Mar 3, 2021
Collaborator
Could you declare this function outside the switch case please?
Could you declare this function outside the switch case please?
@@ -482,6 +670,16 @@ drawing.singlePointStyle = function(d, sel, trace, fns, gd) { | |||
if(!gradientInfo[gradientType]) gradientType = 0; | |||
} | |||
|
|||
var getPatternAttr = function(mp, i, dflt) { |
archmoj
Mar 3, 2021
Collaborator
Non-blocking:
It looks like you could also move this function above drawing.pointStyle
.
Non-blocking:
It looks like you could also move this function above drawing.pointStyle
.
var getPatternAttr = function(mp, i, dflt) { | ||
if(mp && Array.isArray(mp)) { | ||
if(i < mp.length) return mp[i]; | ||
else return dflt; |
archmoj
Mar 3, 2021
Collaborator
How about simply
return i < mp.length ? mp[i] : dflt;
How about simply
return i < mp.length ? mp[i] : dflt;
|
||
var fillColor = d0.mc || marker.color; | ||
|
||
var getPatternAttr = function(mp, dflt) { |
archmoj
Mar 3, 2021
Collaborator
This function looks quite similar to getPatternAttr
in src/components/drawing/index.js
.
Could you explain the difference between two?
This function looks quite similar to getPatternAttr
in src/components/drawing/index.js
.
Could you explain the difference between two?
s417-lama
Mar 8, 2021
Author
Contributor
getPatternAttr
in legends does not require any index because the first element of the array is always used, but the same getPatternAttr
function can be used for both. If we unify them, which location do you think is appropriate for putting the common getPatternAttr
fn?
getPatternAttr
in legends does not require any index because the first element of the array is always used, but the same getPatternAttr
function can be used for both. If we unify them, which location do you think is appropriate for putting the common getPatternAttr
fn?
archmoj
Mar 8, 2021
Collaborator
It could go in drawing.
It could go in drawing.
} | ||
}); | ||
} | ||
for(k in fullLayout._gradientUrlQueryParts) queryParts.push(k); |
archmoj
Mar 3, 2021
Collaborator
Can't we use slice
instead of the for loop here and below?
Can't we use slice
instead of the for loop here and below?
s417-lama
Mar 8, 2021
Author
Contributor
In this regard, I couldn't see the point in storing query parts as a dict (_gradientUrlQueryParts
) in the first place. Seems like only keys are used and values are not. Why don't we make it just an array?
Also, I suspect that it makes things more complicated to store query parts globally. How about just putting a dedicated class name for pattern fill (and gradient) and collecting them later? Actually, I already did the similar thing (below).
in components/drawing/index.js
:
sel.classed('pattern_filled', true);
var className2query = function(s) {
return '.' + s.attr('class').replace(/\s/g, '.');
};
var k = className2query(d3.select(sel.node().parentNode)) + '>.pattern_filled';
fullLayout._patternUrlQueryParts[k] = 1;
I think it is enough to put .pattern_filled
class and collect them by using a query for .pattern_filled
when exporting svg. Maybe I am missing some points; is there any problem with it?
In this regard, I couldn't see the point in storing query parts as a dict (_gradientUrlQueryParts
) in the first place. Seems like only keys are used and values are not. Why don't we make it just an array?
Also, I suspect that it makes things more complicated to store query parts globally. How about just putting a dedicated class name for pattern fill (and gradient) and collecting them later? Actually, I already did the similar thing (below).
in components/drawing/index.js
:
sel.classed('pattern_filled', true);
var className2query = function(s) {
return '.' + s.attr('class').replace(/\s/g, '.');
};
var k = className2query(d3.select(sel.node().parentNode)) + '>.pattern_filled';
fullLayout._patternUrlQueryParts[k] = 1;
I think it is enough to put .pattern_filled
class and collect them by using a query for .pattern_filled
when exporting svg. Maybe I am missing some points; is there any problem with it?
@@ -23,6 +23,12 @@ module.exports = function handleStyleDefaults(traceIn, traceOut, coerce, default | |||
|
|||
coerce('marker.line.width'); | |||
coerce('marker.opacity'); | |||
var pattern = coerce('marker.pattern.shape'); |
archmoj
Mar 3, 2021
Collaborator
Let's call this variable patternShape
.
Let's call this variable patternShape
.
if(pattern) { | ||
coerce('marker.pattern.bgcolor'); | ||
coerce('marker.pattern.size'); | ||
coerce('marker.pattern.solidity'); |
archmoj
Mar 3, 2021
Collaborator
We need to add a supplyDefaults test somewhere here for these attributes not be coerced when there is no pattern.shape
.
We need to add a supplyDefaults test somewhere here for these attributes not be coerced when there is no pattern.shape
.
@@ -39,6 +39,54 @@ var marker = extendFlat({ | |||
max: 1, | |||
editType: 'style', | |||
description: 'Sets the opacity of the bars.' | |||
}, | |||
pattern: { |
archmoj
Mar 3, 2021
Collaborator
The bar/attributes
is required and used by other traces namely barpolar
, box
, histogram
, funnel
, waterfall
.
Are we planning to add pattern
to any of those?
The bar/attributes
is required and used by other traces namely barpolar
, box
, histogram
, funnel
, waterfall
.
Are we planning to add pattern
to any of those?
s417-lama
Mar 8, 2021
Author
Contributor
Technically it should be quite easy to apply pattern fill to other plots. I'm planning to support all of them and add some tests!
Technically it should be quite easy to apply pattern fill to other plots. I'm planning to support all of them and add some tests!
var perPointPattern = Array.isArray(markerPattern.shape) || | ||
Array.isArray(markerPattern.bgcolor) || | ||
Array.isArray(markerPattern.size) || | ||
Array.isArray(markerPattern.solidity); |
archmoj
Mar 3, 2021
Collaborator
Shouldn't we use Lib.isArrayOrTypedArray
here?
Shouldn't we use Lib.isArrayOrTypedArray
here?
var fillColor = d0.mc || marker.color; | ||
|
||
var getPatternAttr = function(mp, dflt) { | ||
if(mp && Array.isArray(mp)) { |
archmoj
Mar 3, 2021
Collaborator
Should we handle typedArrays
here as well?
Should we handle typedArrays
here as well?
This PR also added Plotly.PlotSchema.get().traces.histogram.attributes.marker.pattern So, let's add/expand |
This PR also added Drawing.pointStyle(gTrace.selectAll('path'), trace, gd); above this line plotly.js/src/traces/funnel/style.js Line 34 in ed52205 |
This PR also added Plotly.PlotSchema.get().traces.barpolar.attributes.marker.pattern So, let's add/expand |
I will handle the remaining issues (some tests and supports for plots other than bar plots) when I have time. |
We would love to include this feature in the upcoming v2.0.0-rc.1. |
@archmoj I don't think I have enough time today. Could you help fixing the remaining things? Thank you! |
Sure. I'll take care of it. |
Amazing contribution, thank you so much! |
Added pattern fill for bar plots ( #3815 ).
Although pattern fill could be applied to various types of plots, for now only bar plots are supported as a first step.
Supported attributes:
shape
: borders (/
,\
,|
,-
), cross hatch (+
,x
), dots (.
)bgcolor
: background colorscale
: scaling factor of the preset shape of patternssolidity
: line width or radius of dotsSample figure: