Chart builder: SelectControl pickers, channel chooser, per-type aggregates

2026-08-08 02:02:33 +00:00 · 2026-06-12 14:45:31 +03:00
parent 4dcff4601d
commit ed66fe9c05
15 changed files with 1232 additions and 276 deletions
@@ -194,7 +194,7 @@ describe('ChartBuilderModal', () => {
      await vi.advanceTimersByTimeAsync(400); // drive the debounced preview render
    });

-    expect(container.querySelector('select[aria-label="Filter operator"]')).toBeTruthy();
+    expect(container.querySelector('button[aria-label^="Filter operator"]')).toBeTruthy();
    const calls = vi.mocked(renderSpec).mock.calls;
    const lastSpec = calls[calls.length - 1][1] as { transform?: unknown };
    expect(lastSpec.transform).toEqual([{ filter: { field: 'revenue', gt: 60 } }]);
@@ -297,7 +297,7 @@ describe('ChartBuilderModal', () => {
    expect(refLink()!.getAttribute('href')).toContain('vega.github.io');
  });

-  test('clicking a field in the shelf assigns it to a channel as a pill (field-first, 2B)', async () => {
+  test('clicking a field opens the channel chooser; picking a channel assigns it (field-first, 2B)', async () => {
    const ds = createDataset({
      name: 'Shop',
      data: [{ region: 'E', sales: 5 }],
@@ -309,7 +309,6 @@ describe('ChartBuilderModal', () => {
    const id = useDatasetStore.getState().datasets[0].id;
    const store = useChartBuilderStore.getState();
    store.init(id);
-    // Clear the smart-default axes so the click lands on the first empty channel (X).
    store.setChannelColumn('x', null);
    store.setChannelColumn('y', null);

@@ -328,12 +327,100 @@ describe('ChartBuilderModal', () => {
      await Promise.resolve();
    });

+    // Unarmed, the click opens an explicit channel chooser (portaled to <body>)
+    // rather than silently filling the first empty seat (council 2026-06-12).
+    expect(useChartBuilderStore.getState().config.encodings.x).toBeNull();
+    const option = Array.from(document.body.querySelectorAll('button')).find((b) =>
+      b.textContent?.includes('Columns (X)'),
+    );
+    expect(option).toBeDefined();
+
+    await act(async () => {
+      option!.click();
+      await Promise.resolve();
+    });
+
    expect(useChartBuilderStore.getState().config.encodings.x).toEqual({
      field: 'region',
      type: 'nominal',
    });
  });

+  test('an armed channel short-circuits the chooser: the field assigns directly (2B)', async () => {
+    const ds = createDataset({
+      name: 'Shop',
+      data: [{ region: 'E', sales: 5 }],
+      format: 'json',
+      source: 'inline',
+      now: T,
+    });
+    useDatasetStore.getState().add(ds);
+    const id = useDatasetStore.getState().datasets[0].id;
+    const store = useChartBuilderStore.getState();
+    store.init(id);
+    store.setChannelColumn('x', null);
+    store.setChannelColumn('y', null);
+    store.focusChannel('y');
+
+    await act(async () => {
+      root.render(<ChartBuilderModal />);
+      await Promise.resolve();
+    });
+
+    // The armed state announces itself at the shelf.
+    expect(container.textContent).toContain('Assigning to Y');
+
+    const fieldButton = Array.from(container.querySelectorAll('button')).find((b) =>
+      b.textContent?.includes('region'),
+    );
+    await act(async () => {
+      fieldButton!.click();
+      await Promise.resolve();
+    });
+
+    expect(useChartBuilderStore.getState().config.encodings.y).toEqual({
+      field: 'region',
+      type: 'nominal',
+    });
+    expect(useChartBuilderStore.getState().activeChannel).toBeNull();
+  });
+
+  test('opening a SelectControl lands focus on the selected option, not the first (regression)', async () => {
+    const ds = createDataset({
+      name: 'Sales',
+      data: [{ day: '2026-01-01', v: 1 }],
+      format: 'json',
+      source: 'inline',
+      now: T,
+    });
+    useDatasetStore.getState().add(ds);
+    const id = useDatasetStore.getState().datasets[0].id;
+    const store = useChartBuilderStore.getState();
+    store.init(id);
+    store.setChannelColumn('x', 'day'); // temporal → the pill offers Granularity
+    store.setChannelTimeUnit('x', 'month'); // "Month" sits mid-list, after "None (raw)"
+
+    await act(async () => {
+      root.render(<ChartBuilderModal />);
+      await Promise.resolve();
+    });
+
+    const trigger = container.querySelector<HTMLButtonElement>(
+      'button[aria-label^="Granularity for"]',
+    );
+    expect(trigger).toBeTruthy();
+    await act(async () => {
+      trigger!.click();
+      await Promise.resolve();
+    });
+
+    // A selector list ('[aria-current="true"], button') would return the first
+    // button in document order — the "None (raw)" option — instead of the selection.
+    const focused = document.activeElement as HTMLElement;
+    expect(focused.getAttribute('aria-current')).toBe('true');
+    expect(focused.textContent).toContain('Month');
+  });
+
  test('Colour can be switched to a constant value (the Property model, 2A/2B)', async () => {
    const ds = createDataset({
      name: 'Shop',
@@ -351,9 +438,10 @@ describe('ChartBuilderModal', () => {
      await Promise.resolve();
    });

-    // The empty Colour slot offers an "or constant" affordance (Colour is first in Marks).
+    // The empty Colour slot offers a "Use a constant" ghost button (Colour is
+    // first in Marks).
    const constButton = Array.from(container.querySelectorAll('button')).find(
-      (b) => b.textContent === 'or constant',
+      (b) => b.textContent === 'Use a constant',
    );
    expect(constButton).toBeDefined();