diff --git a/src/plots/ternary/layout/axis_defaults.js b/src/plots/ternary/layout/axis_defaults.js index bc48a9b1a05..fc584be7d3e 100644 --- a/src/plots/ternary/layout/axis_defaults.js +++ b/src/plots/ternary/layout/axis_defaults.js @@ -24,10 +24,6 @@ module.exports = function supplyLayoutDefaults(containerIn, containerOut, option return Lib.coerce(containerIn, containerOut, layoutAttributes, attr, dflt); } - function coerce2(attr, dflt) { - return Lib.coerce2(containerIn, containerOut, layoutAttributes, attr, dflt); - } - containerOut.type = 'linear'; // no other types allowed for ternary var dfltColor = coerce('color'); @@ -54,10 +50,8 @@ module.exports = function supplyLayoutDefaults(containerIn, containerOut, option handleTickValueDefaults(containerIn, containerOut, coerce, 'linear'); handleTickLabelDefaults(containerIn, containerOut, coerce, 'linear', { noHover: false }); - handleTickMarkDefaults(containerIn, containerOut, coerce, 'linear', - { outerticks: false }); - - // TODO - below is a bit repetitious from cartesian still... + handleTickMarkDefaults(containerIn, containerOut, coerce, + { outerTicks: true }); var showTickLabels = coerce('showticklabels'); if(showTickLabels) { @@ -72,23 +66,17 @@ module.exports = function supplyLayoutDefaults(containerIn, containerOut, option coerce('hoverformat'); - var lineColor = coerce2('linecolor', dfltColor), - lineWidth = coerce2('linewidth'), - showLine = coerce('showline', !!lineColor || !!lineWidth); - - if(!showLine) { - delete containerOut.linecolor; - delete containerOut.linewidth; + var showLine = coerce('showline'); + if(showLine) { + coerce('linecolor', dfltColor); + coerce('linewidth'); } - // default grid color is darker here (60%, vs cartesian default ~91%) - // because the grid is not square so the eye needs heavier cues to follow - var gridColor = coerce2('gridcolor', colorMix(dfltColor, options.bgColor, 60).toRgbString()), - gridWidth = coerce2('gridwidth'), - showGridLines = coerce('showgrid', !!gridColor || !!gridWidth); - - if(!showGridLines) { - delete containerOut.gridcolor; - delete containerOut.gridwidth; + var showGridLines = coerce('showgrid'); + if(showGridLines) { + // default grid color is darker here (60%, vs cartesian default ~91%) + // because the grid is not square so the eye needs heavier cues to follow + coerce('gridcolor', colorMix(dfltColor, options.bgColor, 60).toRgbString()); + coerce('gridwidth'); } }; diff --git a/src/traces/scatter/attributes.js b/src/traces/scatter/attributes.js index a570069d77b..072af6dc6b8 100644 --- a/src/traces/scatter/attributes.js +++ b/src/traces/scatter/attributes.js @@ -152,18 +152,34 @@ module.exports = { }, fill: { valType: 'enumerated', - values: ['none', 'tozeroy', 'tozerox', 'tonexty', 'tonextx'], + values: ['none', 'tozeroy', 'tozerox', 'tonexty', 'tonextx', 'toself', 'tonext'], dflt: 'none', role: 'style', description: [ 'Sets the area to fill with a solid color.', - 'Use with `fillcolor`.' + 'Use with `fillcolor` if not *none*.', + '*tozerox* and *tozeroy* fill to x=0 and y=0 respectively.', + '*tonextx* and *tonexty* fill between the endpoints of this', + 'trace and the endpoints of the trace before it, connecting those', + 'endpoints with straight lines (to make a stacked area graph);', + 'if there is no trace before it, they behave like *tozerox* and', + '*tozeroy*.', + '*toself* connects the endpoints of the trace (or each segment', + 'of the trace if it has gaps) into a closed shape.', + '*tonext* fills the space between two traces if one completely', + 'encloses the other (eg consecutive contour lines), and behaves like', + '*toself* if there is no trace before it. *tonext* should not be', + 'used if one trace does not enclose the other.' ].join(' ') }, fillcolor: { valType: 'color', role: 'style', - description: 'Sets the fill color.' + description: [ + 'Sets the fill color.', + 'Defaults to a half-transparent variant of the line color,', + 'marker color, or marker line color, whichever is available.' + ].join(' ') }, marker: { symbol: { diff --git a/src/traces/scatter/plot.js b/src/traces/scatter/plot.js index 20f59d43516..7c10cf41732 100644 --- a/src/traces/scatter/plot.js +++ b/src/traces/scatter/plot.js @@ -41,7 +41,7 @@ module.exports = function plot(gd, plotinfo, cdscatter) { // BUILD LINES AND FILLS var prevpath = '', - tozero, tonext, nexttonext; + ownFillEl3, ownFillDir, tonext, nexttonext; scattertraces.each(function(d) { var trace = d[0].trace, @@ -49,6 +49,9 @@ module.exports = function plot(gd, plotinfo, cdscatter) { tr = d3.select(this); if(trace.visible !== true) return; + ownFillDir = trace.fill.charAt(trace.fill.length - 1); + if(ownFillDir !== 'x' && ownFillDir !== 'y') ownFillDir = ''; + d[0].node3 = tr; // store node for tweaking by selectPoints arraysToCalcdata(d); @@ -56,6 +59,7 @@ module.exports = function plot(gd, plotinfo, cdscatter) { if(!subTypes.hasLines(trace) && trace.fill === 'none') return; var thispath, + thisrevpath, // fullpath is all paths for this curve, joined together straight // across gaps, for filling fullpath = '', @@ -67,12 +71,12 @@ module.exports = function plot(gd, plotinfo, cdscatter) { // make the fill-to-zero path now, so it shows behind the line // fill to next puts the fill associated with one trace // grouped with the previous - if(trace.fill.substr(0, 6) === 'tozero' || + if(trace.fill.substr(0, 6) === 'tozero' || trace.fill === 'toself' || (trace.fill.substr(0, 2) === 'to' && !prevpath)) { - tozero = tr.append('path') + ownFillEl3 = tr.append('path') .classed('js-fill', true); } - else tozero = null; + else ownFillEl3 = null; // make the fill-to-next path now for the NEXT trace, so it shows // behind both lines. @@ -91,7 +95,15 @@ module.exports = function plot(gd, plotinfo, cdscatter) { } else if(line.shape === 'spline') { pathfn = revpathbase = function(pts) { - return Drawing.smoothopen(pts, line.smoothing); + var pLast = pts[pts.length - 1]; + if(pts[0][0] === pLast[0] && pts[0][1] === pLast[1]) { + // identical start and end points: treat it as a + // closed curve so we don't get a kink + return Drawing.smoothclosed(pts.slice(1), line.smoothing); + } + else { + return Drawing.smoothopen(pts, line.smoothing); + } }; } else { @@ -102,7 +114,7 @@ module.exports = function plot(gd, plotinfo, cdscatter) { revpathfn = function(pts) { // note: this is destructive (reverses pts in place) so can't use pts after this - return 'L' + revpathbase(pts.reverse()).substr(1); + return revpathbase(pts.reverse()); }; var segments = linePoints(d, { @@ -121,27 +133,58 @@ module.exports = function plot(gd, plotinfo, cdscatter) { for(var i = 0; i < segments.length; i++) { var pts = segments[i]; thispath = pathfn(pts); - fullpath += fullpath ? ('L' + thispath.substr(1)) : thispath; - revpath = revpathfn(pts) + revpath; + thisrevpath = revpathfn(pts); + if(!fullpath) { + fullpath = thispath; + revpath = thisrevpath; + } + else if(ownFillDir) { + fullpath += 'L' + thispath.substr(1); + revpath = thisrevpath + ('L' + revpath.substr(1)); + } + else { + fullpath += 'Z' + thispath; + revpath = thisrevpath + 'Z' + revpath; + } if(subTypes.hasLines(trace) && pts.length > 1) { tr.append('path').classed('js-line', true).attr('d', thispath); } } - if(tozero) { + if(ownFillEl3) { if(pt0 && pt1) { - if(trace.fill.charAt(trace.fill.length - 1) === 'y') { - pt0[1] = pt1[1] = ya.c2p(0, true); + if(ownFillDir) { + if(ownFillDir === 'y') { + pt0[1] = pt1[1] = ya.c2p(0, true); + } + else if(ownFillDir === 'x') { + pt0[0] = pt1[0] = xa.c2p(0, true); + } + + // fill to zero: full trace path, plus extension of + // the endpoints to the appropriate axis + ownFillEl3.attr('d', fullpath + 'L' + pt1 + 'L' + pt0 + 'Z'); } - else pt0[0] = pt1[0] = xa.c2p(0, true); - - // fill to zero: full trace path, plus extension of - // the endpoints to the appropriate axis - tozero.attr('d', fullpath + 'L' + pt1 + 'L' + pt0 + 'Z'); + // fill to self: just join the path to itself + else ownFillEl3.attr('d', fullpath + 'Z'); } } else if(trace.fill.substr(0, 6) === 'tonext' && fullpath && prevpath) { // fill to next: full trace path, plus the previous path reversed - tonext.attr('d', fullpath + prevpath + 'Z'); + if(trace.fill === 'tonext') { + // tonext: for use by concentric shapes, like manually constructed + // contours, we just add the two paths closed on themselves. + // This makes strange results if one path is *not* entirely + // inside the other, but then that is a strange usage. + tonext.attr('d', fullpath + 'Z' + prevpath + 'Z'); + } + else { + // tonextx/y: for now just connect endpoints with lines. This is + // the correct behavior if the endpoints are at the same value of + // y/x, but if they *aren't*, we should ideally do more complicated + // things depending on whether the new endpoint projects onto the + // existing curve or off the end of it + tonext.attr('d', fullpath + 'L' + prevpath.substr(1) + 'Z'); + } } prevpath = revpath; } diff --git a/src/traces/scatterternary/attributes.js b/src/traces/scatterternary/attributes.js index 697b86fcb92..2d69598209e 100644 --- a/src/traces/scatterternary/attributes.js +++ b/src/traces/scatterternary/attributes.js @@ -80,6 +80,21 @@ module.exports = { smoothing: scatterLineAttrs.smoothing }, connectgaps: scatterAttrs.connectgaps, + fill: extendFlat({}, scatterAttrs.fill, { + values: ['none', 'toself', 'tonext'], + description: [ + 'Sets the area to fill with a solid color.', + 'Use with `fillcolor` if not *none*.', + 'scatterternary has a subset of the options available to scatter.', + '*toself* connects the endpoints of the trace (or each segment', + 'of the trace if it has gaps) into a closed shape.', + '*tonext* fills the space between two traces if one completely', + 'encloses the other (eg consecutive contour lines), and behaves like', + '*toself* if there is no trace before it. *tonext* should not be', + 'used if one trace does not enclose the other.' + ].join(' ') + }), + fillcolor: scatterAttrs.fillcolor, marker: { symbol: scatterMarkerAttrs.symbol, opacity: scatterMarkerAttrs.opacity, diff --git a/src/traces/scatterternary/defaults.js b/src/traces/scatterternary/defaults.js index 8094cb3fa7e..169d922a958 100644 --- a/src/traces/scatterternary/defaults.js +++ b/src/traces/scatterternary/defaults.js @@ -17,6 +17,7 @@ var handleMarkerDefaults = require('../scatter/marker_defaults'); var handleLineDefaults = require('../scatter/line_defaults'); var handleLineShapeDefaults = require('../scatter/line_shape_defaults'); var handleTextDefaults = require('../scatter/text_defaults'); +var handleFillColorDefaults = require('../scatter/fillcolor_defaults'); var attributes = require('./attributes'); @@ -84,8 +85,11 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout coerce('marker.maxdisplayed'); } - coerce('hoverinfo', (layout._dataLength === 1) ? 'a+b+c+text' : undefined); + coerce('fill'); + if(traceOut.fill !== 'none') { + handleFillColorDefaults(traceIn, traceOut, defaultColor, coerce); + if(!subTypes.hasLines(traceOut)) handleLineShapeDefaults(traceIn, traceOut, coerce); + } - // until 'fill' and 'fillcolor' are supported - traceOut.fill = 'none'; + coerce('hoverinfo', (layout._dataLength === 1) ? 'a+b+c+text' : undefined); }; diff --git a/test/image/baselines/plot_types.png b/test/image/baselines/plot_types.png index 2dadfb539a2..645a0528173 100644 Binary files a/test/image/baselines/plot_types.png and b/test/image/baselines/plot_types.png differ diff --git a/test/image/baselines/scatter_fill_self_next.png b/test/image/baselines/scatter_fill_self_next.png new file mode 100644 index 00000000000..4bf5797e8b1 Binary files /dev/null and b/test/image/baselines/scatter_fill_self_next.png differ diff --git a/test/image/baselines/ternary_fill.png b/test/image/baselines/ternary_fill.png new file mode 100644 index 00000000000..a31859f445c Binary files /dev/null and b/test/image/baselines/ternary_fill.png differ diff --git a/test/image/baselines/ternary_multiple.png b/test/image/baselines/ternary_multiple.png index bb6d078a5ab..fd20bdf14c6 100644 Binary files a/test/image/baselines/ternary_multiple.png and b/test/image/baselines/ternary_multiple.png differ diff --git a/test/image/mocks/scatter_fill_self_next.json b/test/image/mocks/scatter_fill_self_next.json new file mode 100644 index 00000000000..2c83c332ddd --- /dev/null +++ b/test/image/mocks/scatter_fill_self_next.json @@ -0,0 +1,25 @@ +{ + "data":[ + { + "x": [1, 2, 3, 1, null, 4, 5, 6], + "y": [2, 3, 2, 2, null, 3, 4, 3], + "fill": "tonext", + "line":{"shape": "spline"} + }, + { + "x": [-1, 4, 9, null, 0, 1, 2], + "y": [1, 6, 1, null, 5, 6, 5], + "fill": "tonext" + }, + { + "x": [6, 7, 8], + "y": [5, 6, 5], + "fill": "toself" + } + ], + "layout":{ + "title": "Fill toself and tonext", + "width": 400, + "height": 400 + } +} diff --git a/test/image/mocks/ternary_fill.json b/test/image/mocks/ternary_fill.json new file mode 100644 index 00000000000..642e69bd71a --- /dev/null +++ b/test/image/mocks/ternary_fill.json @@ -0,0 +1,26 @@ +{ + "data": [ + { + "a": [0.4, 0.4, 0.2, 0.4], + "b":[0.2, 0.4, 0.4, 0.2], + "type": "scatterternary", + "fill": "tonext" + }, + { + "a":[0.5, 0.5, 0, 0.5], + "b":[0, 0.5, 0.5, 0], + "type": "scatterternary", + "fill": "tonext" + }, + { + "a": [0.8, 0.6, 0.6, 0.8, null, 0.1, 0.1, 0.3, 0.1, null, 0.1, 0.1, 0.3, 0.1], + "b": [0.1, 0.1, 0.3, 0.1, null, 0.8, 0.6, 0.6, 0.8, null, 0.3, 0.1, 0.1, 0.3], + "type": "scatterternary", + "fill": "toself" + } + ], + "layout": { + "height": 400, + "width": 500 + } +}