Break chart lines across weeks a population is missing from (fix #339)

AGENTS.md

@@ -177,7 +177,7 @@ In recorded mode, `fixtureData.js`'s `test` answers every `https://data.firefox.
 | Switching branches without reinstalling | `node_modules` can still hold another branch's packages. Run `npm install` after a branch switch before trusting any result. |
 | Gitignored `src/components/views/css/` | Fresh clone cannot resolve the CSS imports until `npm run build:css` runs. Never hand-edit `css/` — edit `styl/`. |
 | Renaming a landmark ID | Breaks Playwright selectors silently. |
-| `g.chart-line path` in a Playwright locator | A population with one data point draws a zero-size line (shown as a dot, `g.chart-point circle`). To check a chart drew, use `g.chart-line path` with `.filter({ visible: true }).first()`. Match a line to its population by its stroke color against the legend entry's color, not by index: a population with no points keeps its legend entry but draws no line. |
+| `g.chart-line path` in a Playwright locator | A population with one data point, or a point with gaps on both sides, draws a zero-size line piece (shown as a dot, `g.chart-point circle`). A line breaks wherever its population skips a date that another population on the chart has. To check a chart drew, use `g.chart-line path` with `.filter({ visible: true }).first()`. Match a line to its population by its stroke color against the legend entry's color, not by index: a population with no points keeps its legend entry but draws no line. |
 | Redrawing every chart on every resize event | Across the hardware dashboard's 11 charts, redrawing on each resize event froze the page for about a minute on live data (#409). `containers/ChartContainer.jsx` waits 150 ms after the last resize event; `tests/playwright/specs/fixtures/resize.spec.js` guards it. |
 | `build:version.json` | Needs `build/` to already exist and `git` on PATH; must follow `build:app`. |
 | `no-shadow` / `no-redeclare` with `builtinGlobals` | Naming a local `name`, `status`, `history`, `event` is a hard error. |

CONTRIBUTING.md

@@ -265,7 +265,7 @@ style indirection layer, so a failure points straight at the markup it cares abo
 | Renaming a landmark ID | Breaks Playwright selectors silently. |
 | `build:version.json` | Needs `build/` to already exist and `git` on PATH; must follow `build:app`. |
 | `no-shadow` / `no-redeclare` with `builtinGlobals` | Naming a local `name`, `status`, `history`, `event` is a hard error. |
-| Observable Plot charts and `g.chart-line path` | A population with a single data point draws a zero-size line path, shown as a dot (`g.chart-point circle`) instead. To check that a chart drew a line, use `g.chart-line path` with `.filter({ visible: true }).first()`. A population with no points keeps its legend entry but draws no line, so match lines to legend entries by color, not by index. |
+| Observable Plot charts and `g.chart-line path` | A population with a single data point, or a point with gaps on both sides, draws a zero-size line piece, shown as a dot (`g.chart-point circle`) instead. A line breaks wherever its population skips a date that another population on the chart has. To check that a chart drew a line, use `g.chart-line path` with `.filter({ visible: true }).first()`. A population with no points keeps its legend entry but draws no line, so match lines to legend entries by color, not by index. |
 
 Known rot, recorded for recognition only, not as a roadmap: `react-app-polyfill` and
 `babel-polyfill`, plus a `browserslist` of `>0.2%, not dead, not ie <= 10, not op_mini all`, were

src/components/views/Chart.jsx

@@ -58,6 +58,31 @@ function assignMarkerRows(markers, xToPixel) {
     });
 }
 
