From 834f80cdc7227232c8bfd8fa8dc7be447f0bf2b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 13:58:32 +0000 Subject: [PATCH 1/3] Fix fillgradient bounds on zoom, secondary axes, log axes and radial A fillgradient with start or stop uses bounds in user space. Four problems affected these bounds: - gradientWithBounds set them only when it created the gradient element, so the gradient did not follow a zoom or a pan - a missing bound read trace._extremes.x or .y, which does not exist for a trace on a secondary axis - the extremes are in linear space, but the code converted them with c2p, which is wrong on a log axis - a radial gradient with start or stop threw an error Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01M7Q9TzC2e8vaoBxrhzAjhd --- src/components/drawing/index.js | 90 +++++++++++++++++------------- test/jasmine/tests/scatter_test.js | 90 ++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 40 deletions(-) diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js index 707c7b2cfb6..fc85a134506 100644 --- a/src/components/drawing/index.js +++ b/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); diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js index 4c27d0a9bb8..ae23fe6195b 100644 --- a/test/jasmine/tests/scatter_test.js +++ b/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', From 75aee4ecd833486a59caca0b98a61b8b723eb60b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 13:59:00 +0000 Subject: [PATCH 2/3] Add draftlog for the fillgradient fixes Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01M7Q9TzC2e8vaoBxrhzAjhd --- draftlogs/3_fix.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 draftlogs/3_fix.md diff --git a/draftlogs/3_fix.md b/draftlogs/3_fix.md new file mode 100644 index 00000000000..c4b272fbaa8 --- /dev/null +++ b/draftlogs/3_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 [[#3](https://github.com/plotly/plotly.js/pull/3)] + - Fix an error for a *radial* `fillgradient` with `start` or `stop` [[#3](https://github.com/plotly/plotly.js/pull/3)] From 6d5cf80a4c7df8366b8cef9c75656440b1b1c767 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 14:24:14 +0000 Subject: [PATCH 3/3] Rename the fix draftlog to the upstream PR number 8096 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01M7Q9TzC2e8vaoBxrhzAjhd --- draftlogs/{3_fix.md => 8096_fix.md} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename draftlogs/{3_fix.md => 8096_fix.md} (60%) diff --git a/draftlogs/3_fix.md b/draftlogs/8096_fix.md similarity index 60% rename from draftlogs/3_fix.md rename to draftlogs/8096_fix.md index c4b272fbaa8..88c0ff2046e 100644 --- a/draftlogs/3_fix.md +++ b/draftlogs/8096_fix.md @@ -1,2 +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 [[#3](https://github.com/plotly/plotly.js/pull/3)] - - Fix an error for a *radial* `fillgradient` with `start` or `stop` [[#3](https://github.com/plotly/plotly.js/pull/3)] + - 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)]