Uh oh!
There was an error while loading. Please reload this page.
feat(ui): tab component spec — maka-tab + variants, migrate 4 tab surfaces (#499 P0-3) - #517
Merged
Merged
Conversation
…slice 1) Introduce the shared tab spec: maka-tab class + underline/pill variants on the Base UI Tabs primitive, active/hover repointed to --state-selected-bg / --state-hover-bg (no brand token, no per-surface hand-written tab CSS). underline keeps the Base UI indicator recolored to neutral --foreground (was brand --control); pill hides the indicator and fills the active tab with --state-selected-bg + bold. Migrate plan tabs as the first consumer: TabsList passes variant= "underline", TabsTrigger carries maka-tab (layout stays on maka-plan-tab). Removes the dead .maka-plan-tab[data-state="active"] under-bar (Base UI sets data-active, not data-state, so it never matched — plan's active visual was actually the Base UI default sliding plate, not the hand-written under-bar). Tab spec contract test (tab-spec-499-contract.test.ts) locks: primitive exposes maka-tab + underline|pill; .maka-tab uses neutral state tokens; plan consumes maka-tab + underline with no hand-written active CSS.
ui.tsx carried a second hand-rolled tab set (Base UI + bg-muted plate, no variant, dead data-[selected] active selectors — Base UI sets data-active) that plan-reminder-panel consumed, bypassing the tab spec primitive. Re-export TabsRoot/TabsList/TabsTrigger/TabsPanel from primitives/tabs so every tab surface gets the maka-tab class + underline/pill variants + the correct data-active attribute from one primitive. Removes the BaseTabs import. This is the #477 duplicate-component debt that #499 P0-3 tab governance has to collapse: one tab primitive, not two.
…3 slice 2)
ProvidersPanel catalog tabs move to the shared spec: TabsList passes
variant="pill", TabsTrigger carries maka-tab (catalogTab hand-written class
dropped), content renders through PrimitiveTabsPanel (one per category:
oauth -> ModelOAuthSection, domestic/overseas/local -> provider grid filtered
by category) instead of a bare conditional render. The hand-written
data-active={catalogTab === tab.id} boolean is removed (Base UI sets
data-active); data-catalog-tab={tab.id} stays (locked by model-oauth contract
as the tab identifier).
CSS cleanup: drop .catalogPillTabs button base/hover/active and the
[data-slot=tab-indicator] display:none (models.css), and .catalogTab:hover /
[data-active] (provider-editor.css). The pill base shape (radius/border/
bg/padding) moves to .maka-tab[data-variant=pill] in maka-tokens.css; active
repoints from brand --nav-active to neutral --state-selected-bg + a neutral
border. .catalogTabs/.catalogPillTabs strong stay as the label font hook.
model-oauth-section-contract.test.ts: update the catalog tab assertions from
the old catalogTab/catalogTabs catalogPillTabs class lock to variant=pill +
maka-tab + TabsPanel (intent unchanged — use the primitive, keep
data-catalog-tab, no manual keyboard handler). tab-spec-499-contract.test.ts
adds the slice 2 contract: catalog consumes maka-tab + pill + TabsPanel, no
hand-written catalogTab/catalogPillTabs active/hover/indicator CSS.…nderline + TabsPanel (#499 P0-3 slice 3) skills-panel tabs move from a hand-rolled segmented switcher (UiButton + aria-pressed + data-state, with a comment explicitly disowning the ARIA tabs contract) to the shared Base UI Tabs primitive: TabsList passes variant="underline", TabsTrigger carries maka-tab, and content renders through TabsPanel (one per market/builtin/installed). This upgrades the surface from a segmented switcher to real tabs with the roving-tabindex + arrow-key contract + linked tabpanels. The dispersed conditional render ({activeSkillTab === 'market' ? market : skillList(...)} + {activeSkillTab !== 'market' && templates}) concentrates into three TabsPanels. skillList gains a label param (replacing the internal activeSkillTab === 'installed' ? ... : ... label switch) so each panel renders its own label without depending on the active tab state — safe under the default keepMounted=false (only the active panel mounts) and robust if keepMounted is ever flipped. The empty-title/empty-body strings hoist to component scope (skillListEmptyTitle/skillListEmptyBody) so both the empty-skills and populated returns share them. The tabs-bar keeps its right-side filter actions (static labels, market-only) as a sibling of TabsList inside TabsRoot — they are not a tab/panel. CSS: drop .maka-skill-tab[data-state="active"] and its ::after under-bar (skills.css). The active state + under-bar move to .maka-tab[data-variant= underline] via the Base UI indicator (neutral --foreground). skills.test.ts: update the skillList section assertion from the literal aria-label="已安装技能" to the dynamic aria-label={label} (intent unchanged — the section still carries an aria-label, now driven by the label param). tab-spec-499-contract.test.ts adds the slice 3 contract: skill uses maka-tab + underline + TabsPanel, no hand-written aria-pressed/data-state switcher, no hand-written .maka-skill-tab[data-state="active"] / ::after CSS.
…tive (#499 P0-3 slice 4) daily-review range (今日/本周/本月) switches a time-window parameter — the report re-fetches for the chosen range, it is not three distinct views — so it is a segmented control, not tabs. Move it from a hand-rolled segmented switcher (UiButton + aria-pressed + data-active) to the project's SettingsSegmented primitive (Base UI ToggleGroup, single-select, roving tabindex + arrow keys + data-pressed). This is the a11y-correct primitive for a parameter switcher (radiogroup/pressed semantics), unlike Tabs/TabsPanel which expect linked tabpanel views — the other three surfaces (plan/catalog/skill) are view switches and stay on Tabs. Active repoints from brand --nav-active (0.12 bg + color) to the neutral .settingsSegmented button[data-pressed] chrome (white fill + shadow), inherited via the primitive's global CSS. The hand-written .maka-daily-review-range-tab base + [data-active="true"] brand block (daily-review.css) and the PR-DAILY-REVIEW-RANGE-0 brand-accent comment are removed; .maka-daily-review- range-tabs keeps its flex/gap layout (now the SettingsSegmented className). daily-review-copy-feedback-contract.test.ts: update the range-tab assertion from the hand-rolled UiButton maka-daily-review-range-tab to SettingsSegmented + maka-daily-review-range-tabs, and flip the brand-active CSS assertion from assert.match to assert.doesNotMatch (the brand active CSS is gone). Intent unchanged — Daily Review controls use shared primitives, no legacy classes. tab-spec-499-contract.test.ts adds the slice 4 contract: range uses SettingsSegmented, no hand-written aria-pressed/data-active switcher, no .maka-daily-review-range-tab[data-active] brand CSS.
…13, contract updates (#499 P0-3 slice 5) Tab spec close-out: - state-token-governance-499-contract.test.ts: drop the 5 tab selectors (.maka-daily-review-range-tab / .catalogTab / .catalogPillTabs / .maka-skill-tab / .maka-plan-tab) from the selected/active brand-token allowlist. All four tab surfaces migrated in slices 1-4 now use neutral state tokens, so the "pending tab-spec" carve-out is gone; only the onboarding brand-emphasis allowlist (.maka-firstrun-step / .maka-onboarding-setup-steps) remains. - design-system.md §3.13 Tabs: document the spec — one primitive (primitives/tabs.tsx, re-exported from ui.tsx), two variants (underline = neutral Base UI indicator; pill = --state-selected-bg fill + bold), no dedicated tab color token (reuse --state-hover-bg / --state-selected-bg), and the segmented-control rule (parameter switches like daily-review range use SettingsSegmented / Base UI ToggleGroup, not Tabs). - radius-converge-contract.test.ts: remove the stale TabsTrigger/TabsList entries (they used to be declared in ui.tsx with --radius-* tokens; #499 P0-3 re-exports them from primitives/tabs.tsx, which uses Tailwind rounded-md/ rounded-sm governed by primitives-design-contract escape hatches, not the radius-token convergence contract). - design-system-governance-406-contract.test.ts: the tab underline indicator moves from brand --control to neutral --foreground (structural chrome stays calm, brand is garnish — the principle already documented in plan-reminders.css). Update the assertion from bg-control to bg-foreground and add a ban on bg-control for the under-bar (checkbox/progress/menu keep bg-control — those are checked controls, not structural chrome). - claude-subscription-experimental-gate.test.ts: the OAuth catalog content moved from a bare conditional render (catalogTab === 'oauth' ? <ModelOAuthSection/>) into a TabsPanel, matching the model-oauth-section contract update in slice 2. Intent unchanged — the OAuth tab renders the real login cards. Verified: npm run -w @maka/desktop test 1878 pass / 0 fail; typecheck clean.
Astro-Hanforce-pushed
the
feat/499-tab-component-spec
branch
from
July 4, 2026 16:01
3bfaf67 to
d08300aCompare…l return (#499 P0-3 review P3-2/4) Address review P3-2 + P3-4 on PR #517 (tab component spec). P3-2 — redundant maka-tab class on tab triggers. The TabsTab primitive already emits `maka-tab` (tabs.tsx), so callers passing `className="maka-tab ..."` duplicate it. Drop `maka-tab` from the three call sites, keeping local layout classes: - skills-panel.tsx: "maka-tab maka-skill-tab" → "maka-skill-tab" - plan-reminder-panel.tsx: "maka-tab maka-plan-tab" → "maka-plan-tab" (×2) - ProvidersPanel.tsx: drop the bare "maka-tab" (catalog has no local class) Sync the contract tests that locked "caller must pass maka-tab" — that constraint is redundant now the primitive guarantees the class: - tab-spec-499-contract.test.ts: drop the three per-surface assert.match(TabsTrigger className maka-tab). Keep variant, TabsPanel, and the doesNotMatch anti-regression locks. - model-oauth-section-contract.test.ts: catalog tablist contract keeps PrimitiveTabsTrigger + value={tab.id}, drops className="maka-tab" (maka-tab comes from the primitive). P3-4 — skills-panel double return. The empty-skills and non-empty returns duplicated the entire TabsRoot/tabs/TabsPanel tree, differing only by `skillList([] …)` vs `skillList(filteredSkills …)` and a trailing tool-summary span. Merge to one return: filteredSkills is `[]` when props.skills is empty (it's `(props.skills ?? []).filter(…)`), so `skillList(filteredSkills, …)` is equivalent in both states; the summary span becomes conditional. Cuts ~15 lines and removes a future drift hazard (changing the tabs tree used to require editing two returns). Not done (P3-3 / P3-5): the rest of tab-spec-499-contract is not redundant lock-on-implementation. `variant="underline|pill"` locks each surface's form spec (a result). `TabsPanel` is the only a11y guard (tabpanel role + roving tabindex) until the project has render-test infrastructure. The doesNotMatch locks prevent hand-written tab CSS/switcher regression. Only the className maka-tab lock was true redundant lock-on-implementation; the rest stays. Verified: npm run -w @maka/desktop test 1888 pass / 0 fail; typecheck clean.
…view P3) Address review round 2 P3 (PR #517): the `assert.match(panel, /TabsPanel/)` locks were too wide — they matched the import statement (`TabsPanel } from './ui.js'`) and any TabsPanel, not a panel for each tab value. Tighten to per-value locks so each tab value has a corresponding panel (tabpanel a11y pairing), and deleting any panel fails the test: - plan: TabsPanel value="tasks" + value="runs" (new — plan had no TabsPanel lock before) - catalog: PrimitiveTabsPanel value="oauth" (fixed OAuth panel) + value={cat} (mapped category panel template) + ['domestic','overseas','local'] (the mapped array — proves every category has a panel without a runtime render test) - skill: TabsPanel value="market" + value="builtin" + value="installed" Unrelated TabsPanel occurrences (e.g. import lines) no longer satisfy the lock: deleting any target surface panel fails the test, the import line alone does not pass it. Verified: npm run -w @maka/desktop test 1888 pass / 0 fail; typecheck clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Tab component governance for #499 P0-3: one tab spec primitive (
maka-tabclass +underline/pillvariants on Base UI Tabs), migrate the 4 hand-written tab surfaces, retire per-surface hand-written tab CSS, and collapse the duplicate tab sets (#477 debt). Tab active/hover repoint to the neutral--state-*tokens from #503 — no dedicated tab color token.Why
#499 P0-3 — tabs had no spec; 4 hand-written implementations drifted: plan/skill underline with dead
[data-state]selectors (Base UI setsdata-active), catalog brand pill hiding the Base UI indicator, daily-review range hand-rolled<button aria-pressed>. Pluspackages/ui/src/ui.tsxcarried a second hand-rolled tab set (Base UI + bg-muted plate, no variant, deaddata-[selected]) that plan-reminder-panel consumed, bypassing the existingprimitives/tabs.tsx. Direction: structural chrome stays neutral (brand is garnish), one primitive, two intentional variants — and a segmented control for parameter switches (not Tabs).Refs #499
Scope
Changed:
primitives/tabs.tsx:TabsVariantadds"pill";TabsListsetsdata-variant+maka-tabs-list; pill hides the Base UI indicator; underline indicator recolored from brandbg-controlto neutralbg-foreground.TabsTabemits the sharedmaka-tabclass.ui.tsx:TabsRoot/TabsList/TabsTrigger/TabsPanelre-export fromprimitives/tabs.tsx(drops the second hand-rolled set + deaddata-[selected]active selectors). One tab primitive, not two (refactor(ui): React architecture debt cleanup (post #430 converge) #477 debt).maka-tokens.css:.maka-tabspec — pill base shape +--state-selected-bgactive +--state-hover-bghover + neutral border; underline active =--foregroundtext. Reuses refactor(ui): retire --hover/--active for state tokens, unify selected neutral, drop press scale #503 state tokens; no dedicated tab color token.plan-reminder-panel.tsx:variant="underline"+maka-tab; drops the dead.maka-plan-tab[data-state="active"]::afterunder-bar (Base UI setsdata-active, notdata-state— plan's active visual was actually the Base UI default sliding plate, not the hand-written under-bar).ProvidersPanel.tsx: catalog →variant="pill"+maka-tab+TabsPanel(oauth → ModelOAuthSection; domestic/overseas/local → filtered provider grid). Dropsdata-active={catalogTab === tab.id}(Base UI sets it) and thecatalogTabhand-written class; keepsdata-catalog-tab. Content moves from bare conditional render into panels.skills-panel.tsx: hand-rolled segmented switcher (UiButton+aria-pressed+data-state) →Tabs/TabsList/TabsTrigger/TabsPanel(market/builtin/installed each a panel). a11y upgrade: roving tabindex + arrow-key contract + linked tabpanels.skillListgains alabelparam (no longer depends onactiveSkillTab— safe under defaultkeepMounted=false).daily-review-panel.tsx: range (今日/本周/本月) →SettingsSegmented(Base UI ToggleGroup). It switches a time-window parameter, not views — segmented control is the a11y-correct primitive, not Tabs/TabsPanel. Active inherits the neutral.settingsSegmented button[data-pressed]chrome..catalogPillTabs buttonbase/hover/active +[data-slot=tab-indicator] display:none(models.css),.catalogTab:hover/[data-active](provider-editor.css),.maka-skill-tab[data-state=active]+::after(skills.css),.maka-daily-review-range-tab[data-active]brand (daily-review.css),.maka-plan-tab[data-state=active]+::after(plan-reminders.css).state-token-governance-499-contract.test.ts: drop the 5 tab selectors from the selected/active brand-token allowlist (tabs no longer need the carve-out). Onboarding brand-emphasis allowlist stays.design-system.md §3.13 Tabs: documents the spec (primitive, variants, no dedicated color token, segmented-control rule).tab-spec-499-contract.test.ts(new, per-surface),model-oauth-section-contract+claude-subscription-experimental-gate(catalog conditional render → TabsPanel),skills.test(skillList aria-label dynamic),daily-review-copy-feedback-contract(range → SettingsSegmented),radius-converge-contract(drop stale TabsTrigger/TabsList ui.tsx entries),design-system-governance-406-contract(under-barbg-control→bg-foreground).Not included:
Verification
npm run -w @maka/desktop test: 1888 pass / 0 fail (rebased onto main85c81d9d).npm run typecheck: clean (main + renderer + storybook).npm run build):provider-workspace(catalog pill),module-skills(skill tabs),module-daily-review(daily-review range),plan-reminders(plan underline). Output:apps/desktop/tests/screenshots/<scenario>/<variant>.png. Conclusion: catalog pill active = neutral wash, skill/plan underline = neutral bar, daily-review range = neutral SettingsSegmented chrome — correct in both light and dark.tab-spec-499-contractverifies.maka-tabactive/hover use--state-selected-bg/--state-hover-bg(neutral) with no brand token;design-system-governance-406verifies the under-bar isbg-foreground; keyboard a11y (roving tabindex + arrow keys + tabpanel pairing) is guaranteed by Base UI Tabs (plan/catalog/skill) and Base UI ToggleGroup (daily-review range).capture-screenshots.mjsvariants are theme × viewport × motion; it does not capture hover/active/keyboard state variants. Hover/active state correctness is covered by the token/static contract tests above (not by screenshots); keyboard behavior is covered by the Base UI primitive contract. A CDP-driven state-variant harness is a separate piece of work.dist/renderer/settings/*.jsby path, butbuild:rendereris vite (chunks, not per-path). A fresh worktree lacks these by-path files; this PR was verified withtsc -p tsconfig.renderer.json --outDir dist/rendererto emit per-path dist. This is a test/build setup mismatch — CI may need the same workaround, or the test script should build renderer per-path.User-facing impact
Reviewer notes
ui.tsxtwo-sets-of-tabs unification is the structural fix that makes slice 1'svariant="underline"actually typecheck (plan importedTabsListfromui.js, which was the hand-rolled set without variant). It's the refactor(ui): React architecture debt cleanup (post #430 converge) #477 duplicate-component debt P0-3 had to collapse.maka-tabclass on tab triggers (primitive already emits it); merged the skills-panel empty/non-empty duplicate return; tightened theTabsPanelcontract locks from/TabsPanel/(matched imports) to per-value (TabsPanel value="tasks",value="oauth"+value={cat}+['domestic','overseas','local'],value="market|builtin|installed"). See Verification for the visual verification record.