+// Add an empty point for each date a population skips between its first and
+// last points, so its line breaks there instead of joining across the gap. A
+// date counts as skipped when another population on the chart has it.
+function withGaps(sortedPoints, chartDates, population) {
+    if (!sortedPoints.length) return [];
+
+    const pointsByDate = new Map(sortedPoints.map(point => [+point.x, point]));
+    const first = +sortedPoints[0].x;
+    const last = +sortedPoints.at(-1).x;
+
+    return chartDates
+        .filter(date => date >= first && date <= last)
+        .map(date => ({ ...(pointsByDate.get(date) || { x: new Date(date), y: null }), population }));
+}
+
+// A point with a gap or the end of its line on both sides has no line through
+// it, so it needs drawing on its own.
+function isolatedPoints(pointsWithGaps) {
+    return pointsWithGaps.filter((point, index) => (
+        point.y !== null
+        && (pointsWithGaps[index - 1]?.y ?? null) === null
+        && (pointsWithGaps[index + 1]?.y ?? null) === null
+    ));
+}
+
 export default props => {
     // Units that can appear right after a value, without a space in between
     const spacelessUnits = ['%'];
@@ -86,8 +111,14 @@ export default props => {
         return <p className="chart-empty">No data to show for this metric.</p>;
     }
 
-    const singlePoints = props.data.flatMap((population, index) => (
-        population.length === 1 ? [{ ...population[0], population: legend[index] }] : []
+    const chartDates = [...new Set(points.map(point => +point.x))].sort((dateA, dateB) => dateA - dateB);
+    const linePoints = legend.flatMap(populationName => withGaps(
+        points.filter(point => point.population === populationName),
+        chartDates,
+        populationName
+    ));
+    const loosePoints = legend.flatMap(populationName => isolatedPoints(
+        linePoints.filter(point => point.population === populationName)
     ));
 
     // The minimum and maximum y values among all populations.
@@ -195,7 +226,7 @@ export default props => {
 
     // A chart with a single population is shaded below its line.
     const maybeArea = props.data.length === 1 ? [
-        Plot.areaY(points, { x: 'x', y1: minYToShow, y2: 'y', curve: 'catmull-rom', fill: '#0000ff', fillOpacity: .2, className: 'chart-area' }),
+        Plot.areaY(linePoints, { x: 'x', y1: minYToShow, y2: 'y', curve: 'catmull-rom', fill: '#0000ff', fillOpacity: .2, className: 'chart-area' }),
     ] : [];
 
     const options = {
@@ -217,8 +248,8 @@ export default props => {
             Plot.ruleX(markersInRange, { x: 'x', stroke: '#b3b2b2', strokeDasharray: '3,1', className: 'chart-marker' }),
             ...markerLabelMarks,
             ...maybeArea,
-            Plot.line(points, { x: 'x', y: 'y', z: 'population', stroke: 'population', strokeWidth: 1, curve: 'catmull-rom', className: 'chart-line' }),
-            Plot.dot(singlePoints, { x: 'x', y: 'y', fill: 'population', r: 3, className: 'chart-point' }),
+            Plot.line(linePoints, { x: 'x', y: 'y', z: 'population', stroke: 'population', strokeWidth: 1, curve: 'catmull-rom', className: 'chart-line' }),
+            Plot.dot(loosePoints, { x: 'x', y: 'y', fill: 'population', r: 3, className: 'chart-point' }),
             Plot.dot(points, Plot.pointer({ x: 'x', y: 'y', fill: 'population', r: 4, maxRadius: hoverRadius, className: 'chart-hover-point' })),
         ],
     };

src/tests/jest/Chart.test.jsx

@@ -259,3 +259,64 @@ it('The y and x axes are drawn as grey lines along the left and bottom of the ch
     expect(left.getAttribute('x1')).toBe(left.getAttribute('x2'));
     expect(bottom.getAttribute('y1')).toBe(bottom.getAttribute('y2'));
 });
+
+const weeks = ['2026-01-04', '2026-01-11', '2026-01-18', '2026-01-25', '2026-02-01', '2026-02-08'].map(date => new Date(date));
+
+// Counts the visible pieces of a line. Plot writes an isolated point as a
+// lone move-to, which draws nothing, so only pieces that go somewhere count.
+function lineSegments(svg, stroke) {
+    const path = [...svg.querySelectorAll('g.chart-line path')].find(line => line.getAttribute('stroke') === stroke);
+    return path.getAttribute('d').split('M').filter(piece => /[LC]/.test(piece)).length;
+}
+
+it('A population\'s line breaks across weeks it is missing from, when other populations have those weeks', () => {
+    const svg = renderChart({
+        data: [
+            weeks.map((x, index) => ({ x, y: 10 + index })),
+            [weeks[0], weeks[1], weeks[4], weeks[5]].map(x => ({ x, y: 5 })),
+        ],
+    }).svg;
+
+    expect(lineSegments(svg, '#4040e8')).toBe(1);
+    expect(lineSegments(svg, '#05b378')).toBe(2);
+    expect(svg.querySelectorAll('g.chart-point circle')).toHaveLength(0);
+});
+
+it('A point with a gap on both sides is drawn as a dot', () => {
+    const svg = renderChart({
+        data: [
+            weeks.map((x, index) => ({ x, y: 10 + index })),
+            [weeks[0], weeks[1], weeks[3], weeks[5]].map(x => ({ x, y: 5 })),
+        ],
+    }).svg;
+
+    expect(lineSegments(svg, '#05b378')).toBe(1);
+    const dots = [...svg.querySelectorAll('g.chart-point circle')];
+    expect(dots).toHaveLength(2);
+    dots.forEach(dot => expect(dot.getAttribute('fill')).toBe('#05b378'));
+});
+
+it('A week that no population has does not break the lines', () => {
+    const everyWeekButOne = weeks.filter((x, index) => index !== 2);
+    const svg = renderChart({
+        data: [
+            everyWeekButOne.map((x, index) => ({ x, y: 10 + index })),
+            everyWeekButOne.map(x => ({ x, y: 5 })),
+        ],
+    }).svg;
+
+    expect(lineSegments(svg, '#4040e8')).toBe(1);
+    expect(lineSegments(svg, '#05b378')).toBe(1);
+});
+
+it('A population that starts later than the others is drawn from its first point, with no gap before it', () => {
+    const svg = renderChart({
+        data: [
+            weeks.map((x, index) => ({ x, y: 10 + index })),
+            weeks.slice(3).map(x => ({ x, y: 5 })),
+        ],
+    }).svg;
+
+    expect(lineSegments(svg, '#05b378')).toBe(1);
+    expect(svg.querySelectorAll('g.chart-point circle')).toHaveLength(0);
+});

tests/playwright/specs/fixtures/chart-series.spec.js

@@ -113,6 +113,28 @@ test('A line is drawn in date order even when its data is not', async ({ page })
     readoutDates.slice(1).forEach((date, index) => expect(date).toBeGreaterThan(readoutDates[index]));
 });
 
+test('A line breaks across weeks its population is missing from, and a point left on its own is a dot', async ({ page }) => {
+    // Oldest first. Sparse is missing two runs of weeks, leaving one week on
+    // its own between them.
+    const oldestFirst = dashboard.dates.toSorted();
+    const sparseWeeks = oldestFirst.filter((date, index) => ![3, 4, 6, 7].includes(index));
+    await reloadWithFirstMetric(page, {
+        Steady: oldestFirst.map(date => ({ x: date, y: 300 })),
+        Sparse: sparseWeeks.map(date => ({ x: date, y: 100 })),
+    });
+    const metricOverview = page.locator('#metric-overview-1');
+    const sparseColor = await metricOverview.locator('.legend .legend-entry').filter({ hasText: 'Sparse' })
+        .evaluate(element => getComputedStyle(element).color);
+
+    const sparseLine = await lineWithColor(metricOverview, sparseColor);
+    const visiblePieces = (await sparseLine.getAttribute('d')).split('M').filter(piece => /[LC]/.test(piece));
+    expect(visiblePieces, 'the line is drawn in two pieces, either side of the gaps').toHaveLength(2);
+
+    const dot = metricOverview.locator('g.chart-point circle');
+    await expect(dot).toHaveCount(1);
+    expect(await dot.evaluate(element => getComputedStyle(element).fill)).toBe(sparseColor);
+});
+
 test.describe('On a narrow screen', () => {
     test.use({ viewport: { width: 375, height: 800 } });