diff --git a/draftlogs/7877_fix.md b/draftlogs/7877_fix.md new file mode 100644 index 00000000000..95dc5f5afa8 --- /dev/null +++ b/draftlogs/7877_fix.md @@ -0,0 +1,2 @@ +- Fix missing last period label when using a negative `ticklabelindex` in a chart where the last major tick is not visible [[#7877](https://github.com/plotly/plotly.js/pull/7877)] +- Fix incorrect period label positioning when using `ticklabelindex` in a chart where not enough minor ticks are visible [[#7877](https://github.com/plotly/plotly.js/pull/7877)] \ No newline at end of file diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 08bfda6f367..d4011db4494 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -860,25 +860,40 @@ function adjustPeriodDelta(ax) { // adjusts ax.dtick and sets ax._definedDelta ax._definedDelta = definedDelta; } -function positionPeriodTicks(tickVals, ax, definedDelta) { +/** + * Calculates the period label position for each tick in tickVals. + * @param {array} tickVals: the list of ticks for which to position the period labels + * @param {object} ax: the axis of the ticks + * @param {number} definedDelta: the defined distance between two ticks + * @param {array (optional)} periodEndTicks: the optional list of neighboring ticks + * for each tick in tickVals. If not provided, the function will use the next + * tick in tickVals as the neighbor. Useful if tickVals is not evenly spaced. + */ +function positionPeriodTicks(tickVals, ax, definedDelta, periodEndTicks) { for(var i = 0; i < tickVals.length; i++) { var v = tickVals[i].value; - - var a = i; - var b = i + 1; - if(i < tickVals.length - 1) { - a = i; - b = i + 1; - } else if(i > 0) { - a = i - 1; - b = i; + var A, B; + if (periodEndTicks != null) { + A = tickVals[i].value; + B = periodEndTicks[i].value; } else { - a = i; - b = i; + var a = i; + var b = i + 1; + if(i < tickVals.length - 1) { + a = i; + b = i + 1; + } else if(i > 0) { + a = i - 1; + b = i; + } else { + a = i; + b = i; + } + + A = tickVals[a].value; + B = tickVals[b].value; } - var A = tickVals[a].value; - var B = tickVals[b].value; var actualDelta = Math.abs(B - A); var delta = definedDelta || actualDelta; var periodLength = 0; @@ -971,12 +986,16 @@ axes.calcTicks = function calcTicks(ax, opts) { // all ticks for which labels are drawn which is not necessarily the major ticks when // `ticklabelindex` is set. var allTicklabelVals = []; + // for period label positioning when using `ticklabelindex`: + // for each tick in `allTicklabelVals` holds the neighboring period end tick + var periodEndTicks; 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; - + var calcMinor = hasMinor || (ticklabelIndex != null && ticklabelIndex !== 0); + + var majorDtick = ax.dtick; // calc major first for(var major = 1; major >= (calcMinor ? 0 : 1); major--) { var isMinor = !major; @@ -994,6 +1013,7 @@ axes.calcTicks = function calcTicks(ax, opts) { axes.prepMinorTicks(mockAx, ax, opts); } else { axes.prepTicks(mockAx, opts); + majorDtick = mockAx.dtick; } // now that we've figured out the auto values for formatting @@ -1020,9 +1040,11 @@ axes.calcTicks = function calcTicks(ax, opts) { var exRng = expandRange(rng); var startTick = exRng[0]; var endTick = exRng[1]; + var visibleEndTick = endTick; // for period axis, there will be an additional major tick after that + var dtick = mockAx.dtick; - var numDtick = isNumeric(mockAx.dtick); - var isDLog = (type === 'log') && !(numDtick || mockAx.dtick.charAt(0) === 'L'); + var numDtick = isNumeric(dtick); + var isDLog = (type === 'log') && !(numDtick || dtick.charAt(0) === 'L'); // find the first tick var x0 = axes.tickFirst(mockAx, opts); @@ -1066,8 +1088,6 @@ axes.calcTicks = function calcTicks(ax, opts) { ) / _dTick) - 1; } - var dtick = mockAx.dtick; - if(mockAx.rangebreaks && mockAx._tick0Init !== mockAx.tick0) { // adjust tick0 x = moveOutsideBreak(x, ax); @@ -1076,12 +1096,12 @@ axes.calcTicks = function calcTicks(ax, opts) { } } - if((major || ticklabelIndex) && isPeriod) { - // if major: add one item to label period before tick0 - // if minor: add one item for ticklabelindex positioning. positionPeriodTicks requires - // at least 2 ticks to calculate the period length, so we add a dummy tick, ensuring - // that if a tick is labeled, there are always at least 2 ticks. - x = axes.tickIncrement(x, dtick, !axrev, calendar); + if (isPeriod) { + // add an additional major tick and correspondingly many minor ticks + // before the first and after the last major tick + // to be able to label a period before the first and last visible ticks. + x = axes.tickIncrement(x, majorDtick, !axrev, calendar); + endTick = axes.tickIncrement(endTick, majorDtick, axrev, calendar); if (major) majorId--; } @@ -1112,6 +1132,11 @@ axes.calcTicks = function calcTicks(ax, opts) { var obj = { value: x }; + // mark ticks that were only added for period label positioning as "noTick" so they aren't drawn. + if (axrev ? (x > startTick || x < visibleEndTick) : (x < startTick || x > visibleEndTick)) { + obj.noTick = true; + } + if(major) { if(isDLog && (x !== (x | 0))) { obj.simpleLabel = true; @@ -1133,53 +1158,45 @@ axes.calcTicks = function calcTicks(ax, opts) { // check if ticklabelIndex makes sense, otherwise ignore it. // It makes sense if in addition to the always present dummy, there are at least 2 minor ticks // with the required distance to each other. - if(!minorTickVals || minorTickVals.length < 3) { + const visibleMinorTicks = minorTickVals.filter((minorTick) => minorTick.noTick !== true); + if(!visibleMinorTicks || visibleMinorTicks.length < 2) { ticklabelIndex = false; } else { - var diff = (minorTickVals[2].value - minorTickVals[1].value) * (isReversed ? -1 : 1); + var diff = (visibleMinorTicks[1].value - visibleMinorTicks[0].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 if(!ticklabelIndex) { allTicklabelVals = tickVals; } else { - // Collect and sort all major and minor ticks, to find the minor ticks `ticklabelIndex` - // steps away from each major tick. For those minor ticks we want to draw the label. - - var allTickVals = tickVals.concat(minorTickVals); - if(isPeriod && tickVals.length) { - // first major tick was just added for period handling - allTickVals = allTickVals.slice(1); - } - - allTickVals = - allTickVals - .sort(function(a, b) { return a.value - b.value; }) - .filter(function(tick, index, self) { - return index === 0 || tick.value !== self[index - 1].value; - }); - - var majorTickIndices = - allTickVals - .map(function(item, index) { - return item.minor === undefined && !item.skipLabel ? index : null; - }) - .filter(function(index) { return index !== null; }); - - majorTickIndices.forEach(function(majorIdx) { - ticklabelIndex.map(function(nextLabelIdx) { - var minorIdx = majorIdx + nextLabelIdx; - if(minorIdx >= 0 && minorIdx < allTickVals.length) { - Lib.pushUnique(allTicklabelVals, allTickVals[minorIdx]); - } - }); - }); - tickVals.forEach(function(tick) { - tick.skipLabel = allTicklabelVals.indexOf(tick) === -1; + // For each major tick, find the minor tick `ticklabelIndex` steps away. + // This minor tick will be labeled instead of the major tick. + if (isPeriod) periodEndTicks = []; // for each minor tick at the start of a labeled period this will hold the neighboring period end tick. + const minorTickValsAscending = minorTickVals.toSorted((a, b) => a.value - b.value); + tickVals.forEach(function(majorTick) { + if (!majorTick.skipLabel) { + ticklabelIndex.forEach((labelIndex) => { + if (labelIndex < 0) { + const smallerMinorTicks = minorTickValsAscending.filter((minorTick) => minorTick.value <= majorTick.value); + const absLabelIndex = Math.abs(labelIndex); + const minorTickIndex = smallerMinorTicks.length - absLabelIndex - 1; + if (absLabelIndex <= smallerMinorTicks.length - 1 && !smallerMinorTicks[minorTickIndex].noTick) { + allTicklabelVals.push(smallerMinorTicks[minorTickIndex]); + if (isPeriod) periodEndTicks.push(smallerMinorTicks[minorTickIndex + 1]); + } + } else { // labelIndex >= 0 + const largerMinorTicks = minorTickValsAscending.filter((minorTick) => minorTick.value >= majorTick.value); + if (labelIndex < largerMinorTicks.length - 1 && !largerMinorTicks[labelIndex].noTick) { + allTicklabelVals.push(largerMinorTicks[labelIndex]); + if (isPeriod) periodEndTicks.push(largerMinorTicks[labelIndex + 1]); + } + } + majorTick.skipLabel = true; + }); + } }); } @@ -1194,8 +1211,8 @@ axes.calcTicks = function calcTicks(ax, opts) { var majorValues = tickVals.map(function(d) { return d.value; }); var list = []; - for(var k = 0; k < minorTickVals.length; k++) { - var T = minorTickVals[k]; + for(var k = 0; k < visibleMinorTicks.length; k++) { + var T = visibleMinorTicks[k]; var v = T.value; if(majorValues.indexOf(v) !== -1) { continue; @@ -1215,8 +1232,7 @@ axes.calcTicks = function calcTicks(ax, opts) { minorTickVals = list; } } - - if(isPeriod) positionPeriodTicks(allTicklabelVals, ax, ax._definedDelta); + if(isPeriod) positionPeriodTicks(allTicklabelVals, ax, ax._definedDelta, periodEndTicks); var i; if(ax.rangebreaks) { @@ -1275,7 +1291,7 @@ axes.calcTicks = function calcTicks(ax, opts) { tickVals = tickVals.concat(minorTickVals); function setTickLabel(ax, tickVal) { - var text = axes.tickText( + var labeledTick = axes.tickText( ax, tickVal.value, false, // hover @@ -1283,15 +1299,15 @@ axes.calcTicks = function calcTicks(ax, opts) { ); var p = tickVal.periodX; if(p !== undefined) { - text.periodX = p; + labeledTick.periodX = p; if(p > maxRange || p < minRange) { // hide label if outside the range - if(p > maxRange) text.periodX = maxRange; - if(p < minRange) text.periodX = minRange; + if(p > maxRange) labeledTick.periodX = maxRange; + if(p < minRange) labeledTick.periodX = minRange; - hideLabel(text); + hideLabel(labeledTick); } } - return text; + return labeledTick; } var t; @@ -1305,6 +1321,7 @@ axes.calcTicks = function calcTicks(ax, opts) { } else { t = { x: _value }; } + t.noTick = tickVals[i].noTick; t.minor = true; minorTicks.push(t); } else { @@ -1313,24 +1330,14 @@ axes.calcTicks = function calcTicks(ax, opts) { if (tickVals[i].skipLabel) { hideLabel(t); } - + t.noTick = tickVals[i].noTick; 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; - if(isPeriod && ticksOut.length) { - // drop very first tick that we added to handle period - ticksOut[0].noTick = true; - } - + ticksOut = ticksOut.concat(minorTicks); return ticksOut; }; diff --git a/test/image/baselines/date_axes_period2_ticklabelindex.png b/test/image/baselines/date_axes_period2_ticklabelindex.png index c4473a55386..5d55b19f6d8 100644 Binary files a/test/image/baselines/date_axes_period2_ticklabelindex.png and b/test/image/baselines/date_axes_period2_ticklabelindex.png differ diff --git a/test/image/baselines/date_axes_period_ticklabelindex.png b/test/image/baselines/date_axes_period_ticklabelindex.png index 49277798a98..b7843595c4a 100644 Binary files a/test/image/baselines/date_axes_period_ticklabelindex.png and b/test/image/baselines/date_axes_period_ticklabelindex.png differ diff --git a/test/image/baselines/ticklabelindex-2.png b/test/image/baselines/ticklabelindex-2.png new file mode 100644 index 00000000000..b1a7e3063f4 Binary files /dev/null and b/test/image/baselines/ticklabelindex-2.png differ diff --git a/test/image/baselines/ticklabelindex.png b/test/image/baselines/ticklabelindex.png index 5cf10b8ec07..f83b04d6680 100644 Binary files a/test/image/baselines/ticklabelindex.png and b/test/image/baselines/ticklabelindex.png differ diff --git a/test/image/mocks/ticklabelindex-2.json b/test/image/mocks/ticklabelindex-2.json new file mode 100644 index 00000000000..9321d4923ea --- /dev/null +++ b/test/image/mocks/ticklabelindex-2.json @@ -0,0 +1,68 @@ +{ + "data": [ + { + "x": [ + "2020", "2021", "2022" + ], + "y": [ + 3, 2, 1 + ] + }, + { + "mode": "markers+text", + "texttemplate": "%{x|Q%q %y}", + "textposition": "top center", + "x": [ + "2019-07-01", "2019-10-01", "2020-01-01", "2020-04-01", "2020-07-01" + ], + "y": [ + 2, 2, 2, 2, 2 + ], + "yaxis": "y2", + "xaxis": "x2" + } + ], + "layout": { + "width": 600, + "height": 800, + "grid": { + "rows": 2, + "columns": 1, + "pattern": "independent" + }, + "xaxis": { + "dtick": "M12", + "insiderange": [ + "2020-03-27", + "2022-07-21" + ], + "tickformat": "%Y", + "ticklabelindex": -1, + "ticks": "outside", + "ticklen": 20, + "ticklabelmode": "period", + "type": "date", + "title": { + "text": "Should display 2 major ticks and labels 2020, 2021, 2022" + } + }, + "xaxis2": { + "dtick": "M24", + "insiderange": [ + "2019-03-01", + "2021-01-01" + ], + "tick0": "2021-01-01", + "tickformat": "%Y", + "ticklabelindex": -1, + "ticklabelmode": "period", + "ticklen": 20, + "type": "date", + "ticks": "outside", + "minor": { "dtick": "M12", "ticks": "outside", "ticklen": 5 }, + "title": { + "text": "Should display 1 major tick, 1 minor tick and a label 2020 in between." + } + } + } +}