fix: use as many time ticks as d3 can fit on the x-axis (#146)

* fix: use the most number of time ticks that d3 will fit on the x-axis

* fix: use efficient spacing for small number of ticks and add explanations

* fix: update variable name

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,13 @@
+/*
+  utils/getTextMetrics uses the DOM to measure the actual width and height of a string
+  Since Jest does not have access to the DOM, we will fix the width and height to be some arbitrary numbers,
+  so that our tests will run properly for consumers of getTextMetrics
+
+  This is a mock using a fake font that will always have
+  width equal to 1.2 times the length of the string
+  heigth equal to 12
+*/
+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,79 @@ const getNumTicks = (
   return sampleTickWidth === 0 ? 0 : Math.round(length / sampleTickWidth)
 }
 
-const getTimeTicks = (
+/*
+  Optimal spacing defined as:
+    I. when there are less than four ticks:
+      for each tick, spacing is one-quarter of the length of a tick
+      thusly: 0.25, 0.50, 0.75 of a tick's length as spacing for one, two, and three ticks respectively
+    II. when there are four or more ticks:
+      sum of all tick lengths is shorter than total available space by at least one tick's length
+    III. Based on rules I and II, we call d3 with the suggested number of ticks, and
+      allow d3 to determine the actual number of ticks and spacing
+*/
+const hasOptimalSpacing = (
+  rangeLength: number,
+  timeTickLength: number,
+  ticks: Date[]
+): boolean => {
+  const fractionalSpaceAsPadding = 0.25
+  const totalLength = ticks.length * timeTickLength
+  if (ticks.length < 4) {
+    const padding = totalLength * fractionalSpaceAsPadding
+    if (totalLength < rangeLength - padding) {
+      return true
+    }
+    return false
+  } else if (totalLength < rangeLength - timeTickLength) {
+    return true
+  }
+  return false
+}
+
+const getOptimalTimeTicks = (
   [d0, d1]: number[],
-  length: number,
-  numTicks: number
-): number[] => {
-  const results = scaleUtc()
+  rangeLength: number,
+  timeTickLength: number
+): Date[] => {
+  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)
+
+  for (
+    let counter = 1;
+    counter < maxNumTicks &&
+    !hasOptimalSpacing(rangeLength, timeTickLength, optimalTicks);
+    counter += 1
+  ) {
+    optimalTicks = scaledTime.ticks(maxNumTicks - counter)
+  }
+  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()