fix(charts): one gridline treatment for cartesian axes, keep category labels when compact (#1247)

.claude/skills/design-review/skill.md

@@ -132,6 +132,13 @@ grid: { left: 16, right: 16, top: 16, bottom: 24, containLabel: true }
 
 // Compact mode (container < 300px)
 grid: { left: 8, right: 8, top: 8, bottom: 8 }
+// Compact drops the axis NAME and the value-axis numbers. Category labels
+// stay — truncated to 10 chars under 400px — because a chart with no
+// category names identifies nothing (#1247).
+
+// Gridlines: one weight and colour for every cartesian chart, from the
+// registered theme (GRID_LINE_COLOR in charts/theme.ts). Charts set only
+// `splitLine: { show }` — never `splitLine.lineStyle`.
 
 // Legend position
 legend: { bottom: 0 }  // ALWAYS bottom-aligned
@@ -146,6 +153,8 @@ tooltip: { trigger: "axis", axisPointer: { type: "shadow" } }
 - NEVER set chart colors inline — always use `resolveChartColors()`.
 - NEVER add title inside the chart — widget card header IS the title.
 - NEVER register additional ECharts themes — use `neoboard-light` / `neoboard-dark` only.
+- NEVER set `splitLine.lineStyle` in a chart module — gridline weight belongs to the theme (#1247).
+- NEVER let a responsive breakpoint hide category labels — degrade to truncation, not to nothing (#1247).
 - Dark mode chart colors are DIFFERENT from light mode — this is by design (higher lightness for contrast).
 
 ### Graph Chart (NVL)

component/src/charts/__tests__/axis-label-utils.test.ts

@@ -48,9 +48,22 @@ describe("buildCategoryAxisLabel", () => {
     expect(result.tooltip).toEqual({ show: true });
   });
 
-  it("returns show: false when compact is true", () => {
-    const result = buildCategoryAxisLabel(10, { compact: true });
-    expect(result.show).toBe(false);
+  it("keeps labels in compact mode, truncated not hidden (#1247)", () => {
+    const result = buildCategoryAxisLabel(4, {
+      containerWidth: 280,
+    });
+    expect(result.show).toBe(true);
+    const fmt = result.formatter as (value: string) => string;
+    expect(fmt("Electronics")).toBe("Electroni…");
+    expect(fmt("Home")).toBe("Home");
+  });
+
+  it("keeps common-prefix labels distinguishable in a compact container", () => {
+    const result = buildCategoryAxisLabel(7, {
+      containerWidth: 280,
+    });
+    const fmt = result.formatter as (value: string) => string;
+    expect(new Set(["Widget A", "Widget G"].map(fmt)).size).toBe(2);
   });
 
   it("normalizes -1 sentinel to automatic rotation", () => {

component/src/charts/__tests__/cartesian-axis-defaults.test.tsx

@@ -0,0 +1,168 @@
+import { render } from "@testing-library/react";
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { BarChart } from "../bar-chart";
+import { LineChart } from "../line-chart";
+import { GanttChart } from "../gantt-chart";
+import { registerNeoboardThemes, GRID_LINE_COLOR } from "../theme";
+
+/**
+ * Cross-chart axis and gridline defaults (#1247).
+ *
+ * Two failures this pins down:
+ * 1. Gridline weight must come from the registered theme only — a chart that
+ *    declares its own splitLine.lineStyle makes two widgets on one dashboard
+ *    look like they came from different tools.
+ * 2. A compact container (< 300px) may drop value numbers, but never the
+ *    category identification — four unlabelled bars are not readable data.
+ */
+
+const mockSetOption = vi.fn();
+
+const size = vi.hoisted(() => ({ width: 600, height: 400 }));
+
+vi.mock("@/hooks/useContainerSize", () => ({
+  useContainerSize: () => ({
+    width: size.width,
+    height: size.height,
+    containerRef: vi.fn(),
+  }),
+}));
+
+vi.mock("echarts/core", () => {
+  const use = vi.fn();
+  const init = vi.fn(() => ({
+    setOption: mockSetOption,
+    resize: vi.fn(),
+    dispose: vi.fn(),
+    on: vi.fn(),
+    off: vi.fn(),
+    showLoading: vi.fn(),
+    hideLoading: vi.fn(),
+  }));
+  const registerTheme = vi.fn();
+  return { use, init, registerTheme, default: { use, init, registerTheme } };
+});
+
+type AxisOption = {
+  type?: string;
+  axisLabel?: { show?: boolean; formatter?: (v: string) => string };
+  splitLine?: { show?: boolean; lineStyle?: unknown };
+};
+
+/** Render a chart and return the flattened x/y axis options it emitted. */
+function axesOf(ui: React.ReactElement): AxisOption[] {
+  render(ui);
+  const opts = mockSetOption.mock.calls[0][0] as {
+    xAxis?: AxisOption | AxisOption[];
+    yAxis?: AxisOption | AxisOption[];
+  };
+  return [opts.xAxis, opts.yAxis].flat().filter(Boolean) as AxisOption[];
+}
+
+const categoryAxis = (axes: AxisOption[]) =>
+  axes.find((a) => a.type === "category") as AxisOption;
+const valueAxis = (axes: AxisOption[]) =>
+  axes.find((a) => a.type === "value") as AxisOption;
+
+const barData = [
+  { label: "Electronics & Media", value: 100 },
+  { label: "Home", value: 200 },
+  { label: "Garden", value: 150 },
+  { label: "Toys", value: 90 },
+];
+
+const lineData = [
+  { x: "Jan", value: 10 },
+  { x: "Feb", value: 20 },
+  { x: "Mar", value: 15 },
+];
+
+const ganttData = [
+  { task: "Design", start: 1700000000000, end: 1700500000000 },
+  { task: "Build", start: 1700500000000, end: 1701000000000 },
+];
+
+describe("cartesian gridlines come from the theme (#1247)", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    size.width = 600;
+    size.height = 400;
+  });
+
+  it.each([
+    ["bar", <BarChart key="b" data={barData} />],
+    ["line", <LineChart key="l" data={lineData} />],
+    ["gantt", <GanttChart key="g" data={ganttData} />],
+  ])("%s chart declares no gridline style of its own", (_name, ui) => {
+    for (const axis of axesOf(ui)) {
+      expect(axis.splitLine?.lineStyle).toBeUndefined();
+    }
+  });
+
+  it.each(["neoboard-light", "neoboard-dark"] as const)(
+    "%s registers one gridline colour for category and value axes",
+    (name) => {
+      const themes: Record<string, Record<string, never>> = {};
+      registerNeoboardThemes((themeName, theme) => {
+        themes[themeName] = theme as Record<string, never>;
+      });
+      const theme = themes[name] as unknown as Record<
+        string,
+        { splitLine: { lineStyle: { color: string } } }
+      >;
+      const expected =
+        GRID_LINE_COLOR[name === "neoboard-dark" ? "dark" : "light"];
+      // timeAxis included: a time-series line chart and the gantt draw
+      // ECharts' un-themed light-grey grid without it — glaring in dark.
+      for (const axis of ["categoryAxis", "valueAxis", "timeAxis"]) {
+        expect(theme[axis].splitLine.lineStyle.color).toBe(expected);
+      }
+    },
+  );
+});
+
+describe("compact containers keep category identification (#1247)", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    size.width = 280;
+    size.height = 400;
+  });
+
+  it("bar chart still labels its categories", () => {
+    const axis = categoryAxis(axesOf(<BarChart data={barData} />));
+    expect(axis.axisLabel?.show).toBe(true);
+  });
+
+  it("bar chart truncates rather than dropping labels", () => {
+    const axis = categoryAxis(axesOf(<BarChart data={barData} />));
+    const formatter = axis.axisLabel?.formatter as (v: string) => string;
+    expect(formatter("Electronics & Media")).toBe("Electroni…");
+    expect(formatter("Home")).toBe("Home");
+  });
+
+  it("bar chart still drops the value numbers", () => {
+    const axis = valueAxis(axesOf(<BarChart data={barData} />));
+    expect(axis.axisLabel?.show).toBe(false);
+  });
+
+  it("line chart truncates long category labels like the bar chart does", () => {
+    const axis = categoryAxis(
+      axesOf(
+        <LineChart
+          data={[
+            { x: "Electronics & Media", value: 10 },
+            { x: "Home", value: 20 },
+          ]}
+        />,
+      ),
+    );
+    const formatter = axis.axisLabel?.formatter as (v: string) => string;
+    expect(formatter("Electronics & Media")).toBe("Electroni\u2026");
+  });
+
+  it("line chart still labels its x axis", () => {
+    const axes = axesOf(<LineChart data={lineData} />);
+    expect(categoryAxis(axes).axisLabel?.show).toBe(true);
+    expect(valueAxis(axes).axisLabel?.show).toBe(false);
+  });
+});

component/src/charts/bar-chart.tsx

@@ -136,7 +136,6 @@ function BarChart({
 
     const categoryLabels = data.map((d) => d.label);
     const axisLabelConfig = buildCategoryAxisLabel(categoryLabels.length, {
-      compact,
       rotateOverride: axisLabelRotation,
       containerWidth: width,
     });

component/src/charts/chart-utils.ts

@@ -280,8 +280,6 @@ export interface CategoryAxisLabelOptions {
   rotateOverride?: number;
   /** Maximum label length before truncation (default: 15). */
   maxLabelLength?: number;
-  /** Whether the chart is in compact mode (hides labels). */
-  compact?: boolean;
   /** Container width in pixels — used for width-based auto-rotation. */
   containerWidth?: number;
 }
@@ -299,6 +297,8 @@ export interface CategoryAxisLabelConfig {
  * - 15+ categories: rotate 45°
  * - Labels longer than maxLabelLength are truncated with ellipsis (U+2026)
  * - ECharts axisPointer tooltip shows the full text on hover
+ * - Category labels are never hidden: a compact container drops the value
+ *   axis, since a chart with no category names identifies nothing (#1247)
  *
  * A `rotateOverride` of -1 is the "automatic" sentinel from the UI and is
  * normalized to undefined so the category-count heuristic applies.
@@ -307,7 +307,7 @@ export function buildCategoryAxisLabel(
   categoryCount: number,
   options: CategoryAxisLabelOptions = {},
 ): CategoryAxisLabelConfig {
-  const { maxLabelLength = 15, compact = false, containerWidth } = options;
+  const { maxLabelLength = 15, containerWidth } = options;
   // Normalize -1 sentinel (automatic mode) to undefined so ECharts uses its
   // default auto-rotation instead of receiving an invalid rotate: -1.
   const rotateOverride =
@@ -337,7 +337,10 @@ export function buildCategoryAxisLabel(
     rotate = 0;
   }
 
-  // Width-aware truncation: tighter limit in narrow containers
+  // Width-aware truncation: tighter limit in narrow containers. A compact
+  // container is by definition under 400px, so it already gets the tight
+  // budget — going tighter still collapses common-prefix labels
+  // ("Widget A".."Widget G") into seven identical stubs (#1247).
   const effectiveMaxLength =
     containerWidth && containerWidth < 400
       ? Math.min(maxLabelLength, 10)
@@ -353,7 +356,7 @@ export function buildCategoryAxisLabel(
     : undefined;
 
   return {
-    show: !compact,
+    show: true,
     rotate,
     formatter,
     tooltip: { show: true },

component/src/charts/gantt-chart.tsx

@@ -247,10 +247,9 @@ function GanttChart({
       },
       xAxis: {
         type: "time",
-        splitLine: {
-          show: showGridLines,
-          lineStyle: { type: "dashed", opacity: 0.3 },
-        },
+        // No local lineStyle: the registered theme owns gridline weight and
+        // colour so every cartesian chart draws the same grid (#1247).
+        splitLine: { show: showGridLines },
       },
       yAxis: {
         type: "category",

component/src/charts/line-chart.tsx

@@ -16,6 +16,7 @@ import {
   parseReferenceLines,
   buildMarkLineFromRefs,
   isTimeSeriesData,
+  buildCategoryAxisLabel,
   fadeToTransparent,
   isDark,
 } from "./chart-utils";
@@ -245,14 +246,32 @@ function LineChart({
           (useDualAxis && !compact ? 56 : 0) +
             (legendPos === "right" ? 40 : 0) || undefined,
       },
-      xAxis: {
-        type: useTimeAxis ? "time" : "category",
-        ...(useTimeAxis ? {} : { data: xValues.map(String) }),
-        name: compact ? undefined : xAxisLabel,
-        nameLocation: "middle",
-        nameGap: 30,
-        axisLabel: { show: !compact },
-      },
+      // Built as two literals rather than one object with ternaries: ECharts
+      // discriminates the axis union on `type`, so a `"time" | "category"`
+      // union defeats narrowing and the whole option fails to type-check.
+      // Compact drops the axis *name* and the value numbers, never the x
+      // labels (#1247). The category branch takes the shared config so long
+      // labels truncate exactly as they do on the bar chart; the time branch
+      // keeps plain labels, since ECharts formats and thins dates itself and
+      // the rotation heuristic is meaningless for them.
+      xAxis: useTimeAxis
+        ? {
+            type: "time" as const,
+            name: compact ? undefined : xAxisLabel,
+            nameLocation: "middle" as const,
+            nameGap: 30,
+            axisLabel: { show: true },
+          }
+        : {
+            type: "category" as const,
+            data: xValues.map(String),
+            name: compact ? undefined : xAxisLabel,
+            nameLocation: "middle" as const,
+            nameGap: 30,
+            axisLabel: buildCategoryAxisLabel(xValues.length, {
+              containerWidth: width,
+            }),
+          },
       yAxis: useDualAxis ? [leftYAxis, rightYAxis] : leftYAxis,
       series: seriesKeys.map((key, idx) => buildSeries(key, idx)),
     };
@@ -276,6 +295,7 @@ function LineChart({
     rightYAxisLabel,
     compact,
     hideLegend,
+    width,
     samplingThreshold,
     samplingMethod,
   ]);

component/src/charts/theme.ts

@@ -75,6 +75,18 @@ export function formatAxisCompact(value: number | string): string {
   return String(value);
 }
 
+/**
+ * One gridline colour per theme, for every cartesian chart (#1247).
+ *
+ * Charts must not set `splitLine.lineStyle` themselves — differing grid
+ * weight makes two widgets on one dashboard look like they came from
+ * different tools. Enforced by cartesian-axis-defaults.test.tsx.
+ */
+export const GRID_LINE_COLOR = {
+  light: "#f0f2f4",
+  dark: "#1d2025",
+} as const;
+
 function axisStyle(line: string, label: string, split: string) {
   return {
     axisLine: { lineStyle: { color: line } },
@@ -134,14 +146,17 @@ export function registerNeoboardThemes(
 ) {
   // Light: border hsl(220 13% 91%) ≈ #e5e7eb, muted-fg hsl(220 9% 44%) ≈ #666d7a
   const lightAxis = {
-    ...axisStyle("#e5e7eb", "#666d7a", "#f0f2f4"),
+    ...axisStyle("#e5e7eb", "#666d7a", GRID_LINE_COLOR.light),
   };
   registerTheme(THEME_LIGHT, {
     color: CITRINE_LIGHT,
     backgroundColor: "transparent",
     textStyle: { color: "#14161a" }, // foreground hsl(220 13% 9%)
     title: { textStyle: { color: "#14161a" } },
     categoryAxis: lightAxis,
+    // A time axis is a cartesian axis too — without this entry the gantt and
+    // any date-based line chart draw ECharts' un-themed grid (#1247).
+    timeAxis: lightAxis,
     valueAxis: {
       ...lightAxis,
       axisLabel: { color: "#666d7a", formatter: formatAxisCompact },
@@ -161,13 +176,15 @@ export function registerNeoboardThemes(
   });
 
   // Dark: border hsl(220 13% 17%) ≈ #262931, muted-fg hsl(220 9% 62%) ≈ #959ba7
-  const darkAxis = axisStyle("#262931", "#959ba7", "#1d2025");
+  const darkAxis = axisStyle("#262931", "#959ba7", GRID_LINE_COLOR.dark);
   registerTheme(THEME_DARK, {
     color: CITRINE_DARK,
     backgroundColor: "transparent",
     textStyle: { color: "#f3f4f6" }, // foreground hsl(220 14% 96%)
     title: { textStyle: { color: "#f3f4f6" } },
     categoryAxis: darkAxis,
+    // See the light theme note — time axes need the entry too (#1247).
+    timeAxis: darkAxis,
     valueAxis: {
       ...darkAxis,
       axisLabel: { color: "#959ba7", formatter: formatAxisCompact },