Skip to content
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

zeroline is not visible when axis has no gridlines #4027

Closed
DaanyWaay opened this issue Jul 8, 2019 · 4 comments · Fixed by #4189
Closed

zeroline is not visible when axis has no gridlines #4027

DaanyWaay opened this issue Jul 8, 2019 · 4 comments · Fixed by #4189
Assignees
Labels
bug something broken
Milestone

Comments

@DaanyWaay
Copy link

DaanyWaay commented Jul 8, 2019

https://codepen.io/anon/pen/pXxqeW
zeroline is not visible if these conditions are met:

rangemode: 'tozero',
nticks: 1 (or 2)

If you change nticks to: 0 | 3 | 3+ it is shown again.
Unfortunately I need to hide all the ticks except tick0 as this configuration will be used for subplots.

The solution with mirror axis works better, but:

@etpinard
Copy link
Contributor

Thanks for writing in!

At present, we don't draw the ax.zeroline line on axes that don't have any visible grid lines associated with them e.g. as in the nticks: 1 or nticks: 2 cases you pointed out (note also that 0 isn't a valid nticks values, so you get the default ticks in that case).

The logic is here:

axes.shouldShowZeroLine = function(gd, ax, counterAxis) {
var rng = Lib.simpleMap(ax.range, ax.r2l);
return (
(rng[0] * rng[1] <= 0) &&
ax.zeroline &&
(ax.type === 'linear' || ax.type === '-') &&
ax._gridVals.length &&
(
clipEnds(ax, 0) ||
!anyCounterAxLineAtZero(gd, ax, counterAxis, rng) ||
hasBarsOrFill(gd, ax)
)
);
};


I can't remember exactly why the ax._gridVals.length condition exists. We may consider changing it and honouring ax.zeroline: true no matter what, but that would have to be part of a major version update (e.g. in our v2 release).

@etpinard etpinard changed the title zeroline for X axis is not visible under some conditions zeroline is not visible when axis has no gridlines Jul 11, 2019
@etpinard
Copy link
Contributor

On second thoughts, I'm starting to think this report should be considered a bug (thanks @alexcjohnson )

Removing

ax._gridVals.length &&

does not make any of our tests fail.

The only problem I see is: as some axes coerce('zeroline', true), according to the logic below

var showZeroLine = coerce('zeroline', opts.showGrid || !!zeroLineColor || !!zeroLineWidth);

a "new" zeroline will appear on some graphs. Now I agree that:

xaxis: {
  zeroline: true,
  nticks: 1
}

should show the zeroline, even when it's on one of the plot area edges.

Would anyone be opposed to adding a ax._input.zeroline condition in here:

 axes.shouldShowZeroLine = function(gd, ax, counterAxis) { 
     var rng = Lib.simpleMap(ax.range, ax.r2l); 
     return ( 
         (rng[0] * rng[1] <= 0) && 
         ax.zeroline && 
         (ax.type === 'linear' || ax.type === '-') && 
         (ax._gridVals.length || ax._input.zeroline) &&
         ( 
             clipEnds(ax, 0) || 
             !anyCounterAxLineAtZero(gd, ax, counterAxis, rng) || 
             hasBarsOrFill(gd, ax) 
         ) 
     ); 
 }; 

@etpinard etpinard added the bug something broken label Jul 12, 2019
@alexcjohnson
Copy link
Collaborator

My gut reaction is if zeroline gets coerced true, we should show it in these edge cases too, not just when it’s explicitly set true. Is there a case where that would look wrong?

@etpinard
Copy link
Contributor

etpinard commented Sep 6, 2019

I'm going to slip in a fix for this bug i.e. by simply removing

ax._gridVals.length &&

in 1.50.0.

@etpinard etpinard added this to the v1.50.0 milestone Sep 6, 2019
@etpinard etpinard self-assigned this Sep 6, 2019
etpinard added a commit that referenced this issue Sep 9, 2019
etpinard added a commit that referenced this issue Sep 13, 2019
- see #4027
  for complete rationale
- add assertZeroLines assertion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants