From 150ccd351e7636360b0539b49b06002ca54281d2 Mon Sep 17 00:00:00 2001 From: My-Tien Nguyen Date: Tue, 6 May 2025 11:19:54 +0200 Subject: [PATCH 1/7] Fixes #7416 Hidden ticklabels don't take up space anymore. --- src/plots/cartesian/axes.js | 15 ++++++++- test/image/mocks/zz-label-spacing.json | 44 ++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 test/image/mocks/zz-label-spacing.json diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 84ebe5b3bb1..dad5a4bbb36 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -3915,11 +3915,24 @@ axes.drawLabels = function(gd, ax, opts) { } } + function removeHiddenLabels() { + // Remove all hidden labels to prevent them from affecting the layout. + tickLabels.each(function(d) { + var thisLabel = d3.select(this); + var textNode = thisLabel.select('text').node(); + var opacity = window.getComputedStyle(textNode).opacity; + if (opacity == '0') { + thisLabel.select('text').text(''); + d3.select(textNode).text(''); + } + }); + } + if(ax._selections) { ax._selections[cls] = tickLabels; } - var seq = [allLabelsReady]; + var seq = [allLabelsReady, removeHiddenLabels]; // N.B. during auto-margin redraws, if the axis fixed its label overlaps // by rotating 90 degrees, do not attempt to re-fix its label overlaps diff --git a/test/image/mocks/zz-label-spacing.json b/test/image/mocks/zz-label-spacing.json new file mode 100644 index 00000000000..310dea817b9 --- /dev/null +++ b/test/image/mocks/zz-label-spacing.json @@ -0,0 +1,44 @@ +{ + "data": [ + { + "x": ["A", "B", "C"], + "y": [0, 40, 80], + "type": "scatter" + }, + { + "x": ["A", "B", "C"], + "y": [0, 40, 80], + "type": "scatter", + "xaxis": "x2", + "yaxis": "y2" + } + ], + "layout": { + "title": { + "text": "The vertical grid lines in the subplots should be aligned." + }, + "width": 600, + "xaxis": { + "anchor": "y" + }, + "xaxis2": { + "anchor": "y2" + }, + "yaxis": { + "range": [0, 80], + "dtick": 20, + "side": "right", + "ticklabelposition": "inside", + "anchor": "x", + "domain": [0, 0.45] + }, + "yaxis2": { + "range": [0, 100], + "dtick": 20, + "side": "right", + "ticklabelposition": "inside", + "anchor": "x2", + "domain": [0.55, 1] + } + } +} From 684d05c0a74d6596c6b5c15bd1cd1b2ccd36cc31 Mon Sep 17 00:00:00 2001 From: My-Tien Nguyen Date: Tue, 6 May 2025 12:31:09 +0200 Subject: [PATCH 2/7] Update some tests in axes_test.js, because hidden labels no longer take up space. Also use floating point comparison in "Test axes minor ticks" --- test/jasmine/tests/axes_test.js | 47 +++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/test/jasmine/tests/axes_test.js b/test/jasmine/tests/axes_test.js index e42cf4a60df..4283bd8ac57 100644 --- a/test/jasmine/tests/axes_test.js +++ b/test/jasmine/tests/axes_test.js @@ -1696,7 +1696,8 @@ describe('Test axes', function() { return Plotly.relayout(gd, 'xaxis.autorange', true); }).then(function() { - expect(gd._fullLayout.xaxis.range).toBeCloseToArray([0.37, 3.22], 1); + + expect(gd._fullLayout.xaxis.range).toBeCloseToArray([0.27, 3.23], 1); }) .then(done, done.fail); }); @@ -1721,13 +1722,13 @@ describe('Test axes', function() { width: 600, height: 600 }).then(function() { - expect(gd._fullLayout.xaxis.range).toBeCloseToArray([-0.110, 2]); + expect(gd._fullLayout.xaxis.range).toBeCloseToArray([-0.13, 2]); return Plotly.relayout(gd, { 'xaxis.insiderange': [1, 3] }); }).then(function() { - expect(gd._fullLayout.xaxis.range).toBeCloseToArray([0.889, 3]); + expect(gd._fullLayout.xaxis.range).toBeCloseToArray([0.866, 3]); }).then(done, done.fail); }); }); @@ -4907,32 +4908,32 @@ describe('Test axes', function() { }) .then(function() { _assert('on labels (defaults)', { - ticks: [110.75, 350, 589.25], - gridLines: [110.75, 350, 589.25], + ticks: [108.75, 348, 587.25], + gridLines: [108.75, 348, 587.25], tickLabels: [106.421, 345.671, 585.25] }); return Plotly.relayout(gd, 'xaxis.tickson', 'boundaries'); }) .then(function() { _assert('inside on boundaries', { - ticks: [230.369, 469.619], // N.B. first and last tick are clipped - gridLines: [230.369, 469.619], + ticks: [228.367, 467.617], // N.B. first and last tick are clipped + gridLines: [228.367, 467.617], tickLabels: [106.421875, 345.671875, 585.25] }); return Plotly.relayout(gd, 'xaxis.ticks', 'outside'); }) .then(function() { _assert('outside on boundaries', { - ticks: [230.369, 469.619], - gridLines: [230.369, 469.619], + ticks: [228.367, 467.617], + gridLines: [228.367, 467.617], tickLabels: [106.421875, 345.671875, 585.25] }); return Plotly.restyle(gd, 'x', [[1, 2, 1]]); }) .then(function() { _assert('fallback to *labels* on non-category axes', { - ticks: [110.75, 206.449, 302.149, 397.85, 493.549, 589.25], - gridLines: [110.75, 206.449, 302.149, 397.85, 493.549, 589.25], + ticks: [108.75, 204.433, 300.133, 395.85, 491.533, 587.25], + gridLines: [108.75, 204.433, 300.133, 395.85, 491.533, 587.25], tickLabels: [106.421, 197.121, 292.821, 388.521, 484.221, 584.921] }); }) @@ -6901,6 +6902,18 @@ describe('Test axes', function() { expect(positions).toEqual(expPositions); } + function _assertClose(expPositions) { + var ax = gd._fullLayout.xaxis; + + // minor positions + var positions = + ax._vals + .filter(function(d) { return d.minor; }) + .map(function(d) { return d.x; }); + + expect(positions).toBeCloseToArray(expPositions); + } + it('minor tickvals', function(done) { Plotly.newPlot(gd, { data: [{ @@ -7127,7 +7140,7 @@ describe('Test axes', function() { } }) .then(function() { - _assert([ -0.22184874961635648, -0.1549019599857433, -0.09691001300805657, -0.04575749056067513, 0.4771212547196623, 0.6020599913279623, 0.7781512503836435, 0.8450980400142567, 0.9030899869919434, 0.9542425094393249, 1.4771212547196624, 1.6020599913279623, 1.7781512503836434, 1.8450980400142567, 1.9030899869919433, 1.9542425094393248, 2.477121254719662, 2.6020599913279625, 2.7781512503836434, 2.845098040014257, 2.9030899869919433, 2.9542425094393248, 3.477121254719662, 3.6020599913279625, 3.7781512503836434, 3.845098040014257, 3.9030899869919433, 3.9542425094393248 ]); + _assertClose([ -0.22184874961635648, -0.1549019599857433, -0.09691001300805657, -0.04575749056067513, 0.4771212547196623, 0.6020599913279623, 0.7781512503836435, 0.8450980400142567, 0.9030899869919434, 0.9542425094393249, 1.4771212547196624, 1.6020599913279623, 1.7781512503836434, 1.8450980400142567, 1.9030899869919433, 1.9542425094393248, 2.477121254719662, 2.6020599913279625, 2.7781512503836434, 2.845098040014257, 2.9030899869919433, 2.9542425094393248, 3.477121254719662, 3.6020599913279625, 3.7781512503836434, 3.845098040014257, 3.9030899869919433, 3.9542425094393248 ]); }) .then(done, done.fail); }); @@ -7169,7 +7182,7 @@ describe('Test axes', function() { } }) .then(function() { - _assert([ -0.017728766960431602, -0.00877392430750515, 0.008600171761917567, 0.017033339298780367, 0.025305865264770258, 0.03342375548694973, 0.049218022670181646, 0.056904851336472634, 0.06445798922691853, 0.07188200730612543, 0.08635983067474828, 0.09342168516223513, 0.10037054511756296, 0.10720996964786844, 0.12057393120584996, 0.1271047983648077, 0.13353890837021762, 0.13987908640123659, 0.15228834438305658, 0.15836249209524975, 0.1643528557844372, 0.17026171539495752, 0.18184358794477265, 0.18752072083646318, 0.1931245983544617, 0.1986570869544227, 0.20951501454263102, 0.21484384804769796, 0.22010808804005513, 0.22530928172586287, 0.23552844690754896, 0.24054924828259974, 0.24551266781414988, 0.250420002308894, 0.2600713879850748, 0.2648178230095364, 0.26951294421791616, 0.2741578492636797, 0.28330122870354935, 0.2878017299302258, 0.29225607135647574, 0.29666519026153076, 0.30535136944662333, 0.3096301674258983, 0.31386722036915293, 0.31806333496276107 ]); + _assertClose([ -0.017728766960431602, -0.00877392430750515, 0.008600171761917567, 0.017033339298780367, 0.025305865264770258, 0.03342375548694973, 0.049218022670181646, 0.056904851336472634, 0.06445798922691853, 0.07188200730612543, 0.08635983067474828, 0.09342168516223513, 0.10037054511756296, 0.10720996964786844, 0.12057393120584996, 0.1271047983648077, 0.13353890837021762, 0.13987908640123659, 0.15228834438305658, 0.15836249209524975, 0.1643528557844372, 0.17026171539495752, 0.18184358794477265, 0.18752072083646318, 0.1931245983544617, 0.1986570869544227, 0.20951501454263102, 0.21484384804769796, 0.22010808804005513, 0.22530928172586287, 0.23552844690754896, 0.24054924828259974, 0.24551266781414988, 0.250420002308894, 0.2600713879850748, 0.2648178230095364, 0.26951294421791616, 0.2741578492636797, 0.28330122870354935, 0.2878017299302258, 0.29225607135647574, 0.29666519026153076, 0.30535136944662333, 0.3096301674258983, 0.31386722036915293, 0.31806333496276107 ]); }) .then(done, done.fail); }); @@ -7191,7 +7204,7 @@ describe('Test axes', function() { } }) .then(function() { - _assert([ -0.30102999566398125, 0.30102999566398114, 0.6989700043360187, 1.3010299956639813, 1.6989700043360187, 2.3010299956639813, 2.6989700043360187, 3.3010299956639813, 3.6989700043360187, 4.301029995663981, 4.698970004336019, 5.301029995663981, 5.698970004336019, 6.301029995663981, 6.698970004336019, 7.301029995663981 ]); + _assertClose([ -0.30102999566398125, 0.30102999566398114, 0.6989700043360187, 1.3010299956639813, 1.6989700043360187, 2.3010299956639813, 2.6989700043360187, 3.3010299956639813, 3.6989700043360187, 4.301029995663981, 4.698970004336019, 5.301029995663981, 5.698970004336019, 6.301029995663981, 6.698970004336019, 7.301029995663981 ]); }) .then(done, done.fail); }); @@ -7213,7 +7226,7 @@ describe('Test axes', function() { } }) .then(function() { - _assert([ 0.17609125905568124, 0.3979400086720376, 0.5440680443502756, 0.6532125137753436, 0.7403626894942437, 0.8129133566428552, 0.8750612633916998, 0.9294189257142923, 0.9777236052888472, 1.0211892990699374, 1.0413926851582243, 1.0606978403536107 ]); + _assertClose([ 0.17609125905568124, 0.3979400086720376, 0.5440680443502756, 0.6532125137753436, 0.7403626894942437, 0.8129133566428552, 0.8750612633916998, 0.9294189257142923, 0.9777236052888472, 1.0211892990699374, 1.0413926851582243, 1.0606978403536107 ]); }) .then(done, done.fail); }); @@ -8139,11 +8152,11 @@ describe('more react tests', function() { Plotly.newPlot(gd, fig1) .then(function() { - expect(gd._fullLayout.xaxis.range).toBeCloseToArray([-0.110, 2]); + expect(gd._fullLayout.xaxis.range).toBeCloseToArray([-0.134, 2]); return Plotly.react(gd, fig2); }).then(function() { - expect(gd._fullLayout.xaxis.range).toBeCloseToArray([-0.164, 2]); + expect(gd._fullLayout.xaxis.range).toBeCloseToArray([-0.192, 2]); }).then(done, done.fail); }); }); From a287e906f0312f930ca3389429215edb5f6780ec Mon Sep 17 00:00:00 2001 From: My-Tien Nguyen Date: Tue, 6 May 2025 13:29:41 +0200 Subject: [PATCH 3/7] Add draftlog for 7417 --- draftlogs/7417_fix.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 draftlogs/7417_fix.md diff --git a/draftlogs/7417_fix.md b/draftlogs/7417_fix.md new file mode 100644 index 00000000000..62aef9598f7 --- /dev/null +++ b/draftlogs/7417_fix.md @@ -0,0 +1 @@ +- Fix hidden ticklabels taking up plot space [[#7417](https://github.com/plotly/plotly.js/pull/7417)] From e157374597f1a377462b11a9257716ec03fac304 Mon Sep 17 00:00:00 2001 From: My-Tien Nguyen Date: Fri, 9 May 2025 11:10:21 +0200 Subject: [PATCH 4/7] Instead of removing text of hidden ticklabels, just set their display mode to none. This is actually what we want: the label should not be visible and not take up space. --- src/plots/cartesian/axes.js | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index dad5a4bbb36..9d55139e45f 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -3648,7 +3648,7 @@ axes.drawLabels = function(gd, ax, opts) { 'text-anchor': anchor }); - thisText.style('opacity', 1); // visible + thisText.style('display', null); // visible if(ax._adjustTickLabelsOverflow) { ax._adjustTickLabelsOverflow(); @@ -3706,9 +3706,9 @@ axes.drawLabels = function(gd, ax, opts) { var t = thisLabel.select('text'); if(adjust) { - if(hideOverflow) t.style('opacity', 0); // hidden + if(hideOverflow) t.style('display', 'none'); // hidden } else { - t.style('opacity', 1); // visible + t.style('display', null); // visible if(side === 'bottom' || side === 'right') { visibleLabelMin = Math.min(visibleLabelMin, isX ? bb.top : bb.left); @@ -3783,7 +3783,7 @@ axes.drawLabels = function(gd, ax, opts) { q > ax['_visibleLabelMin_' + anchorAx._id] ) { t.style('display', 'none'); // hidden - } else if(e.K === 'tick' && !idx) { + } else if(e.K === 'tick' && !idx && t.style('display') != 'none') { t.style('display', null); // visible } }); @@ -3807,6 +3807,7 @@ axes.drawLabels = function(gd, ax, opts) { var autoangle = null; function fixLabelOverlaps() { + console.log("fix label overlaps!"); positionLabels(tickLabels, tickAngle); // check for auto-angling if x labels overlap @@ -3915,24 +3916,11 @@ axes.drawLabels = function(gd, ax, opts) { } } - function removeHiddenLabels() { - // Remove all hidden labels to prevent them from affecting the layout. - tickLabels.each(function(d) { - var thisLabel = d3.select(this); - var textNode = thisLabel.select('text').node(); - var opacity = window.getComputedStyle(textNode).opacity; - if (opacity == '0') { - thisLabel.select('text').text(''); - d3.select(textNode).text(''); - } - }); - } - if(ax._selections) { ax._selections[cls] = tickLabels; } - var seq = [allLabelsReady, removeHiddenLabels]; + var seq = [allLabelsReady]; // N.B. during auto-margin redraws, if the axis fixed its label overlaps // by rotating 90 degrees, do not attempt to re-fix its label overlaps From 049b7febff8639afc55c4f3da1b3cd7184f6a22a Mon Sep 17 00:00:00 2001 From: My-Tien Nguyen Date: Tue, 13 May 2025 14:04:19 +0200 Subject: [PATCH 5/7] Fix error: d3 selection.style called as getter Also remove left-over console debug print --- src/plots/cartesian/axes.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 9d55139e45f..f879bd91851 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -3783,8 +3783,11 @@ axes.drawLabels = function(gd, ax, opts) { q > ax['_visibleLabelMin_' + anchorAx._id] ) { t.style('display', 'none'); // hidden - } else if(e.K === 'tick' && !idx && t.style('display') != 'none') { - t.style('display', null); // visible + } else if(e.K === 'tick' && !idx) { + var display = window.getComputedStyle(t.node()).display; + if (display != 'none') { + t.style('display', null); // visible + } } }); }); @@ -3807,7 +3810,6 @@ axes.drawLabels = function(gd, ax, opts) { var autoangle = null; function fixLabelOverlaps() { - console.log("fix label overlaps!"); positionLabels(tickLabels, tickAngle); // check for auto-angling if x labels overlap From 23a29f21c2a2bc0f2a0f6b3ae8d87cf5b306cfdd Mon Sep 17 00:00:00 2001 From: My-Tien Nguyen Date: Tue, 13 May 2025 14:10:26 +0200 Subject: [PATCH 6/7] axes_test: Add precision param to new toBeCloseToArray call --- test/jasmine/tests/axes_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jasmine/tests/axes_test.js b/test/jasmine/tests/axes_test.js index 4283bd8ac57..34209ff574a 100644 --- a/test/jasmine/tests/axes_test.js +++ b/test/jasmine/tests/axes_test.js @@ -6911,7 +6911,7 @@ describe('Test axes', function() { .filter(function(d) { return d.minor; }) .map(function(d) { return d.x; }); - expect(positions).toBeCloseToArray(expPositions); + expect(positions).toBeCloseToArray(expPositions, 3); } it('minor tickvals', function(done) { From 89677323d67f8c1635152024295382c98deb6c77 Mon Sep 17 00:00:00 2001 From: My-Tien Nguyen Date: Tue, 13 May 2025 14:24:30 +0200 Subject: [PATCH 7/7] Correct fix for "d3 selection.style called as getter" --- src/plots/cartesian/axes.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index f879bd91851..14963988be1 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -3783,11 +3783,8 @@ axes.drawLabels = function(gd, ax, opts) { q > ax['_visibleLabelMin_' + anchorAx._id] ) { t.style('display', 'none'); // hidden - } else if(e.K === 'tick' && !idx) { - var display = window.getComputedStyle(t.node()).display; - if (display != 'none') { - t.style('display', null); // visible - } + } else if(e.K === 'tick' && !idx && t.node().style.display !== 'none') { + t.style('display', null); // visible } }); });