Bugfix/fillgradient bugs

draftlogs/8096_fix.md

@@ -0,0 +1,2 @@
+ - Fix `fillgradient` bounds in `scatter` traces after a zoom or a pan, and with a single `start` or `stop` on secondary and log axes [[#8096](https://github.com/plotly/plotly.js/pull/8096)]
+ - Fix an error for a *radial* `fillgradient` with `start` or `stop` [[#8096](https://github.com/plotly/plotly.js/pull/8096)]

src/components/drawing/index.js

@@ -250,45 +250,12 @@ function setFillStyle(sel, trace, gd, forLegend) {
             gradientID = 'legendfill-' + trace.uid;
         }
 
-        if (!forLegend && (fillgradient.start !== undefined || fillgradient.stop !== undefined)) {
-            var start, stop;
-            if (direction === 'horizontal') {
-                start = {
-                    x: fillgradient.start,
-                    y: 0
-                };
-                stop = {
-                    x: fillgradient.stop,
-                    y: 0
-                };
-            } else if (direction === 'vertical') {
-                start = {
-                    x: 0,
-                    y: fillgradient.start
-                };
-                stop = {
-                    x: 0,
-                    y: fillgradient.stop
-                };
-            }
-
-            start.x = trace._xA.c2p(start.x === undefined ? trace._extremes.x.min[0].val : start.x, true);
-            start.y = trace._yA.c2p(start.y === undefined ? trace._extremes.y.min[0].val : start.y, true);
-
-            stop.x = trace._xA.c2p(stop.x === undefined ? trace._extremes.x.max[0].val : stop.x, true);
-            stop.y = trace._yA.c2p(stop.y === undefined ? trace._extremes.y.max[0].val : stop.y, true);
-            sel.call(
-                gradientWithBounds,
-                gd,
-                gradientID,
-                'linear',
-                fillgradient.colorscale,
-                'fill',
-                start,
-                stop,
-                true,
-                false
-            );
+        if (
+            !forLegend &&
+            direction !== 'radial' &&
+            (fillgradient.start !== undefined || fillgradient.stop !== undefined)
+        ) {
+            axisGradient(sel, trace, gd, fillgradient, gradientID, 'fill');
         } else {
             if (direction === 'horizontal') {
                 direction = direction + 'reversed';
@@ -300,6 +267,47 @@ function setFillStyle(sel, trace, gd, forLegend) {
     }
 }
 
+/**
+ * Apply a linear gradient that follows one axis of a cartesian trace.
+ *
+ * @param sel - d3 selection to apply the gradient to
+ * @param trace - the full trace. It must have `_xA`, `_yA` and `_extremes`.
+ * @param gd - the graph div
+ * @param gradient - a `fillgradient` container with `type` *horizontal* or *vertical*
+ * @param gradientID - an identifier for the gradient, unique in the plot
+ * @param prop - 'fill' or 'stroke'
+ *
+ * If `gradient.start` or `gradient.stop` is undefined, the gradient uses the lowest or highest
+ * value of the trace along the axis.
+ */
+function axisGradient(sel, trace, gd, gradient, gradientID, prop) {
+    const isHorizontal = gradient.type === 'horizontal';
+    const ax = isHorizontal ? trace._xA : trace._yA;
+    const extremes = trace._extremes[ax._id];
+
+    let startPx;
+    if (gradient.start !== undefined) {
+        startPx = ax.c2p(gradient.start, true);
+    } else {
+        let min = Infinity;
+        for (const item of extremes.min) min = Math.min(min, item.val);
+        startPx = ax.l2p(min);
+    }
+
+    let stopPx;
+    if (gradient.stop !== undefined) {
+        stopPx = ax.c2p(gradient.stop, true);
+    } else {
+        let max = -Infinity;
+        for (const item of extremes.max) max = Math.max(max, item.val);
+        stopPx = ax.l2p(max);
+    }
+
+    const start = isHorizontal ? { x: startPx, y: 0 } : { x: 0, y: startPx };
+    const stop = isHorizontal ? { x: stopPx, y: 0 } : { x: 0, y: stopPx };
+    gradientWithBounds(sel, gd, gradientID, 'linear', gradient.colorscale, prop, start, stop, true, false);
+}
+
 // Same as fillGroupStyle, except in this case the selection may be a transition
 drawing.singleFillStyle = function (sel, gd) {
     var node = d3.select(sel.node());
@@ -506,7 +514,6 @@ function gradientWithBounds(sel, gd, gradientID, type, colorscale, prop, start,
         .append(info.node)
         .each(function () {
             var el = d3.select(this);
-            if (info.attrs) el.attr(info.attrs);
 
             el.attr('id', fullID);
 
@@ -523,6 +530,9 @@ function gradientWithBounds(sel, gd, gradientID, type, colorscale, prop, start,
             });
         });
 
+    // Set the bounds on every call: user-space bounds move with the axis range.
+    if (info.attrs) gradient.attr(info.attrs);
+
     sel.style(prop, getFullUrl(fullID, gd)).style(prop + '-opacity', null);
 
     sel.classed('gradient_filled', true);

test/jasmine/tests/scatter_test.js

@@ -1164,6 +1164,96 @@ describe('end-to-end scatter tests', function() {
     });
 });
 
+describe('scatter gradients', () => {
+    const colorscale = [
+        [0, 'blue'],
+        [1, 'red']
+    ];
+    let gd;
+
+    beforeEach(() => {
+        gd = createGraphDiv();
+    });
+
+    afterEach(destroyGraphDiv);
+
+    function getGradient(prefix, trace) {
+        return document.getElementById(`g${gd._fullLayout._uid}-${prefix}-${trace.uid}`);
+    }
+
+    function assertBounds(gradient, attr, ax, start, stop) {
+        expect(gradient.getAttribute('gradientUnits')).toBe('userSpaceOnUse');
+        expect(Number(gradient.getAttribute(`${attr}1`))).toBeCloseTo(ax.c2p(start), 1, `${attr}1`);
+        expect(Number(gradient.getAttribute(`${attr}2`))).toBeCloseTo(ax.c2p(stop), 1, `${attr}2`);
+    }
+
+    it('updates fillgradient bounds with the axis range', async () => {
+        await Plotly.newPlot(
+            gd,
+            [
+                {
+                    y: [1, 3, 2],
+                    fill: 'tozeroy',
+                    fillgradient: { type: 'vertical', start: 0, stop: 3, colorscale }
+                }
+            ],
+            { width: 400, height: 400, yaxis: { range: [0, 4] } }
+        );
+
+        const trace = gd._fullData[0];
+        assertBounds(getGradient('scatterfill', trace), 'y', gd._fullLayout.yaxis, 0, 3);
+
+        await Plotly.relayout(gd, 'yaxis.range', [0, 8]);
+        assertBounds(getGradient('scatterfill', trace), 'y', gd._fullLayout.yaxis, 0, 3);
+    });
+
+    it('draws fillgradient with a single bound on a secondary y axis', async () => {
+        await Plotly.newPlot(
+            gd,
+            [
+                { y: [1, 3, 2] },
+                {
+                    y: [10, 30, 20],
+                    yaxis: 'y2',
+                    fill: 'tozeroy',
+                    fillgradient: { type: 'vertical', start: 5, colorscale }
+                }
+            ],
+            { width: 400, height: 400, yaxis2: { overlaying: 'y', side: 'right' } }
+        );
+
+        assertBounds(getGradient('scatterfill', gd._fullData[1]), 'y', gd._fullLayout.yaxis2, 5, 30);
+    });
+
+    it('draws fillgradient with a single bound on a log axis', async () => {
+        await Plotly.newPlot(
+            gd,
+            [
+                {
+                    y: [1, 100, 10],
+                    fill: 'toself',
+                    fillgradient: { type: 'vertical', start: 2, colorscale }
+                }
+            ],
+            { width: 400, height: 400, yaxis: { type: 'log' } }
+        );
+
+        assertBounds(getGradient('scatterfill', gd._fullData[0]), 'y', gd._fullLayout.yaxis, 2, 100);
+    });
+
+    it('ignores fillgradient start and stop with type *radial*', async () => {
+        await Plotly.newPlot(gd, [
+            {
+                y: [1, 3, 2],
+                fill: 'tozeroy',
+                fillgradient: { type: 'radial', start: 1, stop: 2, colorscale }
+            }
+        ]);
+
+        expect(getGradient('scatterfill', gd._fullData[0]).tagName).toBe('radialGradient');
+    });
+});
+
 describe('Text templates on scatter traces:', function() {
     checkTextTemplate([{
         type: 'scatter',