-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Calculate hidden minor ticks if ticklabelindex is set. #7735
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
base: master
Are you sure you want to change the base?
Changes from 13 commits
ecd3c76
4366ed7
52a00cb
081da50
a3cc377
92c800a
e3f9006
3d9714c
e85de13
202ea7b
5861fc5
474daa5
60b5d66
f5700e9
af9a2f3
50a7135
71ba43a
5bed43e
2bde37f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Fix unexpected `ticklabelindex` behavior when minor ticks are not shown. [[#7735](https://github.com/plotly/plotly.js/pull/7735)] |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -603,7 +603,7 @@ function autoShiftMonthBins(binStart, data, dtick, dataMin, calendar) { | |||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // ensure we have minor tick0 and dtick calculated | ||||||||||||||||||||||||||||||
| axes.prepMinorTicks = function(mockAx, ax, opts) { | ||||||||||||||||||||||||||||||
| if(!ax.minor.dtick) { | ||||||||||||||||||||||||||||||
| if(!ax.minor?.dtick) { | ||||||||||||||||||||||||||||||
| delete mockAx.dtick; | ||||||||||||||||||||||||||||||
| var hasMajor = ax.dtick && isNumeric(ax._tmin); | ||||||||||||||||||||||||||||||
| var mockMinorRange; | ||||||||||||||||||||||||||||||
|
|
@@ -690,7 +690,7 @@ axes.prepMinorTicks = function(mockAx, ax, opts) { | |||||||||||||||||||||||||||||
| // put back the original range, to use to find the full set of minor ticks | ||||||||||||||||||||||||||||||
| mockAx.range = ax.range; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| if(ax.minor._tick0Init === undefined) { | ||||||||||||||||||||||||||||||
| if(ax.minor?._tick0Init === undefined) { | ||||||||||||||||||||||||||||||
| // ensure identical tick0 | ||||||||||||||||||||||||||||||
| mockAx.tick0 = ax.tick0; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
@@ -973,21 +973,23 @@ axes.calcTicks = function calcTicks(ax, opts) { | |||||||||||||||||||||||||||||
| var allTicklabelVals = []; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| var hasMinor = ax.minor && (ax.minor.ticks || ax.minor.showgrid); | ||||||||||||||||||||||||||||||
| // minor ticks should be calculated if they are visible or if ticklabelindex is set because then | ||||||||||||||||||||||||||||||
| // the labels are placed at minor ticks (even if invisible) instead of major ticks. | ||||||||||||||||||||||||||||||
| var calcMinor = hasMinor || ticklabelIndex; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // calc major first | ||||||||||||||||||||||||||||||
| for(var major = 1; major >= (hasMinor ? 0 : 1); major--) { | ||||||||||||||||||||||||||||||
| for(var major = 1; major >= (calcMinor ? 0 : 1); major--) { | ||||||||||||||||||||||||||||||
| var isMinor = !major; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if(major) { | ||||||||||||||||||||||||||||||
| ax._dtickInit = ax.dtick; | ||||||||||||||||||||||||||||||
| ax._tick0Init = ax.tick0; | ||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||
| } else if (hasMinor) { | ||||||||||||||||||||||||||||||
| ax.minor._dtickInit = ax.minor.dtick; | ||||||||||||||||||||||||||||||
| ax.minor._tick0Init = ax.minor.tick0; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| var mockAx = major ? ax : Lib.extendFlat({}, ax, ax.minor); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| var mockAx = major ? ax : Lib.extendFlat({}, ax, hasMinor ? ax.minor : {"minor": {}}); | ||||||||||||||||||||||||||||||
| if(isMinor) { | ||||||||||||||||||||||||||||||
| axes.prepMinorTicks(mockAx, ax, opts); | ||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||
|
|
@@ -1074,10 +1076,11 @@ axes.calcTicks = function calcTicks(ax, opts) { | |||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if(major && isPeriod) { | ||||||||||||||||||||||||||||||
| // add one item to label period before tick0 | ||||||||||||||||||||||||||||||
| if((major || ticklabelIndex) && isPeriod) { | ||||||||||||||||||||||||||||||
| // if major: add one item to label period before tick0 | ||||||||||||||||||||||||||||||
| // if minor: add one item for ticklabelindex positioning | ||||||||||||||||||||||||||||||
| x = axes.tickIncrement(x, dtick, !axrev, calendar); | ||||||||||||||||||||||||||||||
| majorId--; | ||||||||||||||||||||||||||||||
| if (major) majorId--; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| for(; | ||||||||||||||||||||||||||||||
|
|
@@ -1126,12 +1129,14 @@ axes.calcTicks = function calcTicks(ax, opts) { | |||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // check if ticklabelIndex makes sense, otherwise ignore it | ||||||||||||||||||||||||||||||
| if(!minorTickVals || minorTickVals.length < 2) { | ||||||||||||||||||||||||||||||
| if(!minorTickVals || minorTickVals.length < 3) { | ||||||||||||||||||||||||||||||
| ticklabelIndex = false; | ||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||
| var diff = (minorTickVals[1].value - minorTickVals[0].value) * (isReversed ? -1 : 1); | ||||||||||||||||||||||||||||||
| var diff = (minorTickVals[2].value - minorTickVals[1].value) * (isReversed ? -1 : 1); | ||||||||||||||||||||||||||||||
| if(!periodCompatibleWithTickformat(diff, ax.tickformat)) { | ||||||||||||||||||||||||||||||
| ticklabelIndex = false; | ||||||||||||||||||||||||||||||
| // remove previously added tick before tick0 for handling ticklabelindex positioning | ||||||||||||||||||||||||||||||
| minorTickVals = minorTickVals.slice(1); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| // Determine for which ticks to draw labels | ||||||||||||||||||||||||||||||
|
|
@@ -1298,14 +1303,24 @@ axes.calcTicks = function calcTicks(ax, opts) { | |||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||
| lastVisibleHead = ax._prevDateHead; | ||||||||||||||||||||||||||||||
| t = setTickLabel(ax, tickVals[i]); | ||||||||||||||||||||||||||||||
| if(tickVals[i].skipLabel || | ||||||||||||||||||||||||||||||
| ticklabelIndex && allTicklabelVals.indexOf(tickVals[i]) === -1) { | ||||||||||||||||||||||||||||||
| hideLabel(t); | ||||||||||||||||||||||||||||||
| if (ticklabelIndex) { | ||||||||||||||||||||||||||||||
| if (allTicklabelVals.indexOf(tickVals[i]) === -1) { | ||||||||||||||||||||||||||||||
| hideLabel(t); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||
| if (tickVals[i].skipLabel) { | ||||||||||||||||||||||||||||||
| hideLabel(t); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just a style change, correct? The code is logically equivalent before and after the change, right? IMO it would be clearer to leave as-is, but add extra parentheses around the i.e. Or, otherwise, maybe
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I reverted the code. No strong opinion on my end 👍
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, this does something different. Only if ticklabelindex is false should the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see... the outcome is different in the case where Previously that would trigger a call to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @emilykl, since the logic here is very subtle, I went back and made the behavior clearer. Basically, my first attempt said: if ticklabelindex is present, the skipLabel flags are not valid, instead the |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| ticksOut.push(t); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if(isPeriod && ticklabelIndex && minorTicks.length) { | ||||||||||||||||||||||||||||||
| // drop very first minor tick that we added to handle ticklabelindex | ||||||||||||||||||||||||||||||
| minorTicks[0].noTick = true; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| ticksOut = ticksOut.concat(minorTicks); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| ax._inCalcTicks = false; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| { | ||
| "data": [ | ||
| { | ||
| "showlegend": false, | ||
| "x": [ | ||
| "2023-01-01", | ||
| "2024-01-01", | ||
| "2025-01-01", | ||
| "2026-01-01", | ||
| "2027-01-01" | ||
| ], | ||
| "type": "scatter", | ||
| "xaxis": "x", | ||
| "yaxis": "y" | ||
| }, | ||
| { | ||
| "showlegend": false, | ||
| "x": [ | ||
| "2023-01-01", | ||
| "2024-01-01", | ||
| "2025-01-01", | ||
| "2026-01-01" | ||
| ], | ||
| "type": "scatter", | ||
| "xaxis": "x2", | ||
| "yaxis": "y2" | ||
| }, | ||
| { | ||
| "showlegend": false, | ||
| "x": [ | ||
| "2023-01-01", | ||
| "2024-01-01", | ||
| "2025-01-01", | ||
| "2026-01-01", | ||
| "2027-01-01" | ||
| ], | ||
| "type": "scatter", | ||
| "xaxis": "x3", | ||
| "yaxis": "y3" | ||
| } | ||
| ], | ||
| "layout": { | ||
| "width": 700, | ||
| "height": 700, | ||
| "grid": { | ||
| "rows": 3, | ||
| "columns": 1, | ||
| "pattern": "independent" | ||
| }, | ||
| "xaxis": { | ||
| "ticklen": 20, | ||
| "ticklabelindex": -1, | ||
| "tickformat": "%Y", | ||
| "ticklabelmode": "period", | ||
| "dtick": "M24", | ||
| "minor": { | ||
| "dtick": "M12", | ||
| "tick0": "2023-01-01" | ||
| }, | ||
| "title": { | ||
| "text": "Should display 2 ticks and labels 2023 and 2025 to the left of them." | ||
| } | ||
| }, | ||
| "xaxis2": { | ||
| "ticklen": 20, | ||
| "ticklabelindex": -2, | ||
| "tickformat": "%Y", | ||
| "ticklabelmode": "period", | ||
| "dtick": "M24", | ||
| "minor": { | ||
| "dtick": "M12", | ||
| "tick0": "2023-01-01" | ||
| }, | ||
| "title": { | ||
| "text": "Should display a label at 2024" | ||
| } | ||
| }, | ||
| "xaxis3": { | ||
| "ticklen": 20, | ||
| "ticklabelindex": -1, | ||
| "tickformat": "%Y", | ||
| "ticklabelmode": "period", | ||
| "ticklabelstep": 2, | ||
| "dtick": "M12", | ||
| "minor": { | ||
| "dtick": "M12", | ||
| "tick0": "2023-01-01" | ||
| }, | ||
| "title": { | ||
| "text": "Should display yearly ticks with labels at 2023 and 2025" | ||
| } | ||
| } | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.