fix(xy): adding missing last tick and other tick improvements

packages/charts/src/chart_types/xy_chart/renderer/canvas/axes/tick.ts

@@ -11,15 +11,15 @@ import { colorToRgba, RgbaTuple } from '../../../../../common/color_library_wrap
 import { Position } from '../../../../../utils/common';
 import { isHorizontalAxis } from '../../../utils/axis_type_utils';
 import { AxisTick } from '../../../utils/axis_utils';
-import { HIERARCHICAL_GRID_WIDTH, OUTSIDE_RANGE_TOLERANCE } from '../../../utils/grid_lines';
+import { HIDE_MINOR_TIME_GRID, HIERARCHICAL_GRID_WIDTH, OUTSIDE_RANGE_TOLERANCE } from '../../../utils/grid_lines';
 import { renderMultiLine } from '../primitives/line';
 
 const BASELINE_CORRECTION = 2; // the bottom of the em is a bit higher than the bottom alignment; todo consider measuring
 
 /** @internal */
 export function renderTick(
   ctx: CanvasRenderingContext2D,
-  { position, domainClampedPosition: tickPosition, layer, detailedLayer, axisTickLabel }: AxisTick,
+  { position, domainClampedPosition: tickPosition, layer, detailedLayer }: AxisTick,
   {
     axisSpec: { position: axisPosition, timeAxisLayerCount },
     size: { width, height },
@@ -28,9 +28,13 @@ export function renderTick(
   }: AxisProps,
 ) {
   if (Math.abs(tickPosition - position) > OUTSIDE_RANGE_TOLERANCE) return;
-  const tickOnTheSide = timeAxisLayerCount > 0 && typeof layer === 'number' && axisTickLabel.length > 0;
+  const tickOnTheSide = timeAxisLayerCount > 0 && typeof layer === 'number';
+  const extensionLayer = tickOnTheSide ? layer + 1 : 0;
   const tickSize =
-    tickLine.size + (tickOnTheSide ? (layer + 1) * layerGirth + tickLine.padding - BASELINE_CORRECTION : 0);
+    tickLine.size +
+    (tickOnTheSide && (detailedLayer > 0 || !HIDE_MINOR_TIME_GRID)
+      ? extensionLayer * layerGirth + (extensionLayer < 1 ? 0 : tickLine.padding - BASELINE_CORRECTION)
+      : 0);
   const xy = isHorizontalAxis(axisPosition)
     ? {
         x1: tickPosition,

packages/charts/src/chart_types/xy_chart/state/selectors/visible_ticks.ts

@@ -40,6 +40,7 @@ type Projections = Map<AxisId, Projection>;
 
 const adaptiveTickCount = true;
 const MAX_TIME_TICK_COUNT = 50; // this doesn't do much for narrow charts, but limits tick count to a maximum on wider ones
+const MAX_TIME_GRID_COUNT = 12;
 const WIDTH_FUDGE = 1.05; // raster bin widths are sometimes approximate, but there's no raster that's just 5% denser/sparser, so it's safe
 
 function axisMinMax(axisPosition: Position, chartRotation: Rotation, { width, height }: Size): [number, number] {
@@ -60,6 +61,7 @@ export function generateTicks(
   tickFormatOptions: TickFormatterOptions & { labelFormat?: (d: string | number, ...otherArgs: unknown[]) => string },
   layer: number | undefined,
   detailedLayer = 0,
+  showGrid = true,
 ): AxisTick[] {
   const axisLabelFormat =
     tickFormatOptions.labelFormat ?? axisSpec.labelFormat ?? axisSpec.tickFormat ?? fallBackTickFormatter;
@@ -76,6 +78,7 @@ export function generateTicks(
       domainClampedPosition: (scale.scale(domainClampedValue) || 0) + offset, // todo it doesn't look desirable to convert a NaN into a zero
       layer,
       detailedLayer,
+      showGrid,
     };
   });
 }
@@ -93,6 +96,7 @@ function getVisibleTicks(
   detailedLayer: number,
   ticks: (number | string)[],
   isMultilayerTimeAxis: boolean = false,
+  showGrid = true,
 ): AxisTick[] {
   const isSingleValueScale = scale.domain[0] === scale.domain[1];
   const makeRaster = enableHistogramMode && scale.bandwidth > 0 && !isMultilayerTimeAxis;
@@ -123,6 +127,7 @@ function getVisibleTicks(
             domainClampedPosition: (scale.scale(firstTickValue) || 0) + offset,
             layer: undefined, // no multiple layers with `singleValueScale`s
             detailedLayer: 0,
+            showGrid,
           },
           {
             value: firstTickValue + scale.minInterval,
@@ -133,9 +138,20 @@ function getVisibleTicks(
             domainClampedPosition: scale.bandwidth + halfPadding * 2,
             layer: undefined, // no multiple layers with `singleValueScale`s
             detailedLayer: 0,
+            showGrid,
           },
         ]
-      : generateTicks(axisSpec, scale, ticks, offset, fallBackTickFormatter, tickFormatOptions, layer, detailedLayer);
+      : generateTicks(
+          axisSpec,
+          scale,
+          ticks,
+          offset,
+          fallBackTickFormatter,
+          tickFormatOptions,
+          layer,
+          detailedLayer,
+          showGrid,
+        );
 
   const { showOverlappingTicks, showOverlappingLabels, position } = axisSpec;
   const requiredSpace = isVerticalAxis(position) ? labelBox.maxLabelBboxHeight / 2 : labelBox.maxLabelBboxWidth / 2;
@@ -172,6 +188,7 @@ function getVisibleTickSet(
   ticks: (number | string)[],
   labelFormat?: (d: number | string) => string,
   isMultilayerTimeAxis = false,
+  showGrid = true,
 ): AxisTick[] {
   const vertical = isVerticalAxis(axisSpec.position);
   const tickFormatter = vertical ? fallBackTickFormatter : defaultTickFormatter;
@@ -191,6 +208,7 @@ function getVisibleTickSet(
     detailedLayer,
     ticks,
     isMultilayerTimeAxis,
+    showGrid,
   );
 }
 
@@ -209,15 +227,19 @@ export const getVisibleTickSetsSelector = createCustomCachedSelector(
   getVisibleTickSets,
 );
 
-const notTooDense = (domainFrom: number, domainTo: number, binWidth: number, cartesianWidth: number) => (
-  raster: TimeRaster<TimeBin>,
-) => {
+const notTooDense = (
+  domainFrom: number,
+  domainTo: number,
+  binWidth: number,
+  cartesianWidth: number,
+  maxTickCount = MAX_TIME_TICK_COUNT,
+) => (raster: TimeRaster<TimeBin>) => {
   const domainInSeconds = domainTo - domainFrom;
   const pixelsPerSecond = cartesianWidth / domainInSeconds;
   return (
     pixelsPerSecond > raster.minimumPixelsPerSecond &&
     raster.approxWidthInMs * WIDTH_FUDGE >= binWidth &&
-    (domainInSeconds * 1000) / MAX_TIME_TICK_COUNT <= raster.approxWidthInMs
+    (domainInSeconds * 1000) / maxTickCount <= raster.approxWidthInMs
   );
 };
 
@@ -248,6 +270,7 @@ function getVisibleTickSets(
         layer: number | undefined,
         detailedLayer: number,
         labelFormat?: (d: number | string) => string,
+        showGrid = true,
       ): Projection => {
         const labelBox = getLabelBox(axesStyle, ticks, labelFormat || tickFormatter, textMeasure, axisSpec, gridLine);
         return {
@@ -265,6 +288,7 @@ function getVisibleTickSets(
             ticks,
             labelFormat,
             isMultilayerTimeAxis,
+            showGrid,
           ),
           labelBox,
           scale, // tick count driving nicing; nicing drives domain; therefore scale may vary, downstream needs it
@@ -288,6 +312,7 @@ function getVisibleTickSets(
         detailedLayer: number,
         timeTicks: number[],
         labelFormat: (n: number) => string,
+        showGrid: boolean,
       ) => {
         const scale = getScale(100); // 10 is just a dummy value, the scale is only needed for its non-tick props like step, bandwidth, ...
         if (!scale) throw new Error('Scale generation for the multilayer axis failed');
@@ -298,6 +323,7 @@ function getVisibleTickSets(
             layer,
             detailedLayer,
             labelFormat as (d: number | string) => string,
+            showGrid,
           ),
           fallbackAskedTickCount: NaN,
         };
@@ -359,13 +385,13 @@ function getVisibleTickSets(
         const domainExtension = extendByOneBin ? binWidth : 0;
         const domainToS = (((domain && Number(domainValues[domainValues.length - 1])) || NaN) + domainExtension) / 1000;
         const layers = rasterSelector(notTooDense(domainFromS, domainToS, binWidth, Math.abs(range[1] - range[0])));
-        let layerIndex = 0;
+        let layerIndex = -1;
         return acc.set(
           axisId,
           layers.reduce(
             (combinedEntry: { ticks: AxisTick[] }, l: TimeRaster<TimeBin>, detailedLayerIndex) => {
+              if (l.labeled) layerIndex++; // we want three (or however many) _labeled_ axis layers; others are useful for minor ticks/gridlines, and for giving coarser structure eg. stronger gridline for every 6th hour of the day
               if (layerIndex >= timeAxisLayerCount) return combinedEntry;
-              // times 1000: convert seconds to milliseconds
               const { entry } = fillLayerTimeslip(
                 layerIndex,
                 detailedLayerIndex,
@@ -377,8 +403,8 @@ function getVisibleTickSets(
                   : layerIndex === timeAxisLayerCount - 1
                   ? l.detailedLabelFormat
                   : l.minorTickLabelFormat,
+                notTooDense(domainFromS, domainToS, binWidth, Math.abs(range[1] - range[0]), MAX_TIME_GRID_COUNT)(l),
               );
-              if (l.labeled) layerIndex++; // we want three (or however many) _labeled_ axis layers; others are useful for minor ticks/gridlines, and for giving coarser structure eg. stronger gridline for every 6th hour of the day
               const minLabelGap = 4;
 
               const lastTick = entry.ticks[entry.ticks.length - 1];

packages/charts/src/chart_types/xy_chart/utils/axis_utils.test.ts

@@ -1431,6 +1431,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 25.145833333333332,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547251200000,
@@ -1441,6 +1442,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 85.49583333333334,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547294400000,
@@ -1451,6 +1453,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 145.84583333333333,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547337600000,
@@ -1461,6 +1464,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 206.19583333333333,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547380800000,
@@ -1471,6 +1475,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 266.54583333333335,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547424000000,
@@ -1481,6 +1486,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 326.8958333333333,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547467200000,
@@ -1491,6 +1497,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 387.24583333333334,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547510400000,
@@ -1501,6 +1508,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 447.59583333333336,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547553600000,
@@ -1511,6 +1519,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 507.9458333333333,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547596800000,
@@ -1521,6 +1530,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 568.2958333333333,
         layer,
         detailedLayer,
+        showGrid: true,
       },
     ]);
   });
@@ -1562,6 +1572,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 25.145833333333332,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547251200000,
@@ -1572,6 +1583,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 85.49583333333334,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547294400000,
@@ -1582,6 +1594,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 145.84583333333333,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547337600000,
@@ -1592,6 +1605,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 206.19583333333333,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547380800000,
@@ -1602,6 +1616,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 266.54583333333335,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547424000000,
@@ -1612,6 +1627,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 326.8958333333333,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547467200000,
@@ -1622,6 +1638,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 387.24583333333334,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547510400000,
@@ -1632,6 +1649,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 447.59583333333336,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547553600000,
@@ -1642,6 +1660,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 507.9458333333333,
         layer,
         detailedLayer,
+        showGrid: true,
       },
       {
         value: 1547596800000,
@@ -1652,6 +1671,7 @@ describe('Axis computational utils', () => {
         domainClampedPosition: 568.2958333333333,
         layer,
         detailedLayer,
+        showGrid: true,
       },
     ]);
   });

packages/charts/src/chart_types/xy_chart/utils/axis_utils.ts

@@ -41,6 +41,7 @@ export interface AxisTick {
   domainClampedPosition: number;
   layer?: number;
   detailedLayer: number;
+  showGrid: boolean;
 }
 
 /** @internal */

packages/charts/src/chart_types/xy_chart/utils/grid_lines.ts

@@ -25,6 +25,8 @@ import { AxisSpec } from './specs';
 export const HIERARCHICAL_GRID_WIDTH = 1; // constant 1 scales well and solves some render issues due to fixed 1px wide overpaints
 /** @internal */
 export const OUTSIDE_RANGE_TOLERANCE = 0.01; // can protrude from the scale range by a max of 0.1px, to allow for FP imprecision
+/** @internal */
+export const HIDE_MINOR_TIME_GRID = false; // experimental: retain ticks but don't show grid lines for minor raster
 
 /** @internal */
 export interface GridLineGroup {
@@ -89,6 +91,8 @@ function getGridLinesForAxis(
 
   const visibleTicksPerLayer = visibleTicks.reduce((acc: Map<number, AxisTick[]>, tick) => {
     if (Math.abs(tick.position - tick.domainClampedPosition) > OUTSIDE_RANGE_TOLERANCE) return acc; // no gridline for ticks outside the domain
+    if (typeof tick.layer === 'number' && !tick.showGrid) return acc; // no gridline for ticks outside the domain
+    if (HIDE_MINOR_TIME_GRID && typeof tick.layer === 'number' && tick.detailedLayer === 0) return acc; // no gridline for ticks outside the domain
     const ticks = acc.get(tick.detailedLayer);
     if (ticks) {
       ticks.push(tick);