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.

Sign up for GitHub

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

Implement fgopacity, fgcolor & "overlay" fillmode for bars, fix bar legend and pattern with marker colorscale #5733

Merged
merged 12 commits into from
Jun 18, 2021
Merged

Implement fgopacity, fgcolor & "overlay" fillmode for bars, fix bar legend and pattern with marker colorscale #5733

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
bb8ce4e
implement pattern fgcolor and overlay fillmode
archmoj Jun 16, 2021
d6189b4
refactor and handle falsy values
archmoj Jun 16, 2021
3196777
correct pattern implementation when having colorscale on both modes
archmoj Jun 16, 2021
36dd9a8
add fgopacity and default to 0.5 for overlay fillmode
archmoj Jun 17, 2021
bd19e27
update js doc for pattern function
archmoj Jun 17, 2021
6879e51
refactor pattern arguments
archmoj Jun 17, 2021
fd89e23
apply smaller scale of patterns in legend
archmoj Jun 17, 2021
5f5561f
do not rely on patternID to detect legend
archmoj Jun 17, 2021
a2a66e7
return a default not the first point for legend when having arrays of…
archmoj Jun 17, 2021
69f91e2
in legend use the color in the middle of scale not from the first point
archmoj Jun 17, 2021
38bd5ec
in legend with pattern revise sizes
archmoj Jun 18, 2021
178e0a6
scale down patterns so that default sizes fit in legend icons
archmoj Jun 18, 2021
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
42 changes: 39 additions & 3 deletions src/components/drawing/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,46 @@ exports.pattern = {
'By default, no pattern is used for filling the area.',
].join(' ')
},
fillmode: {
valType: 'enumerated',
values: ['replace', 'overlay'],
dflt: 'replace',
editType: 'style',
description: [
'Determines whether `marker.color` should be used',
'as a default to `bgcolor` or a `fgcolor`.'
].join(' ')
},
bgcolor: {
valType: 'color',
arrayOk: true,
editType: 'style',
description: [
'Sets the background color of the pattern fill.',
'Defaults to a transparent background.',
'When there is no colorscale sets the color of background pattern fill.',
'Defaults to a `marker.color` background when `fillmode` is *overlay*.',
'Otherwise, defaults to a transparent background.'
].join(' ')
},
fgcolor: {
valType: 'color',
arrayOk: true,
editType: 'style',
description: [
'When there is no colorscale sets the color of foreground pattern fill.',
'Defaults to a `marker.color` background when `fillmode` is *replace*.',
'Otherwise, defaults to dark grey or white',
'to increase contrast with the `bgcolor`.',
].join(' ')
},
fgopacity: {
valType: 'number',
editType: 'style',
min: 0,
max: 1,
description: [
'Sets the opacity of the foreground pattern fill.',
'Defaults to a 0.5 when `fillmode` is *overlay*.',
'Otherwise, defaults to 1.'
].join(' ')
},
size: {
Expand Down Expand Up @@ -62,5 +95,8 @@ exports.pattern = {
'and solidty of 1 shows only the foreground color without pattern.',
].join(' ')
},
editType: 'style'
editType: 'style',
description: [
'Sets the pattern within the marker.'
].join(' '),
};
65 changes: 51 additions & 14 deletions src/components/drawing/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,13 +362,25 @@ drawing.gradient = function(sel, gd, gradientID, type, colorscale, prop) {
* @param {DOM element} gd: the graph div `sel` is part of
* @param {string} patternID: a unique (within this plot) identifier
* for this pattern, so that we don't create unnecessary definitions
* @param {string} bgcolor: background color for this pattern
* @param {string} fgcolor: foreground color for this pattern
* @param {number} size: size of unit squares for repetition of this pattern
* @param {number} solidity: how solid lines of this pattern are
* @param {string} prop: the property to apply to, 'fill' or 'stroke'
* @param {string} mcc: color when painted with colorscale
* @param {string} fillmode: fillmode for this pattern
* @param {string} bgcolor: background color for this pattern
* @param {string} fgcolor: foreground color for this pattern
* @param {number} fgopacity: foreground opacity for this pattern
*/
drawing.pattern = function(sel, gd, patternID, shape, bgcolor, fgcolor, size, solidity, prop) {
drawing.pattern = function(sel, gd, patternID, shape, size, solidity, mcc, fillmode, bgcolor, fgcolor, fgopacity) {
if(mcc) {
if(fillmode === 'overlay') {
bgcolor = mcc;
fgcolor = Color.contrast(bgcolor);
} else {
bgcolor = undefined;
fgcolor = mcc;
}
}

var fullLayout = gd._fullLayout;
var fullID = 'p' + fullLayout._uid + '-' + patternID;
var width, height;
Expand All @@ -392,6 +404,7 @@ drawing.pattern = function(sel, gd, patternID, shape, bgcolor, fgcolor, size, so
patternTag = 'path';
patternAttrs = {
'd': path,
'opacity': fgopacity,
'stroke': fgcolor,
'stroke-width': linewidth + 'px'
};
Expand All @@ -406,6 +419,7 @@ drawing.pattern = function(sel, gd, patternID, shape, bgcolor, fgcolor, size, so
patternTag = 'path';
patternAttrs = {
'd': path,
'opacity': fgopacity,
'stroke': fgcolor,
'stroke-width': linewidth + 'px'
};
Expand All @@ -423,6 +437,7 @@ drawing.pattern = function(sel, gd, patternID, shape, bgcolor, fgcolor, size, so
patternTag = 'path';
patternAttrs = {
'd': path,
'opacity': fgopacity,
'stroke': fgcolor,
'stroke-width': linewidth + 'px'
};
Expand All @@ -436,6 +451,7 @@ drawing.pattern = function(sel, gd, patternID, shape, bgcolor, fgcolor, size, so
patternTag = 'path';
patternAttrs = {
'd': path,
'opacity': fgopacity,
'stroke': fgcolor,
'stroke-width': linewidth + 'px'
};
Expand All @@ -449,6 +465,7 @@ drawing.pattern = function(sel, gd, patternID, shape, bgcolor, fgcolor, size, so
patternTag = 'path';
patternAttrs = {
'd': path,
'opacity': fgopacity,
'stroke': fgcolor,
'stroke-width': linewidth + 'px'
};
Expand All @@ -463,6 +480,7 @@ drawing.pattern = function(sel, gd, patternID, shape, bgcolor, fgcolor, size, so
patternTag = 'path';
patternAttrs = {
'd': path,
'opacity': fgopacity,
'stroke': fgcolor,
'stroke-width': linewidth + 'px'
};
Expand All @@ -480,14 +498,25 @@ drawing.pattern = function(sel, gd, patternID, shape, bgcolor, fgcolor, size, so
'cx': width / 2,
'cy': height / 2,
'r': radius,
'opacity': fgopacity,
'fill': fgcolor
};
break;
}

var isLegend = patternID.substr(0, 6) === 'legend';

var str = [
shape || 'noSh',
bgcolor || 'noBg',
fgcolor || 'noFg',
size,
solidity
].join(';');

var pattern = fullLayout._defs.select('.patterns')
.selectAll('#' + fullID)
.data([shape + ';' + bgcolor + ';' + fgcolor + ';' + size + ';' + solidity], Lib.identity);
.data([str], Lib.identity);

pattern.exit().remove();

Expand All @@ -500,7 +529,8 @@ drawing.pattern = function(sel, gd, patternID, shape, bgcolor, fgcolor, size, so
'id': fullID,
'width': width + 'px',
'height': height + 'px',
'patternUnits': 'userSpaceOnUse'
'patternUnits': 'userSpaceOnUse',
'patternTransform': isLegend ? 'scale(0.7)' : ''
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we adjust the size from the legend side, rather than detecting the legend inside here? Also do we need to scale down all these sizes? I'd rather we just restrict the maximum size to something we know will work, and perhaps when we have an array for pattern size just use the default size instead of the first one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good calls.
See 5f5561f, a2a66e7 and 69f91e2.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Again though, do we need to scale down to 0.7 here? In the legend style we already limit the size to 12, can't we keep the same scale as on the graph, but if 12 is too big just reduce that limit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Today in 38bd5ec I did try to avoid this scale reduction. But the problem is that the default size of 8 does not fit nicely in the legend icons and lose their identity.
So again in 178e0a6 I scaled down the patterns for icons using a greater ratio at 0.8.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the current legend items are "ok" ... better than on master, and we can always tweak them a bit more later. I really do need to get this merged and released so unless there's a hard blocker/pathological case here, can we defer the remaining work @alexcjohnson ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK

});

if(bgcolor) {
Expand All @@ -522,8 +552,8 @@ drawing.pattern = function(sel, gd, patternID, shape, bgcolor, fgcolor, size, so
.attr(patternAttrs);
});

sel.style(prop, getFullUrl(fullID, gd))
.style(prop + '-opacity', null);
sel.style('fill', getFullUrl(fullID, gd))
.style('fill-opacity', null);

sel.classed('pattern_filled', true);
var className2query = function(s) {
Expand Down Expand Up @@ -693,18 +723,25 @@ drawing.singlePointStyle = function(d, sel, trace, fns, gd) {
[[0, gradientColor], [1, fillColor]], 'fill');
} else if(patternShape) {
var patternBGColor = drawing.getPatternAttr(markerPattern.bgcolor, d.i, null);
var patternFGColor = drawing.getPatternAttr(markerPattern.fgcolor, d.i, null);
var patternFGOpacity = drawing.getPatternAttr(markerPattern.fgopacity, d.i, 1);
var patternSize = drawing.getPatternAttr(markerPattern.size, d.i, 8);
var patternSolidity = drawing.getPatternAttr(markerPattern.solidity, d.i, 0.3);
var perPointPattern = Lib.isArrayOrTypedArray(markerPattern.shape) ||
Lib.isArrayOrTypedArray(markerPattern.bgcolor) ||
Lib.isArrayOrTypedArray(markerPattern.size) ||
Lib.isArrayOrTypedArray(markerPattern.solidity);
var perPointPattern = d.mcc ||
Lib.isArrayOrTypedArray(markerPattern.shape) ||
Lib.isArrayOrTypedArray(markerPattern.bgcolor) ||
Lib.isArrayOrTypedArray(markerPattern.size) ||
Lib.isArrayOrTypedArray(markerPattern.solidity);

var patternID = trace.uid;
if(perPointPattern) patternID += '-' + d.i;

drawing.pattern(sel, gd, patternID, patternShape, patternBGColor, fillColor,
patternSize, patternSolidity, 'fill');
drawing.pattern(
sel, gd, patternID,
patternShape, patternSize, patternSolidity,
d.mcc, markerPattern.fillmode,
patternBGColor, patternFGColor, patternFGOpacity
);
} else {
Color.fill(sel, fillColor);
}
Expand Down
16 changes: 13 additions & 3 deletions src/components/legend/style.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,18 +348,28 @@ module.exports = function style(s, gd, legend) {

p.style('stroke-width', w + 'px');

var fillColor = d0.mc || marker.color;
var mcc;
if('mc' in d0) {
mcc = Drawing.tryColorscale(marker, '')(d0.mc);
}
var fillColor = mcc || d0.mc || marker.color;

var markerPattern = marker.pattern;
var patternShape = markerPattern && Drawing.getPatternAttr(markerPattern.shape, 0, '');

if(patternShape) {
var patternBGColor = Drawing.getPatternAttr(markerPattern.bgcolor, 0, null);
var patternFGColor = Drawing.getPatternAttr(markerPattern.fgcolor, 0, null);
var patternFGOpacity = Drawing.getPatternAttr(markerPattern.fgopacity, 0, 1);
var patternSize = Math.min(12, Drawing.getPatternAttr(markerPattern.size, 0, 8));
var patternSolidity = Drawing.getPatternAttr(markerPattern.solidity, 0, 0.3);
var patternID = 'legend-' + trace.uid;
p.call(Drawing.pattern, gd, patternID, patternShape, patternBGColor,
fillColor, patternSize, patternSolidity, 'fill');
p.call(
Drawing.pattern, gd, patternID,
patternShape, patternSize, patternSolidity,
mcc, markerPattern.fillmode,
patternBGColor, patternFGColor, patternFGOpacity
);
} else {
p.call(Color.fill, fillColor);
}
Expand Down
25 changes: 22 additions & 3 deletions src/lib/coerce.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ var tinycolor = require('tinycolor2');

var baseTraceAttrs = require('../plots/attributes');
var colorscales = require('../components/colorscale/scales');
var Color = require('../components/color');
var DESELECTDIM = require('../constants/interactions').DESELECTDIM;

var nestedProperty = require('./nested_property');
Expand Down Expand Up @@ -427,12 +428,30 @@ exports.coerceFont = function(coerce, attr, dfltObj) {
/*
* Shortcut to coerce the pattern attributes
*/
exports.coercePattern = function(coerce, attr) {
exports.coercePattern = function(coerce, attr, markerColor, hasMarkerColorscale) {
var shape = coerce(attr + '.shape');
if(shape) {
coerce(attr + '.size');
coerce(attr + '.bgcolor');
coerce(attr + '.solidity');
coerce(attr + '.size');
var fillmode = coerce(attr + '.fillmode');
var isOverlay = fillmode === 'overlay';

if(!hasMarkerColorscale) {
var bgcolor = coerce(attr + '.bgcolor', isOverlay ?
markerColor :
undefined
);

coerce(attr + '.fgcolor', isOverlay ?
Color.contrast(bgcolor) :
markerColor
);
}

coerce(attr + '.fgopacity', isOverlay ?
0.5 :
1
);
}
};

Expand Down
9 changes: 4 additions & 5 deletions src/traces/bar/style_defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ var colorscaleDefaults = require('../../components/colorscale/defaults');
var coercePattern = require('../../lib').coercePattern;

module.exports = function handleStyleDefaults(traceIn, traceOut, coerce, defaultColor, layout) {
coerce('marker.color', defaultColor);

if(hasColorscale(traceIn, 'marker')) {
var markerColor = coerce('marker.color', defaultColor);
var hasMarkerColorscale = hasColorscale(traceIn, 'marker');
if(hasMarkerColorscale) {
colorscaleDefaults(
traceIn, traceOut, layout, coerce, {prefix: 'marker.', cLetter: 'c'}
);
Expand All @@ -24,8 +24,7 @@ module.exports = function handleStyleDefaults(traceIn, traceOut, coerce, default

coerce('marker.line.width');
coerce('marker.opacity');
coercePattern(coerce, 'marker.pattern');

coercePattern(coerce, 'marker.pattern', markerColor, hasMarkerColorscale);
coerce('selected.marker.color');
coerce('unselected.marker.color');
};
2 changes: 2 additions & 0 deletions tasks/test_mock.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ function notBlackListed(name) {
'mathjax',
'ohlc_first',
'pattern_bars',
'pattern_fgcolor_overlay_fillmode',
'pattern_with_colorscale',
'plot_types',
'polar_blank',
'polar_dates',
Expand Down
Binary file modified test/image/baselines/pattern_bars.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added test/image/baselines/pattern_fgcolor_overlay_fillmode.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added test/image/baselines/pattern_with_colorscale.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading