') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); refactor(ui): unify chat model pickers onto the grouped settings-select form by Astro-Han · Pull Request #353 · apache/maka · GitHub
Skip to content

refactor(ui): unify chat model pickers onto the grouped settings-select form - #353

Merged
Astro-Han merged 5 commits into
mainfrom
claude/model-picker-ui
Jun 28, 2026
Merged

refactor(ui): unify chat model pickers onto the grouped settings-select form#353
Astro-Han merged 5 commits into
mainfrom
claude/model-picker-ui

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Unifies the two chat model pickers (in-session ChatModelSwitcher, new-chat NewChatModelPicker) onto one governed dropdown form. Their grouped list, provider headings, and checkmark rows now wear the shared .settingsSelectMenu* recipe in the settings-select family — replacing the bespoke .maka-model-switcher-* popup CSS — and group headings carry the provider brand mark.

Why

Two chat pickers each hand-rolled their own popup/heading/row CSS, drifting from the governed settings-select look. Folding the grouped-menu look into that one CSS family gives every picker the same list, headings, and checkmark selection, with brand logos on the provider groups, instead of a parallel recipe each future reader must decode.

Closes # (no linked issue)

Scope

Changed:

  • feat(ui) — add the .settingsSelectMenu* CSS recipe (grouped popup geometry, provider heading with a brand-mark gutter, checkmark rows) to the settings-select family.
  • refactor(ui) — render both chat pickers' grouped list through it via the shared ModelChoiceOptions; delete the bespoke .maka-model-switcher-* popup/positioner/heading/row CSS (trigger CSS kept). Headings drop the uppercase eyebrow for sentence-case at weight 500. Brand mark injected from the desktop app via the existing renderProviderMark seam so the provider SVG library stays out of @maka/ui. ModelMenuGroup gains providerType to pick the mark.
  • fix(ui) — make the capped menu popup the scroll container; it capped at 420px with overflow visible, so a provider with many models spilled its overflow rows below the rounded box. Now they clip and scroll (keyboard nav to the last row verified over CDP).
  • test(ui) — behavioral coverage for ModelMenuGroup.providerType; contract tests re-pointed to the new chrome.

Not included:

  • The composer pill trigger is deliberately untouched — its 22px pill chrome (status dot, pending state, press feedback) is composer-layout concern, not model selection, and folding it into the settings trigger would add CSS with no payoff. This PR governs the list, not the trigger.
  • The sticky-model send-path contract guards (session-sticky-model-contract.test.ts) are intact — only the picker-chrome assertions move.
  • No new React render-test harness: the repo has none (pure-logic node:test + source contracts + CDP visual smoke). Interactive behavior is verified over CDP; introducing RTL is out of scope.

Verification

  • npm run typecheck (all workspaces) — clean
  • npm run -w @maka/desktop test — 1608 pass
  • npm run -w @maka/ui test — 13 pass
  • Live-app CDP (turn-narrative fixture): grouped menu renders Z.AI / 自定义 / Anthropic / OpenAI headings with brand marks (Anthropic in brand orange) + checkmark on the selected row; overflow clips + scrolls; ArrowDown/End reaches and reveals the last row (lastFullyVisibleInPopup: true).
image

User-facing impact

The in-session and new-chat model menus now group models by provider with a brand mark on each heading; selection is the standard checkmark. No behavior change to model switching or new-chat creation. No docs / CHANGELOG / migration.

Reviewer notes

  • Heading weight is 500 (not the old 600): 600 propped up an uppercase eyebrow; sentence-case + a colored brand mark + muted ink already differentiate headings from full-ink rows, so 500 is the calmer, lower-surprise choice (matches the base SelectGroupLabel default).
  • No premature abstraction: the grouped body lives in ModelChoiceOptions (shared by both pickers); the only cross-cutting governance is the .settingsSelectMenu* CSS recipe. A reusable component waits for a second, non-chat grouped select.
  • Net production LOC is roughly flat — the value is consolidating the grouped-menu look into one owned style family, not a raw line reduction.

