UI/UX Audit: OKLCH tokens, AgentBuilder split, accessibility fixes#151
UI/UX Audit: OKLCH tokens, AgentBuilder split, accessibility fixes#151
Conversation
- Import tokens.css as single source of truth - Map --m-* vars to Tailwind --color-* via @theme block - Replace all hardcoded hex colors with var(--m-*) references - Add Firefox scrollbar support (scrollbar-width/scrollbar-color) - Scope theme transitions to .theme-transition class only - Use Geist (not Geist Sans) for font-sans via --m-font-sans - Keep focus-visible, sr-only, base reset, and React Flow overrides - Remove separate dark/light scrollbar and React Flow theme blocks (tokens.css handles theme switching via data-theme attribute)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e86c78fae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| {constraints.customConstraints.split(' | ||
| ').filter(Boolean).map((rule: string, i: number) => ( |
There was a problem hiding this comment.
Fix unterminated string in customConstraints split
This split call is written as split(' followed by a newline and then '), which creates an unterminated string literal in TSX. In practice the file cannot be parsed, so the frontend build/dev server fails before the Agent Builder can render.
Useful? React with 👍 / 👎.
| workflowTokens={workflowTokens} | ||
| totalUsed={totalUsed} | ||
| tokenBudget={tokenBudget} | ||
| factCount={facts.length} |
There was a problem hiding this comment.
Pass the correct factsCount prop to ContextBudgetCard
The card component expects a factsCount prop, but this call site passes factCount instead, so the “Based on N insights” line never appears even when facts exist. This also leaves factsCount undefined in the child and creates a type mismatch that will fail typed builds.
Useful? React with 👍 / 👎.
src/panels/RuntimePanel.tsx
Outdated
| const color = FACT_COLORS[fact.epistemicType] ?? 'var(--m-text-dim)'; | ||
| return ( | ||
| <span style={{ fontSize: 12, padding: '2px 6px', borderRadius: 4, background: color + '15', color, fontFamily: "'Geist Mono', monospace" }}> | ||
| <span style={{ fontSize: 12, padding: '2px 6px', borderRadius: 4, background: color + '15', color, fontFamily: "var(--m-font-mono), monospace" }}> |
There was a problem hiding this comment.
Use a valid alpha color for fact badge backgrounds
After switching FACT_COLORS to CSS variables, background: color + '15' now produces invalid CSS like var(--m-fact-observation)15. Browsers drop that declaration, so epistemic badges lose their tinted background state and become harder to scan.
Useful? React with 👍 / 👎.
…cat with CSS vars
UI/UX Audit Implementation
Full audit report: see session/2026-04-02_UI_UX_Audit_Modular.md
Design System
src/styles/tokens.css) replacing all hex valuesvar(--m-*)CSS custom propertiesdata-themeattribute on<html>AgentBuilder Split (968 lines → 9 focused files)
src/panels/builder/AgentActionBar.tsx— save/load/export/import barsrc/panels/builder/SectionHeader.tsx— reusable collapsible header + GenerateBtnsrc/panels/builder/IdentitySection.tsx— avatar, name, description, tagssrc/panels/builder/PersonaSection.tsx— persona, tone, expertisesrc/panels/builder/ConstraintsSection.tsx— safety profiles, custom rulessrc/panels/builder/ObjectivesSection.tsx— objectives, success criteriasrc/panels/builder/WorkflowCard.tsx— workflow steps timelinesrc/panels/builder/ContextBudgetCard.tsx— token budget breakdownsrc/panels/AgentBuilder.tsx— thin ~145-line orchestratorAccessibility
Color Consistency
var(--m-*)tokensvar(--m-fact-*)CSS
prefers-reduced-motionsupportscrollbar-width: thin).theme-transitionclass (no blanket*transition)