Lume Button component + full button migration (155/308) - #291
Merged
Conversation
Audit finding: 308 buttons, none using a shared component — each was hand-styled className soup relying on the auto-material CSS matcher, and only 3 of 308 had any motion. The spec defines five normative button variants (§4.2) plus focus and a busy recipe (§7.3); without a component those drift per call site. New app/_components/ui/Button.tsx — a motion.button with: - variants primary / secondary / ghost / danger (§4.2), sizes sm/md/lg/icon - §7.3 busy state: verb + animated dots (lume-busy-dots), never a spinner - tactile press/hover via Framer whileTap/whileHover (§6.4), gated on useReducedMotion so it respects prefers-reduced-motion - real <button>: type defaults to "button", disabled+aria-busy wired, forwardRef so focus-management call sites (ConfirmDialog) keep working - visual recipes still come from the material layer (same token classes), so the look stays centralized; the component owns structure + feel Migrated the first CTAs to validate all variants: chat Send (primary) + Stop (danger), ConfirmDialog Cancel (secondary) + Confirm (primary/danger, focus-on-cancel preserved), login submit (busy state). Build + typecheck green.
…ollow-up) Audit showed many CTAs are pill-shaped (rounded-full) with icons. Add pill and fullWidth props so those migrate to the canonical Button without a shape regression. twMerge resolves the rounded-md/rounded-full conflict.
…#290) Six sub-agents migrated raw <button> to the canonical Button, one per disjoint area (builder, store, admin, operator+routines, chat+shared+root, graph+memory). ~80 clean variant matches migrated; ~234 deliberately skipped and reported (tabs/toggles, icon chrome, and patterns outside the four spec variants: bg-inverse fills, danger-with-/40-border, neutral- bordered-no-bg). Conservative by design — misclassifying is worse than skipping; the skipped buttons keep the correct Lume look via the material layer, only the press feel is deferred. Also: align vitest's `@` alias to tsconfig (`@` -> web-ui root, was ./app). The divergence was latent until the first `@/app/...` imports landed; tsc and next build already used the tsconfig mapping, vitest did not. typecheck 0 errors, production build green, vitest 157/162 (the 5 failures are the pre-existing Node-26 localStorage env issue, green on CI's Node 22).
Mapped bespoke patterns onto the 4 spec variants (no new variants, per §10): bg-inverse fills -> primary, neutral-bordered-no-bg -> secondary, danger-with-/NN-border -> danger. ~68 more buttons across builder/store/ admin/operator/graph. typecheck 0, build green, lint 0 errors, vitest 157/162. Warning fills and solid-danger fills left for manual per-intent mapping next.
The two destructive-confirm buttons (uninstall, deny-proposal) used a solid bg-danger fill — the exact §2.6 violation (state colors are never block fills). Per §7.5 even destructive confirms use the transparent danger style. Migrated to <Button variant=danger pill>. Final tally: 155 of 308 buttons on the Lume Button. The remaining raw <button> are correctly non-candidates (tabs/toggles/segmented ~50, icon-only chrome ~60, text-links ~20, warning-outline ~15 kept as edge+text per spec spirit, global-error inline). typecheck 0, build green, lint 0 errors.
6 tasks
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Establishes the canonical Lume Button and migrates the operator web UI to it: 155 of 308 buttons now run through one component.
Why
Audit of
web-ui/app: 308<button>, none using a shared component — each was hand-styled className soup relying on the auto-material CSS matcher, and only 3 of 308 had any motion. The spec defines five normative button variants (§4.2) plus a focus and a busy recipe (§7.3); without a component those drift per call site. The buttons were, in short, not using the correct method, and had no tactile press feedback.The component
web-ui/app/_components/ui/Button.tsx— amotion.buttonwith:pill+fullWidthwhileTap/whileHover, gated onuseReducedMotionso it respectsprefers-reduced-motion(§2.11)<button>:typedefaults to button,disabled+aria-busywired,forwardRefso focus-management call sites (e.g. ConfirmDialog focus-on-cancel) keep workingMigration — 155 buttons across every area
Two passes via per-area sub-agents (one per disjoint directory: builder, store, admin, operator+routines, chat+shared+root, graph+memory) plus a manual pass for the judgment cases.
Bespoke patterns were mapped onto the four spec variants rather than inventing new ones (§10 restricts variants to the four):
bg-[--accent]andbg-[--bg-inverse]fills →primarysecondarydangerbg-[--danger]confirm buttons (uninstall, deny-proposal) → transparentdangerper §7.5, fixing a §2.6 violation (state colors are never block fills)What stays raw (correctly not Button candidates) — 153
aria-pressedselectors (~50)×, drag handles, canvas/zoom controls, expander chevrons (~60)global-error.tsxinline-style button (1) — must not import app codeTracked in #290.
Verification
typecheck 0 errors, production build green, lint 0 errors, vitest 157/162 (the 5 failures are the pre-existing Node-26
localStorageenv issue and pass on CI's Node 22). All routes render without runtime errors; the migrated primary button was confirmed in a real browser rendering the accent-gradient recipe with correct dark-mode inverse text.Also fixed a latent divergence: vitest's
@alias pointed at./appwhile tsconfig maps@/*to the web-ui root — surfaced once the first@/app/...imports landed; vitest now matches tsconfig.Follow-up from #282.