-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Modebar toggle spikes fixup #4241
Conversation
... 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.
- This is an artefact from the early days of "spikes" where they only worked with hovermode:'closest'
} else if(astr === 'hovermode' && val === 'closest') { | ||
for(i = 0; i < axList.length; i++) { | ||
ax = axList[i]; | ||
if(allSpikesEnabled === 'on' && !ax.showspikes) { | ||
allSpikesEnabled = 'off'; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some history here:
- Cartesian spikes were first introduced in Cartesian dropline support #1461
- back then, they only worked with
hovermode: 'closest'
- this particular block got added in rpaskowitz@762b54a w/o tests
- Add spikelines #2247 made spikes work regardless of
hovermode
, but didn't 🔪 this block
|
||
it('should work after clicking on "autoScale2d"', function() { | ||
var buttonAutoScale = selectButton(modeBar, 'autoScale2d'); | ||
expect(gd._fullLayout._cartesianSpikesEnabled).toBe('off'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, clicking on the "autoScale2d" modebar button on a graph that has _cartesianSpikesEnabled: 'off'
on first draw, results in
gd._fullLayout._cartesianSpikesEnabled // => 'on'
which is obviously wrong.
Brilliant fix. |
fixes #4237 - which I think dates back to #2247 released in
1.33.0
😑cc @archmoj it would be nice to squeeze this one in 1.50.0. Thanks!