From 6fc270e6cecfb40a79c10772cff10217afdf28bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 1 Oct 2019 16:53:41 -0400 Subject: [PATCH 1/3] some linting in modebar button code --- src/components/modebar/buttons.js | 21 +++++++++------------ test/jasmine/tests/modebar_test.js | 3 +++ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/components/modebar/buttons.js b/src/components/modebar/buttons.js index a496dd74f34..27d0a90d56a 100644 --- a/src/components/modebar/buttons.js +++ b/src/components/modebar/buttons.js @@ -214,8 +214,9 @@ function handleCartesian(gd, ev) { if(!ax.fixedrange) { axName = ax._name; - if(val === 'auto') aobj[axName + '.autorange'] = true; - else if(val === 'reset') { + if(val === 'auto') { + aobj[axName + '.autorange'] = true; + } else if(val === 'reset') { if(ax._rangeInitial === undefined) { aobj[axName + '.autorange'] = true; } else { @@ -581,26 +582,22 @@ modeBarButtons.toggleSpikelines = { val: 'on', click: function(gd) { var fullLayout = gd._fullLayout; + var allSpikesEnabled = fullLayout._cartesianSpikesEnabled; - fullLayout._cartesianSpikesEnabled = fullLayout._cartesianSpikesEnabled === 'on' ? 'off' : 'on'; - - var aobj = setSpikelineVisibility(gd); - - Registry.call('_guiRelayout', gd, aobj); + fullLayout._cartesianSpikesEnabled = allSpikesEnabled === 'on' ? 'off' : 'on'; + Registry.call('_guiRelayout', gd, setSpikelineVisibility(gd)); } }; function setSpikelineVisibility(gd) { var fullLayout = gd._fullLayout; + var areSpikesOn = fullLayout._cartesianSpikesEnabled === 'on'; var axList = axisIds.list(gd, null, true); var aobj = {}; - var ax, axName; - for(var i = 0; i < axList.length; i++) { - ax = axList[i]; - axName = ax._name; - aobj[axName + '.showspikes'] = fullLayout._cartesianSpikesEnabled === 'on' ? true : ax._showSpikeInitial; + var ax = axList[i]; + aobj[ax._name + '.showspikes'] = areSpikesOn ? true : ax._showSpikeInitial; } return aobj; diff --git a/test/jasmine/tests/modebar_test.js b/test/jasmine/tests/modebar_test.js index eca3ff4ef5c..3a6c3a974d7 100644 --- a/test/jasmine/tests/modebar_test.js +++ b/test/jasmine/tests/modebar_test.js @@ -1201,6 +1201,7 @@ describe('ModeBar', function() { expect(gd._fullLayout.hovermode).toBe('x'); assertActive(hovermodeButtons, buttonCompare); }); + it('should makes spikelines visible', function() { buttonToggle.click(); expect(gd._fullLayout._cartesianSpikesEnabled).toBe('on'); @@ -1208,6 +1209,7 @@ describe('ModeBar', function() { buttonToggle.click(); expect(gd._fullLayout._cartesianSpikesEnabled).toBe('off'); }); + it('should not become disabled when hovermode is switched off closest', function() { buttonToggle.click(); expect(gd._fullLayout._cartesianSpikesEnabled).toBe('on'); @@ -1215,6 +1217,7 @@ describe('ModeBar', function() { buttonCompare.click(); expect(gd._fullLayout._cartesianSpikesEnabled).toBe('on'); }); + it('should keep the state on changing the hovermode', function() { buttonToggle.click(); expect(gd._fullLayout._cartesianSpikesEnabled).toBe('on'); From f84f7c35240757bc81b7b936614c6bb620fdc870 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 1 Oct 2019 16:55:04 -0400 Subject: [PATCH 2/3] fix #4237 - track current _cartesianSpikesEnabled value ... during modebar button on-click logic for cartesian axes. Previously, we assume that _cartesianSpikesEnabled was always on when clicking on the other modebar buttons for cartesian axes. This is obviously wrong. --- src/components/modebar/buttons.js | 8 +++++--- test/jasmine/tests/modebar_test.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/components/modebar/buttons.js b/src/components/modebar/buttons.js index 27d0a90d56a..fc7c39330c6 100644 --- a/src/components/modebar/buttons.js +++ b/src/components/modebar/buttons.js @@ -199,7 +199,7 @@ function handleCartesian(gd, ev) { var fullLayout = gd._fullLayout; var aobj = {}; var axList = axisIds.list(gd, null, true); - var allSpikesEnabled = 'on'; + var allSpikesEnabled = fullLayout._cartesianSpikesEnabled; var ax, i; @@ -224,6 +224,8 @@ function handleCartesian(gd, ev) { aobj[axName + '.range[0]'] = rangeInitial[0]; aobj[axName + '.range[1]'] = rangeInitial[1]; } + + // N.B. "reset" also resets showspikes if(ax._showSpikeInitial !== undefined) { aobj[axName + '.showspikes'] = ax._showSpikeInitial; if(allSpikesEnabled === 'on' && !ax._showSpikeInitial) { @@ -246,7 +248,6 @@ function handleCartesian(gd, ev) { } } } - fullLayout._cartesianSpikesEnabled = allSpikesEnabled; } else { // if ALL traces have orientation 'h', 'hovermode': 'x' otherwise: 'y' if(astr === 'hovermode' && (val === 'x' || val === 'y')) { @@ -259,12 +260,13 @@ function handleCartesian(gd, ev) { allSpikesEnabled = 'off'; } } - fullLayout._cartesianSpikesEnabled = allSpikesEnabled; } aobj[astr] = val; } + fullLayout._cartesianSpikesEnabled = allSpikesEnabled; + Registry.call('_guiRelayout', gd, aobj); } diff --git a/test/jasmine/tests/modebar_test.js b/test/jasmine/tests/modebar_test.js index 3a6c3a974d7..de6fa2e245d 100644 --- a/test/jasmine/tests/modebar_test.js +++ b/test/jasmine/tests/modebar_test.js @@ -1231,6 +1231,36 @@ describe('ModeBar', function() { buttonClosest.click(); expect(gd._fullLayout._cartesianSpikesEnabled).toBe('off'); }); + + it('should work after clicking on "autoScale2d"', function() { + var buttonAutoScale = selectButton(modeBar, 'autoScale2d'); + expect(gd._fullLayout._cartesianSpikesEnabled).toBe('off'); + + buttonAutoScale.click(); + expect(gd._fullLayout._cartesianSpikesEnabled).toBe('off'); + + buttonToggle.click(); + expect(gd._fullLayout._cartesianSpikesEnabled).toBe('on'); + + buttonToggle.click(); + expect(gd._fullLayout._cartesianSpikesEnabled).toBe('off'); + }); + + it('should work after clicking on "resetScale2d"', function() { + var buttonResetScale = selectButton(modeBar, 'resetScale2d'); + expect(gd._fullLayout._cartesianSpikesEnabled).toBe('off'); + + buttonToggle.click(); + expect(gd._fullLayout._cartesianSpikesEnabled).toBe('on'); + + buttonResetScale.click(); + expect(gd._fullLayout._cartesianSpikesEnabled).toBe('off'); + + buttonToggle.click(); + expect(gd._fullLayout._cartesianSpikesEnabled).toBe('on'); + buttonToggle.click(); + expect(gd._fullLayout._cartesianSpikesEnabled).toBe('off'); + }); }); }); From 98f5f37c3d09ce6f45c1521c26d423ac672167af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 1 Oct 2019 16:56:57 -0400 Subject: [PATCH 3/3] :hocho: obsolete spikes <--> hovermode logic - This is an artefact from the early days of "spikes" where they only worked with hovermode:'closest' --- src/components/modebar/buttons.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/components/modebar/buttons.js b/src/components/modebar/buttons.js index fc7c39330c6..5a9fb54164c 100644 --- a/src/components/modebar/buttons.js +++ b/src/components/modebar/buttons.js @@ -253,13 +253,6 @@ function handleCartesian(gd, ev) { if(astr === 'hovermode' && (val === 'x' || val === 'y')) { val = fullLayout._isHoriz ? 'y' : 'x'; button.setAttribute('data-val', val); - } else if(astr === 'hovermode' && val === 'closest') { - for(i = 0; i < axList.length; i++) { - ax = axList[i]; - if(allSpikesEnabled === 'on' && !ax.showspikes) { - allSpikesEnabled = 'off'; - } - } } aobj[astr] = val;