Add SettingsSelectGroups — a grouped popup body (provider-style headings
with an optional injected brand mark over the shared SelectItem checkmark
rows) — plus the .settingsSelectMenu* CSS recipe that pins its roomier
geometry. This folds the grouped-menu look into the one settings-select
family so any picker with a bespoke trigger can render the same governed
list without a parallel CSS recipe.
…ect form
The in-session and new-chat model pickers rendered their grouped list with a
parallel '.maka-model-switcher-*' popup recipe. Route both through the shared
SettingsSelectGroups menu body so the list, headings, and checkmark rows are
the one governed look; the composer pill trigger stays bespoke and untouched.
- ModelChoiceOptions delegates to SettingsSelectGroups; popups use the
'.settingsSelectMenu*' classes; the bespoke popup/positioner/heading/row CSS
is deleted (trigger CSS kept).
- Group headings carry the provider brand mark, injected from the desktop app
via the existing renderProviderMark seam (threaded through the Composer) so
the provider SVG library stays out of @maka/ui. Headings drop the uppercase
eyebrow for sentence-case at weight 500 — the mark now anchors scanning.
- ModelMenuGroup carries providerType so the menu can pick the right mark.
- Contract tests: re-point only the popup-chrome assertions (the grouped list
now flows through the shared form); the sticky-model send-path guards are
untouched.
The .settingsSelectMenuPopup capped its height at 420px but kept overflow
visible, and the inner SelectList's --available-height ceiling was too tall
to engage — so when a provider exposes many models the overflow rows spilled
out below the rounded box (verified over CDP: scrollHeight 479 > clientHeight
418, last row bottom 806 vs popup bottom 754). Make the capped popup the
scroll container so the rows clip and scroll cleanly.
Review (P3, Occam): SettingsSelectGroups + SettingsSelectGroup wrapped what is
only shared inside the model menu into a 'generic' component with its own type,
adding a production layer whose sole consumer was ModelChoiceOptions. Render the
provider headings, brand mark, and rows directly in ModelChoiceOptions; the
shared governance stays in the .settingsSelectMenu* CSS recipe. Extract a
component again only when a second, non-chat grouped select appears.
Both pickers still share one ModelChoiceOptions; grouping, the selected check,
and the unknown-current-model fallback are unchanged.
Review (P3): add behavioral coverage for the ModelMenuGroup.providerType field
introduced for the grouped menu — a dropped/wrong providerType would silently
render the wrong provider logo. Pure-logic test alongside the existing
heading/leak-safety coverage (the repo has no React render harness; interactive
behavior — open, headings, brand mark, selected row, keyboard scroll — is
verified over CDP).
@Astro-Han

Copy link
Copy Markdown
ContributorAuthor

代码评审回应

逐条按证据处理,采纳两条、用真机数据解决两条"待验证"。

P3 — 不够奥卡姆(SettingsSelectGroups 多包一层)→ 已采纳

砍掉 SettingsSelectGroups 组件和 SettingsSelectGroup 类型,品牌 mark / 分组标题 / .settingsSelectMenu* class 直接放回 ModelChoiceOptions;共享治理留在 CSS recipe 这一层,等出现第二个非聊天场景再抽组件。两个 picker 仍共享同一个 ModelChoiceOptions,分组、选中 check、未知当前模型 fallback 不变。 (3e03066b)

P3 — 测试不够精准 → 部分采纳 + 说明

仓库没有任何 React 渲染/交互测试基础设施(无 jsdom / testing-library / react-test-renderer),@maka/ui 测试是纯逻辑 node:test,渲染靠 source 契约 + CDP visual smoke(repo 既定做法)。引入 RTL 是比本 PR 更大的范式改动,超出范围。所以:

  • 新增纯逻辑行为测试,覆盖驱动品牌 mark 的 ModelMenuGroup.providerType(掉了会静默显示错 logo)。 (0a828229)
  • 保留 CSS 契约(守 CSS 删除回归)。
  • 交互行为用 CDP 实测(见下),而非源码字符串。

待验证 — 长列表键盘导航 + 滚动可见性 → 真机验证通过

CDP 实测(turn-narrative fixture,10 个模型 + 4 个分组,弹层限高 420px):ArrowDown×14 + End 后,最后一项 GPT-4o mini 被高亮,弹层自动滚动(scrollTop 53.5 / max 69),lastFullyVisibleInPopup: true。Base UI 的 scroll-into-view 正确驱动 overflow-y:auto 的弹层。所以溢出修法成立,无需改回 SelectList 作唯一滚动容器。

这个溢出 bug 本身也是 CDP 发现的:修前 scrollHeight 479 > clientHeight 418overflow: visible,最后一行画到圆角框外(bottom 806 vs 弹层 bottom 754)。修后弹层自身成为滚动容器,干净裁切 + 滚动。

待验证 — 缺 PR 级 Checks 证据 → 本地全量结果

npm run typecheck → PASS (所有 workspace)
npm run -w @maka/ui test → tests 13, pass 13, fail 0
npm run -w @maka/desktop test→ tests 1608, pass 1608, fail 0

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Astro-Han