Skip to content

fix(model-settings-menu): make the level being edited readable - #1186

Merged
sudomaggie merged 1 commit into
developfrom
fix/model-settings-menu-readability
Sep 1, 2026
Merged

fix(model-settings-menu): make the level being edited readable#1186
sudomaggie merged 1 commit into
developfrom
fix/model-settings-menu-readability

Conversation

@Harry19081

Copy link
Copy Markdown
Member

Problem

Four defects in the model pill's settings surface, all of the same kind — the UI
did not report the state it was editing.

  1. No feedback while dragging the effort slider. EffortSlider deliberately
    keeps a gesture local (// Keep rapid input local; persist only the completed gesture) and fires onChange on release. The compact panel passes
    showLabel={false}, so during a drag the fill moved but nothing anywhere —
    panel or pill — said which level the thumb had reached.
  2. The level was hard to read while editing it. The pill renders it in
    text-text-3 so the model name leads. That is right at rest and wrong while
    the panel that edits it is open.
  3. Submenu rows overflowed their row box. The Effort and Speed flyouts put a
    description on a second line, which does not fit DropdownItem's fixed
    height — the helper compensated with an !h-auto !py-2 override. Each flyout
    also repeated its own name as a sectionLabel title directly under the row
    that opened it.
  4. The model row's icon lied. It wore the same ArrowRight01Icon chevron
    that every ActionSubmenu row uses to promise a flyout, but it dismisses the
    whole menu and hands off to the model picker.

The compact panel had two smaller inconsistencies: its IconButton renders a
4px corner against the neighbouring Button's 8px at the same 28px height, and
both glyphs used DROPDOWN_ITEM.iconSize (13px), which is sized for 32px
dropdown rows.

Solution

Drag feedback without changing commit semantics. EffortSlider gains an
optional onPreviewChange, reporting the level under the thumb mid-gesture and
undefined when it ends. ModelSettingsMenu holds that preview and passes it to
renderTrigger; the pill renders previewLevel ?? variant.level. onChange
still fires exactly once, on release — the alternative (committing per tick)
would push every intermediate model id through the caller's save path. The
preview is scoped to an open menu and cleared when the compact view unmounts,
since the slider cannot report the end of a gesture it is no longer mounted for.

Tone. The level steps up to text-primary-6 while the panel is open, and
returns to text-text-3 when closed. Ultra keeps text-purple-6 in both states
— that purple is a marker, not a de-emphasis.

Rows. choice() drops its description parameter and the !h-auto !py-2
override, so every radio row is a single-line DropdownItem again. Both section
titles are removed; ActionSubmenu already sets aria-label={label} on the
flyout panel, so each is still announced as "Speed" / "Effort".
standardDescription, fastDescription and ultraDescription become
unreferenced and are deleted from en and zh — the only two locales that
carried them.

The model row takes Search01Icon (the spotlight's own glyph, ICONS.search)
so the affordance matches the searchable picker it opens rather than promising a
flyout.

Compact panel polish. The advanced-settings button becomes
variant="tertiary" (borderless, quieter beside the Fast toggle) and is
relabelled from "Model settings" to "Switch model", reusing the existing
sessions:creator.switchModel key — it was translated in en/zh and had zero
consumers, so this retires a dead key instead of duplicating a string. The Fast
toggle takes rounded-lg to match the Button's 8px corner, both glyphs go to
16px, and the bolt fills (fill="currentColor" strokeWidth={0}, the same pattern
used for PlayIcon/PauseIcon elsewhere) while Fast is on.

The panel's anchor gap goes from the engine's 4px default to 10px, so the pill
stays readable under the panel while the slider is dragged.

Potential risks

  • "Switch model" names a destination, not the click. That button still opens
    the advanced menu; it does not itself switch the model. The rename was
    requested with that behaviour explicitly left alone, but it is a wording/action
    mismatch a reviewer should weigh.
  • The Ultra warning is gone. "Uses usage limits faster" was a real hint and
    the screenshot that prompted this only showed the Speed flyout. It was removed
    for consistency — keeping the description path alive for one caller would
    have kept the two-line row shape. Ultra is still marked by purple label and
    checkmark. Easy to restore if the hint is wanted.
  • Preview/commit handoff. On release the preview clears and onChange fires
    in the same handler, so React batches them into one render. If a caller's apply
    path is asynchronous, the pill could show the pre-drag level for a frame. Not
    observed with the current callers, which update state synchronously.
  • rounded-lg relies on Tailwind's emit order (rounded-lg after rounded
    in the border-radius group) rather than an ! prefix. This matches the
    existing WorkstationTrailTerminalHeader override, but it is a precedence by
    convention, not by specificity.
  • The radius is fixed at one call site, not in IconButton. IconButton's
    base rounded (4px) matches no token in the codebase (DROPDOWN_ITEM is 6px,
    DROPDOWN_PANEL and Button are 8px) and 5 of its 22 call sites already
    override it. Changing the base would restyle 17 unreviewed surfaces, and 8px is
    wrong at the 20x20 sm size. The real fix is a size-scaled radius — a separate
    change.
  • i18n. Deleting three keys is safe only because nothing references them;
    grep confirms zero consumers, and selectors.modelProperties.settings
    survives as the panel's aria-label.

Verification

Run against this branch in an isolated worktree checked out at its own commit
(not the shared dirty checkout), with node_modules symlinked:

  • npx tsc --noEmit --pretty false -p tsconfig.json — exit 0.
  • npx vitest run --config config/vitest.config.ts src/components/ModelSelectorPill src/components/ModelPropertiesDropdown
    — 3 files, 27 tests, all passing.
  • npx eslint src/components/ModelSelectorPill/ src/components/ModelPropertiesDropdown/ — clean.
  • npx prettier --check on the changed files — clean.
  • Both edited locale files re-parse as valid JSON (json.load).

ModelSelectorPill.test.ts gains two tests:

  • Single-line, untitled submenus and the model row's search glyph: the Speed
    flyout's text is exactly "StandardFast", the Effort flyout's first child is a
    level row rather than a heading, both keep their aria-label, and the model
    row's only svg carries data-icon="search". Also pins the tertiary classes
    and the Fast toggle's rounded-lg.
  • Drag tracking and tone: drives a real pointerdown → range input
    pointerup sequence, asserting the pill reads GPT 5.6 Sol Light mid-drag
    with apply not yet called, then commits gpt-5.6-sol-low on release; and
    that the level is text-text-3 at rest and text-primary-6 while open.

Every new assertion was mutation-checked — reintroducing a second line, a section
title, the chevron, the outlined button variant, the 4px corner, the preview, or
the open highlight each fails the corresponding test.

Not run / not applicable:

  • No screenshots. ORG2 is a Tauri app with no browser-renderable entry point,
    so this panel cannot be captured without a full app launch. Behaviour is
    verified through jsdom rendering instead. This is the weakest part of the
    evidence here, since several changes are purely visual (icon size, bolt fill,
    corner radius, anchor gap); the tests pin the classes and attributes that
    produce them, not the rendered result.
  • The anchor gap is not asserted — jsdom returns zero rects, so there is
    nothing meaningful to measure. It is a constant passed to useDropdownEngine.
  • E2E was not run; no rendered spec covers this menu.
  • No "Pre-commit hook ran." trailer. The commit was built with plumbing
    (temp index + commit-tree) so the unrelated uncommitted work in the shared
    checkout could not enter it. Hooks were skipped, so typecheck, lint and tests
    were run by hand as listed above. The hook's TypeScript gate cannot fail a
    commit anyway (TSC_OUTPUT=$(...) || true then $? is always 0).
  • Base. Built on origin/develop (aa17dccd7). That commit's unused-export
    sweep (export interface ModelSelectorPillProps to interface ...) is
    preserved rather than reverted — the local checkout predates it.

While the effort slider was dragged nothing showed which level the thumb
had reached: EffortSlider keeps the gesture local and fires onChange only
on release, and the compact panel passes showLabel={false}. The submenu
rows also carried descriptions on a second line that broke the token-fixed
32px DropdownItem height, and the model row wore the same chevron every
ActionSubmenu row uses even though it dismisses the menu for the model
picker rather than opening a flyout.

EffortSlider gains onPreviewChange, reporting the level under the thumb
without committing it; the pill renders that preview and falls back to the
saved level when the gesture ends. Commit semantics are unchanged. The
level steps up to primary-6 while the panel is open, Ultra keeps purple.
Submenu rows lose their descriptions and section titles (the flyout is
still named by its aria-label), so the choice helper drops its description
parameter and the !h-auto row-height override. The model row takes a
search glyph, the advanced-menu button becomes tertiary and reuses the
existing sessions:creator.switchModel key, the Fast toggle matches the
Button's 8px corner, both compact glyphs go 13px to 16px, and the bolt
fills while Fast is on. standardDescription, fastDescription and
ultraDescription are now unreferenced and are removed from en and zh.
@Harry19081 Harry19081 added UX Improvements to user experience, workflow smoothness frontend-ui Frontend UI, design system, accessibility, layout, or theming labels Sep 1, 2026
@sudomaggie
sudomaggie merged commit 521a9ac into develop Sep 1, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend-ui Frontend UI, design system, accessibility, layout, or theming UX Improvements to user experience, workflow smoothness

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants