From d1ba921141e5952d7d7b61374c9fece0dec57c4e Mon Sep 17 00:00:00 2001
From: Oleh Omelchenko <ptrvtch@protonmail.com>
Date: Fri, 12 Jun 2026 21:46:53 +0300
Subject: [PATCH] SelectControl: 32px trigger scale + option-group divider in
 the theme picker

---
 docs/spec/04-live-preview.md                |  2 +-
 docs/ux-second-pass.md                      | 10 ++--
 src/app/components/LivePreview.tsx          |  8 ++-
 src/app/components/SelectControl.module.css | 19 ++++++--
 src/app/components/SelectControl.test.tsx   | 54 +++++++++++++++++++++
 src/app/components/SelectControl.tsx        | 38 +++++++++------
 src/core/vega-themes.test.ts                | 13 ++++-
 src/core/vega-themes.ts                     | 22 +++++++--
 8 files changed, 136 insertions(+), 30 deletions(-)
 create mode 100644 src/app/components/SelectControl.test.tsx

diff --git a/docs/spec/04-live-preview.md b/docs/spec/04-live-preview.md
index 561748d..3d6ee63 100644
--- a/docs/spec/04-live-preview.md
+++ b/docs/spec/04-live-preview.md
@@ -44,7 +44,7 @@ The preview pane header carries a **Chart theme** picker — a value-select disc
 - **Stock Vega-Lite** — injects nothing; charts render exactly as plain Vega-Lite defaults would anywhere else (white background, default palette and fonts).
 - **Custom themes** — the user's saved themes (see _Theme Builder_ below), listed by name between the built-ins and the presets.
 - **Edit themes…** — closes the custom-themes block (before the long preset roster, so it's visible without scrolling); opens the Theme Builder instead of changing the selection.
-- **Presets** — the `vega-themes` preset configs (Excel, ggplot2, FiveThirtyEight, LA Times, Power BI, the Carbon family, …), rendered verbatim and independent of the app's light/dark theme.
+- **Presets** — the `vega-themes` preset configs (Excel, ggplot2, FiveThirtyEight, LA Times, Power BI, the Carbon family, …), rendered verbatim and independent of the app's light/dark theme. A **divider** separates the preset roster from everything above it — the built-ins, the user's themes, and the manage entry read as "ours"; the presets as the imported catalogue.
 
 Behavior:
 
diff --git a/docs/ux-second-pass.md b/docs/ux-second-pass.md
index 819a43e..c3220bd 100644
--- a/docs/ux-second-pass.md
+++ b/docs/ux-second-pass.md
@@ -15,15 +15,17 @@ record the resolution into the contract (`docs/architecture/09`+`10` and the rel
   pane widths the long trigger labels ("FiveThirtyEight", "Urban Institute") may crowd it.
   Council questions: does the picker deserve header prominence (the "transform your chart"
   showcase) or settings-cluster placement (a persistent global pref); should the popover
-  registry learn nesting; 16 flat options — group presets under a heading?
+  registry learn nesting; should the preset block also carry a group _heading_ (a
+  divider now bounds it — 2026-06-12, user-decided; `SelectControl` `dividerBefore`)?
 
 - **"Edit themes…" action row inside the value picker** (`LivePreview.tsx` —
   ChartThemeControl). A non-value action lives inside a single-select disclosure (the
   VS Code theme-picker pattern), placed after the custom-themes block and before the
   preset roster (first-use feedback: at the very bottom it was invisible without
-  scrolling). Council questions: should an action be visually separated from the
-  values (divider, distinct styling); is a mid-list row that opens a modal instead of
-  selecting surprising to AT users?
+  scrolling). The ours-vs-presets divider (2026-06-12) now bounds it below; remaining
+  council questions: should the action row itself be styled distinctly from the value
+  rows; is a mid-list row that opens a modal instead of selecting surprising to AT
+  users?
 
 - **Theme Builder config editor is a plain textarea** (`ThemeBuilderModal.tsx`). Monaco
   (with the Vega-Lite config schema for completions) would match the main editor but is
diff --git a/src/app/components/LivePreview.tsx b/src/app/components/LivePreview.tsx
index 158987d..a064302 100644
--- a/src/app/components/LivePreview.tsx
+++ b/src/app/components/LivePreview.tsx
@@ -100,10 +100,14 @@ function ChartThemeControl() {
     // pattern); choosing it opens the builder and leaves the selection alone.
     // It closes the custom-themes block — right after the built-ins, BEFORE the
     // long preset roster — so it is visible without scrolling and sits next to
-    // the entries it manages.
+    // the entries it manages. The roster boundary itself (the divider) is set by
+    // chartThemeOptions where the order is decided, so the splice is the only
+    // index this component owns. The first preset is the divider-carrying option,
+    // so splicing right before it needs no count arithmetic.
     // TODO: action row inside a value picker (visual separation? AT surprise?)
     // parked for the batched council pass (docs/ux-second-pass.md).
-    list.splice(2 + customThemes.length, 0, {
+    const firstPreset = list.findIndex((o) => o.dividerBefore);
+    list.splice(firstPreset === -1 ? list.length : firstPreset, 0, {
       value: EDIT_THEMES,
       label: 'Edit themes…',
       detail: 'Create and manage custom themes',
diff --git a/src/app/components/SelectControl.module.css b/src/app/components/SelectControl.module.css
index 7a6afe9..f3d7a79 100644
--- a/src/app/components/SelectControl.module.css
+++ b/src/app/components/SelectControl.module.css
@@ -1,17 +1,19 @@
 /* SelectControl — the app's value-picker disclosure (replaces native <select>;
-   arch 10 §5). Trigger + panel mirror SortControl's geometry and tokens. */
+   arch 10 §5). Trigger + panel mirror SortControl's geometry and tokens: the
+   32px compact control height (arch 09 §6), value text at the option size. */
 
 .trigger {
   display: inline-flex;
   align-items: center;
-  gap: var(--space-1);
-  padding: var(--space-1) var(--space-2);
+  gap: var(--space-2);
+  height: 32px;
+  padding: 0 var(--space-3);
   border: var(--border-width) solid var(--border-strong);
   border-radius: var(--radius);
   background: var(--bg);
   color: var(--text);
   font: inherit;
-  font-size: 12px;
+  font-size: 13px;
   white-space: nowrap;
   cursor: pointer;
   transition:
@@ -44,7 +46,7 @@
 }
 
 .caret {
-  font-size: 9px;
+  font-size: 10px;
   color: var(--text-secondary);
 }
 
@@ -79,6 +81,13 @@
   overflow-y: auto;
 }
 
+/* Group separator (an option's `dividerBefore`) — spans the panel edge to edge. */
+.divider {
+  height: var(--border-width);
+  margin: var(--space-2) calc(-1 * var(--space-2));
+  background: var(--border);
+}
+
 .option {
   display: flex;
   align-items: center;
diff --git a/src/app/components/SelectControl.test.tsx b/src/app/components/SelectControl.test.tsx
new file mode 100644
index 0000000..3b54b82
--- /dev/null
+++ b/src/app/components/SelectControl.test.tsx
@@ -0,0 +1,54 @@
+import { afterEach, beforeEach, expect, test } from 'vitest';
+import { act } from 'react';
+import { createRoot, type Root } from 'react-dom/client';
+import { SelectControl } from './SelectControl';
+
+(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
+
+let container: HTMLDivElement;
+let root: Root;
+
+beforeEach(() => {
+  container = document.createElement('div');
+  document.body.appendChild(container);
+  root = createRoot(container);
+});
+
+afterEach(() => {
+  act(() => root.unmount());
+  container.remove();
+});
+
+test('opens to the option list; a dividerBefore option draws a separator above itself', async () => {
+  await act(async () => {
+    root.render(
+      <SelectControl
+        id="sc-test"
+        label="Pick"
+        options={[
+          { value: 'a', label: 'A' },
+          { value: 'b', label: 'B', dividerBefore: true },
+        ]}
+        value="a"
+        onSelect={() => {}}
+      />,
+    );
+    await Promise.resolve();
+  });
+
+  const trigger = container.querySelector('button')!;
+  await act(async () => {
+    trigger.click();
+    await Promise.resolve();
+  });
+
+  // The panel is portaled to <body>; both options render, with one presentation
+  // divider sitting between A and B (visual only — not in the keyboard order).
+  const panel = document.getElementById('sc-test')!;
+  const labels = Array.from(panel.querySelectorAll('button')).map((b) => b.textContent);
+  expect(labels.some((t) => t?.includes('A'))).toBe(true);
+  expect(labels.some((t) => t?.includes('B'))).toBe(true);
+  const dividers = panel.querySelectorAll('[role="presentation"]');
+  expect(dividers).toHaveLength(1);
+  expect(dividers[0].nextElementSibling?.textContent).toContain('B');
+});
diff --git a/src/app/components/SelectControl.tsx b/src/app/components/SelectControl.tsx
index a17c877..cfff529 100644
--- a/src/app/components/SelectControl.tsx
+++ b/src/app/components/SelectControl.tsx
@@ -22,7 +22,7 @@
  * caller intercept the click entirely (the armed-channel fast path).
  */
 
-import { type ReactNode } from 'react';
+import { Fragment, type ReactNode } from 'react';
 import { createPortal } from 'react-dom';
 import { usePopover } from '../hooks/usePopover';
 import styles from './SelectControl.module.css';
@@ -35,6 +35,12 @@ export interface SelectControlOption<V extends string> {
   label: string;
   /** Optional secondary line (e.g. "replaces Ship Mode" on an occupied channel). */
   detail?: string;
+  /**
+   * Draw a group separator above this option (purely visual, `role="presentation"`;
+   * keyboard order is untouched) — e.g. the chart-theme picker's boundary between
+   * the user's themes and the preset roster.
+   */
+  dividerBefore?: boolean;
 }
 
 export interface SelectControlProps<V extends string> {
@@ -150,19 +156,23 @@ export function SelectControl<V extends string>({
               {options.map((o) => {
                 const selected = value !== undefined && o.value === value;
                 return (
-                  <button
-                    key={o.value}
-                    type="button"
-                    className={`${styles.option} ${selected ? styles.selected : ''}`}
-                    aria-current={selected || undefined}
-                    onClick={() => choose(o.value)}
-                  >
-                    <span className={styles.optionLabel}>
-                      {o.label}
-                      {o.detail !== undefined && <span className={styles.detail}>{o.detail}</span>}
-                    </span>
-                    {selected && <span aria-hidden="true">✓</span>}
-                  </button>
+                  <Fragment key={o.value}>
+                    {o.dividerBefore && <div className={styles.divider} role="presentation" />}
+                    <button
+                      type="button"
+                      className={`${styles.option} ${selected ? styles.selected : ''}`}
+                      aria-current={selected || undefined}
+                      onClick={() => choose(o.value)}
+                    >
+                      <span className={styles.optionLabel}>
+                        {o.label}
+                        {o.detail !== undefined && (
+                          <span className={styles.detail}>{o.detail}</span>
+                        )}
+                      </span>
+                      {selected && <span aria-hidden="true">✓</span>}
+                    </button>
+                  </Fragment>
                 );
               })}
             </div>
diff --git a/src/core/vega-themes.test.ts b/src/core/vega-themes.test.ts
index 9432b8d..ba0be2f 100644
--- a/src/core/vega-themes.test.ts
+++ b/src/core/vega-themes.test.ts
@@ -171,7 +171,18 @@ describe('custom theme selections', () => {
     expect(options[2].label).toBe('Brand');
   });
 
+  it('marks the roster boundary on the first preset, wherever the customs end', () => {
+    for (const customs of [[], themes]) {
+      const options = chartThemeOptions(customs);
+      const dividers = options.filter((o) => o.dividerBefore);
+      expect(dividers).toHaveLength(1);
+      expect(dividers[0].value).toBe(CHART_THEME_OPTIONS[2].value); // first preset
+      expect(options.indexOf(dividers[0])).toBe(2 + customs.length);
+    }
+  });
+
   it('lists no custom entries when the library has none', () => {
-    expect(chartThemeOptions([])).toEqual([...CHART_THEME_OPTIONS]);
+    const values = chartThemeOptions([]).map((o) => o.value);
+    expect(values).toEqual(CHART_THEME_OPTIONS.map((o) => o.value));
   });
 });
diff --git a/src/core/vega-themes.ts b/src/core/vega-themes.ts
index 1898820..2093bdf 100644
--- a/src/core/vega-themes.ts
+++ b/src/core/vega-themes.ts
@@ -191,6 +191,12 @@ export interface ChartThemeOption {
   label: string;
   /** Secondary line for pickers (what the choice means). */
   detail?: string;
+  /**
+   * Group boundary: this option starts the preset roster, visually separated
+   * from the built-ins and the user's themes above it. Set here, where the list
+   * order is decided — consumers must never recompute the boundary by index.
+   */
+  dividerBefore?: boolean;
 }
 
 /** Display metadata for every selectable chart theme, in display order. */
@@ -252,7 +258,9 @@ export function isChartThemeSelection(value: unknown): value is ChartThemeSelect
 
 /**
  * The full picker option list: built-ins, the user's saved themes (by name, in
- * library order), then the presets. Pure derivation — callers memoize.
+ * library order), then the presets — with the first preset carrying the group
+ * divider that bounds "ours" from the imported roster. Pure derivation —
+ * callers memoize.
  */
 export function chartThemeOptions(
   customThemes: ReadonlyArray<Pick<CustomTheme, 'id' | 'name'>>,
@@ -262,8 +270,16 @@ export function chartThemeOptions(
     label: t.name,
     detail: 'Custom theme',
   }));
-  // Built-ins first, the user's own themes next, the preset roster last.
-  return [...CHART_THEME_OPTIONS.slice(0, 2), ...custom, ...CHART_THEME_OPTIONS.slice(2)];
+  // Built-ins first, the user's own themes next, the preset roster last. The
+  // divider rides on the first preset option itself, so it stays correct however
+  // a consumer splices into the list (e.g. the picker's "Edit themes…" row).
+  const [firstPreset, ...presets] = CHART_THEME_OPTIONS.slice(2);
+  return [
+    ...CHART_THEME_OPTIONS.slice(0, 2),
+    ...custom,
+    { ...firstPreset, dividerBefore: true },
+    ...presets,
+  ];
 }
 
 /**