Ticklabelindex fixes for issues #7875 and #7876

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)]

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,23 +1291,23 @@ 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
             tickVal.simpleLabel // noSuffixPrefix
         );
         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;
 };
 

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."
+      }
+    }
+  }
+}