fix: use as many time ticks as d3 can fit on the x-axis

giraffe/src/utils/PlotEnv.ts

@@ -98,6 +98,7 @@ export class PlotEnv {
       this.xDomain,
       this.config.width,
       this.charMetrics.width,
+      this.config.tickFont,
       this.xTickFormatter
     )
   }
@@ -113,6 +114,7 @@ export class PlotEnv {
       this.yDomain,
       this.config.height,
       this.charMetrics.height,
+      this.config.tickFont,
       this.yTickFormatter
     )
   }

giraffe/src/utils/__mocks__/getTextMetrics.ts

@@ -0,0 +1,4 @@
+export const getTextMetrics = (...args) => ({
+  width: Math.floor(args[1].length * 1.2),
+  height: 12,
+})

giraffe/src/utils/getTicks.test.ts

@@ -0,0 +1,103 @@
+import {FormatterType} from '../types'
+import {getTicks} from './getTicks'
+
+jest.mock('./getTextMetrics')
+
+describe('getTicks', () => {
+  const charHeight = 12
+  const font = '10px sans-serif'
+
+  describe('vertical axis', () => {
+    const laptopScreenHeight = 788
+    const formatter = (num: number): string => `${num} foo`
+    const domain = [0, 100]
+    const result = [0, 10, 20, 30, 40, 50, 60, 70, 80, 90, 100]
+
+    it('should give the correct number of ticks', () => {
+      expect(
+        getTicks(domain, laptopScreenHeight, charHeight, font, formatter)
+      ).toEqual(result)
+    })
+    it('should handle extreme or unusual inputs', () => {
+      expect(getTicks(domain, 0, charHeight, font, formatter)).toEqual([])
+      expect(getTicks(domain, laptopScreenHeight, 0, font, formatter)).toEqual(
+        []
+      )
+      expect(
+        getTicks([0, 0], laptopScreenHeight, charHeight, font, formatter)
+      ).toEqual([0])
+      expect(
+        getTicks(domain, laptopScreenHeight, charHeight, font, formatter)
+      ).toEqual(result)
+    })
+  })
+
+  describe('horizontal axis', () => {
+    const axisLength = 1380
+    const formatter = (num: number): string => new Date(num).toDateString()
+    formatter._GIRAFFE_FORMATTER_TYPE = FormatterType.Time
+    const domain = [1578355945276, 1578357085276]
+    const result = [
+      1578355950000,
+      1578355980000,
+      1578356010000,
+      1578356040000,
+      1578356070000,
+      1578356100000,
+      1578356130000,
+      1578356160000,
+      1578356190000,
+      1578356220000,
+      1578356250000,
+      1578356280000,
+      1578356310000,
+      1578356340000,
+      1578356370000,
+      1578356400000,
+      1578356430000,
+      1578356460000,
+      1578356490000,
+      1578356520000,
+      1578356550000,
+      1578356580000,
+      1578356610000,
+      1578356640000,
+      1578356670000,
+      1578356700000,
+      1578356730000,
+      1578356760000,
+      1578356790000,
+      1578356820000,
+      1578356850000,
+      1578356880000,
+      1578356910000,
+      1578356940000,
+      1578356970000,
+      1578357000000,
+      1578357030000,
+      1578357060000,
+    ]
+    it('should give the correct number of ticks', () => {
+      expect(getTicks(domain, axisLength, charHeight, font, formatter)).toEqual(
+        result
+      )
+    })
+    it('should handle extreme or unusual inputs', () => {
+      expect(getTicks(domain, 0, charHeight, font, formatter)).toEqual([])
+      expect(getTicks([0, 0], axisLength, charHeight, font, formatter)).toEqual(
+        [0]
+      )
+      expect(getTicks(domain, axisLength, 0, font, formatter)).toEqual(result)
+      expect(
+        getTicks(domain, axisLength, 100000000000000000, font, formatter)
+      ).toEqual(result)
+      expect(getTicks(domain, axisLength, charHeight, font, formatter)).toEqual(
+        result
+      )
+      expect(
+        getTicks(domain, axisLength / 2, charHeight, font, formatter).length
+      ).toBeLessThan(result.length)
+      expect(getTicks(domain, 0, charHeight, font, formatter).length).toEqual(0)
+    })
+  })
+})

giraffe/src/utils/getTicks.ts

@@ -1,24 +1,13 @@
+// Libraries
 import {scaleUtc} from 'd3-scale'
 import {ticks} from 'd3-array'
+import memoizeOne from 'memoize-one'
 
+// Types
 import {Formatter, FormatterType} from '../types'
 
