SelectControl: 32px trigger scale + option-group divider in the theme picker

This commit is contained in:
2026-06-12 21:46:53 +03:00
parent ca70b3e491
commit d1ba921141
8 changed files with 136 additions and 30 deletions
+1 -1
View File
@@ -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:
+6 -4
View File
@@ -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
+6 -2
View File
@@ -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',
+14 -5
View File
@@ -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;
+54
View File
@@ -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');
});
+13 -3
View File
@@ -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,8 +156,9 @@ export function SelectControl<V extends string>({
{options.map((o) => {
const selected = value !== undefined && o.value === value;
return (
<Fragment key={o.value}>
{o.dividerBefore && <div className={styles.divider} role="presentation" />}
<button
key={o.value}
type="button"
className={`${styles.option} ${selected ? styles.selected : ''}`}
aria-current={selected || undefined}
@@ -159,10 +166,13 @@ export function SelectControl<V extends string>({
>
<span className={styles.optionLabel}>
{o.label}
{o.detail !== undefined && <span className={styles.detail}>{o.detail}</span>}
{o.detail !== undefined && (
<span className={styles.detail}>{o.detail}</span>
)}
</span>
{selected && <span aria-hidden="true"></span>}
</button>
</Fragment>
);
})}
</div>
+12 -1
View File
@@ -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));
});
});
+19 -3
View File
@@ -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,
];
}
/**