Repository: paularmstrong/build-tracker
Pull request #140 (2020-02-23 17:24:52)

[app] limit number of tick labels to prevent overlap
<!--
Thank you so much for contributing to open source and the Build Tracker project!
-->

# Problem

Too many builds on the graph can overlap labels

# Solution

<!--
When trying to solve more solutions with Build Tracker, please keep in mind some of the following goals of the project:
* Be lightweight: small package sizes (single-digit KiBs, gzipped)
* Be easy: too many options in an API can become confusing
* Be clear: the intended purpose of every method should be as obvious as possible
* Is it easy to do this in "userland"? Would it be better off done there?
-->

* Set a maximum number of tick labels to 50 (hard-coded)
* Evenly set tick labels to empty string if there are more than 50 builds in the x-axis domain

# TODO

- [x] 🤓 Add & update tests (always try to _increase_ test coverage)
- [ ] 🔬 Ensure CI is passing (`yarn lint:ci`, `yarn test`, `yarn tsc:ci`)
- [ ] 📖 Update relevant documentation



Patch:
diff --git a/src/app/src/components/Graph/XAxis.tsx b/src/app/src/components/Graph/XAxis.tsx
index 2c846e1..4340328 100644
--- a/src/app/src/components/Graph/XAxis.tsx
+++ b/src/app/src/components/Graph/XAxis.tsx
@@ -12,12 +12,21 @@ interface Props {
   scale: ScalePoint<string>;
 }
 
+const MAX_TICK_LABELS = 50;
+
 const XAxis = (props: Props): React.ReactElement => {
   const { height, scale } = props;
   const ref = React.useRef(null);
+  const domainLength = scale.domain().length;
 
   React.useEffect(() => {
-    const axis = axisBottom(scale).tickFormat(d => formatSha(d));
+    const axis = axisBottom(scale).tickFormat((d, i) => {
+      if (domainLength <= MAX_TICK_LABELS || i % Math.floor(domainLength / (MAX_TICK_LABELS / 2)) === 0) {
+        return formatSha(d);
+      }
+      return '';
+    });
+
     select(ref.current)
       .call(axis)
       .selectAll('text')
@@ -25,7 +34,7 @@ const XAxis = (props: Props): React.ReactElement => {
       .attr('x', 6)
       .attr('transform', 'rotate(24)')
       .style('text-anchor', 'start');
-  }, [scale]);
+  }, [domainLength, scale]);
 
   return <g ref={ref} transform={`translate(0, ${height})`} />;
 };


Test patch:
diff --git a/src/app/src/components/Graph/__tests__/XAxis.test.tsx b/src/app/src/components/Graph/__tests__/XAxis.test.tsx
index 680d824..7bc6693 100644
--- a/src/app/src/components/Graph/__tests__/XAxis.test.tsx
+++ b/src/app/src/components/Graph/__tests__/XAxis.test.tsx
@@ -8,12 +8,11 @@ import { render } from '@testing-library/react';
 import { scalePoint } from 'd3-scale';
 import XAxis from '../XAxis';
 
-const scale = scalePoint()
-  .range([0, 100])
-  .domain(['1234567abcdef', 'abcdefg1234567', 'abcd', '1234']);
-
 describe('XAxis', () => {
   test('creates a bottom axis', () => {
+    const scale = scalePoint()
+      .range([0, 100])
+      .domain(['1234567abcdef', 'abcdefg1234567', 'abcd', '1234']);
     const mockCall = jest.fn(() => {
       selectSpy.mockRestore();
       return Selection.select(document.createElement('g'));
@@ -31,6 +30,9 @@ describe('XAxis', () => {
   });
 
   test('formats shas', () => {
+    const scale = scalePoint()
+      .range([0, 100])
+      .domain(['1234567abcdef', 'abcdefg1234567', 'abcd', '1234']);
     const mockTickFormat = jest.fn(() => {
       axisSpy.mockRestore();
       return Axes.axisBottom(scale);
@@ -50,4 +52,34 @@ describe('XAxis', () => {
     // @ts-ignore
     expect(formatter('1234567abcdef')).toEqual('1234567');
   });
+
+  test('limits number of tick labels', () => {
+    const domain = new Array(150).fill(0).map((_v, i) => `12345${i}`);
+    const scale = scalePoint()
+      .range([0, 100])
+      .domain(domain);
+    const mockTickFormat = jest.fn(() => {
+      axisSpy.mockRestore();
+      return Axes.axisBottom(scale);
+    });
+    // @ts-ignore Just want to make sure call is called
+    const axisSpy = jest.spyOn(Axes, 'axisBottom').mockReturnValue({ tickFormat: mockTickFormat });
+
+    render(
+      <svg>
+        <XAxis height={400} scale={scale} />
+      </svg>
+    );
+
+    // @ts-ignore
+    const formatter: (string, number) => string = mockTickFormat.mock.calls[0][0];
+
+    expect(formatter('1234567abc', 0)).toEqual('1234567');
+    expect(formatter('1234567abc', 1)).toEqual('');
+    expect(formatter('1234567abc', 2)).toEqual('');
+    expect(formatter('1234567abc', 3)).toEqual('');
+    expect(formatter('1234567abc', 4)).toEqual('');
+    expect(formatter('1234567abc', 5)).toEqual('');
+    expect(formatter('1234567abc', 6)).toEqual('1234567');
+  });
 });