-export const getTicks = (
-  domain: number[],
-  rangeLength: number,
-  charLength: number,
-  formatter?: Formatter
-): number[] => {
-  const sampleTick = formatter(domain[1])
-  const numTicks = getNumTicks(sampleTick, rangeLength, charLength)
-  switch (formatter._GIRAFFE_FORMATTER_TYPE) {
-    case FormatterType.Time:
-      return getTimeTicks(domain, rangeLength, numTicks)
-
-    default:
-      return ticks(domain[0], domain[1], numTicks)
-  }
-}
+// Utils
+import {getTextMetrics} from './getTextMetrics'
 
 const getNumTicks = (
   sampleTick: string,
@@ -29,21 +18,53 @@ const getNumTicks = (
   return sampleTickWidth === 0 ? 0 : Math.round(length / sampleTickWidth)
 }
 
-const getTimeTicks = (
+const getOptimalTimeTicks = (
   [d0, d1]: number[],
-  length: number,
-  numTicks: number
-): number[] => {
-  const results = scaleUtc()
+  rangeLength: number,
+  timeTickLength: number
+): Date[] => {
+  const maxUseableLength = rangeLength - timeTickLength
+  const scaledTime = scaleUtc()
     .domain([d0, d1])
-    .range([0, length])
-    .ticks(numTicks)
-    .map(d => d.getTime())
-  // added this to force D3 to use the numTicks since D3
-  // treats the tick params as suggestions:
-  // https://observablehq.com/@d3/scale-ticks
-  if (results.length > numTicks) {
-    return results.slice(0, numTicks)
+    .range([0, rangeLength])
+
+  const maxNumTicks = Math.floor(rangeLength / timeTickLength)
+
+  let optimalTicks = scaledTime.ticks(maxNumTicks)
+  let counter = 1
+
+  while (
+    counter < maxNumTicks &&
+    optimalTicks.length * timeTickLength > maxUseableLength
+  ) {
+    optimalTicks = scaledTime.ticks(maxNumTicks - counter)
+    counter += 1
+  }
+  return optimalTicks
+}
+
+const getTimeTicksMemoized = memoizeOne(
+  ([d0, d1]: number[], length: number, timeTickLength: number): number[] => {
+    const timeTicks = getOptimalTimeTicks([d0, d1], length, timeTickLength)
+    return timeTicks.map(d => d.getTime())
+  }
+)
+
+export const getTicks = (
+  domain: number[],
+  rangeLength: number,
+  charLength: number,
+  tickFont: string,
+  formatter: Formatter
+): number[] => {
+  const sampleTick = formatter(domain[1])
+  const tickTextMetrics = getTextMetrics(tickFont, sampleTick)
+  const numTicks = getNumTicks(sampleTick, rangeLength, charLength)
+  switch (formatter._GIRAFFE_FORMATTER_TYPE) {
+    case FormatterType.Time:
+      return getTimeTicksMemoized(domain, rangeLength, tickTextMetrics.width)
+
+    default:
+      return ticks(domain[0], domain[1], numTicks)
   }
-  return results
 }

stories/src/index.stories.tsx

@@ -41,6 +41,7 @@ storiesOf('XY Plot', module)
         'DD/MM/YYYY HH:mm:ss.sss': 'DD/MM/YYYY HH:mm:ss.sss',
         'MM/DD/YYYY HH:mm:ss.sss': 'MM/DD/YYYY HH:mm:ss.sss',
         'YYYY/MM/DD HH:mm:ss': 'YYYY/MM/DD HH:mm:ss',
+        'YYYY-MM-DD HH:mm:ss ZZ': 'YYYY-MM-DD HH:mm:ss ZZ',
         'hh:mm a': 'hh:mm a',
         'HH:mm': 'HH:mm',
         'HH:mm:ss': 'HH:mm:ss',
@@ -49,7 +50,7 @@ storiesOf('XY Plot', module)
         'MMMM D, YYYY HH:mm:ss': 'MMMM D, YYYY HH:mm:ss',
         'dddd, MMMM D, YYYY HH:mm:ss': 'dddd, MMMM D, YYYY HH:mm:ss',
       },
-      'hh:mm a'
+      'YYYY-MM-DD HH:mm:ss ZZ'
     )
     const fill = fillKnob(table, 'cpu')
     const position = select(
@@ -118,6 +119,7 @@ storiesOf('XY Plot', module)
         'DD/MM/YYYY HH:mm:ss.sss': 'DD/MM/YYYY HH:mm:ss.sss',
         'MM/DD/YYYY HH:mm:ss.sss': 'MM/DD/YYYY HH:mm:ss.sss',
         'YYYY/MM/DD HH:mm:ss': 'YYYY/MM/DD HH:mm:ss',
+        'YYYY-MM-DD HH:mm:ss ZZ': 'YYYY-MM-DD HH:mm:ss ZZ',
         'hh:mm a': 'hh:mm a',
         'HH:mm': 'HH:mm',
         'HH:mm:ss': 'HH:mm:ss',
@@ -126,7 +128,7 @@ storiesOf('XY Plot', module)
         'MMMM D, YYYY HH:mm:ss': 'MMMM D, YYYY HH:mm:ss',
         'dddd, MMMM D, YYYY HH:mm:ss': 'dddd, MMMM D, YYYY HH:mm:ss',
       },
-      'YYYY/MM/DD HH:mm:ss'
+      'YYYY-MM-DD HH:mm:ss ZZ'
     )
     const fill = fillKnob(table, 'cpu')
     const interpolation = interpolationKnob()