ADR-012: UI/UX system - #309
Merged
Merged
Conversation
…ift/contrast test Both token files (gl-vars-dev.css, gl-theme.css) still carried "GENERATED - do not hand-edit" headers pointing at graphlink_styles.py and a Qt island- injection mechanism, both deleted in the Qt-removal cutover - there has been no generator and no other consumer for a while, so the files were already being hand-edited against their own header's claim. Made that honest, and used the now-genuine hand-authored status to add what was actually missing: a real light palette alongside dark. The light values are a systematic per-token derivation, not free-picked: every solid color in this app is a pure gray (a deliberate monochrome "graphite" design), so light is a per-channel inversion of dark, with a distinct rule for alpha-wash tokens (highlight/border washes flip polarity, drop shadows keep black but reduce opacity, neutral grays keep their RGB and bump alpha) - spot-checked against real WCAG relative-luminance contrast, not raw RGB distance, catching one token (text-muted) that needed a manual correction to match its own dark-mode counterpart's contrast ratio. Switching is data-theme + prefers-color-scheme, per the ADR; base.css's color-scheme property now follows the same cascade so native form controls (scrollbars, etc.) track the active theme too. The deleted Qt-era Python drift test (test_gl_vars_dev_css.py, which checked against the generator) is replaced by a real frontend one, token-theme-contrast.test.ts: name parity between the two themes, the two duplicated light blocks (media query + explicit attribute) staying identical, and WCAG AA contrast for every surface/text pair that carries real body copy. Verified visually via computed styles in a live browser session: the base (no data-theme set) cascade renders byte-identical to before this stage - theme *application* (stamping data-theme from a real Settings field) is stage 12.2's job, not this one's. Co-Authored-By: Claude <noreply@anthropic.com>
Wires a new setTheme (system|light|dark) app-settings intent end to end: SettingsManager persistence, wire contract, WS intent + undo classification, a Theme control in Settings' General page, and an App.tsx effect that stamps/clears [data-theme] on <html> so stage 12.1's light/dark token cascade actually applies. Verified live in-browser: toggling the control re-themes the whole app immediately, no reload, for all three states. Co-Authored-By: Claude <noreply@anthropic.com>
…nk, aria-live Full WAI-ARIA menu keyboard pattern lives once in NodeMenu.tsx (initial focus, Arrow/Home/End roving, Escape/activate-restores-focus-to-trigger), so all 11 node kinds with a context menu get it for free. Shift+F10/the ContextMenu key opens a node's menu by dispatching a real contextmenu event at the focused node - the actually-focused element is React Flow's own node wrapper, an ancestor of .scene-node, so no per-node-view change was needed. The 5 kinds with no context menu (Chart/Group/Html/Note/Plan) already expose their actions as plain buttons and needed nothing. A visible :focus-visible outline (new --gl-focus-ring token, both themes) is now distinct from .selected's own box-shadow treatment. A skip-link jumps a keyboard user straight to the composer, bypassing every node on the canvas. One aria-live region (announcer.ts) announces assistant streaming start/finish and per-node run status transitions, fed by composerStore.ts and sceneStore.ts. Added eslint-plugin-jsx-a11y to the lint gate and fixed the 12 pre-existing violations it surfaced across the app (real fixes where possible - alt text, real keyboard handlers, aria-selected; scoped, justified suppressions where the existing pattern was already correct, e.g. CommandPalette's input-driven virtual selection). Co-Authored-By: Claude <noreply@anthropic.com>
…rt pans One blanket @media (prefers-reduced-motion: reduce) rule in base.css covers styles.css's ~17 CSS transitions (and any future one, and the animations this app doesn't have yet) - a near-zero duration rather than `transition: none` so transitionend listeners still fire. React Flow's own zoom/pan animations (fitView/zoomIn/zoomOut/setCenter/setViewport) aren't CSS and can't be reached from there; each of their 9 call sites now runs its requested duration through a new motionDuration() helper that collapses to 0 when the OS preference is set. jsdom implements no window.matchMedia at all - added the standard stub to vitest.setup.ts (same posture as the existing ResizeObserver/scrollIntoView stubs) so tests exercising any of these call sites don't crash. Co-Authored-By: Claude <noreply@anthropic.com>
…node filter Adds NodeShell.tsx, the wrapper every content-card node kind (all 16 but GroupNodeView) now renders through instead of hand-rolling its own .scene-node div, Handle pair, and collapse gate - migrated all 15 applicable kinds (CodeNodeView as the reference, then Artifact/Thinking/ PyCoder/WebResearch/Image/Document/Gitlink/CodeSandbox/Chart/Html/Note/ Plan/Chat/Conversation via a verified Workflow pass plus Chart/Note by hand once NodeShell needed a style/resizer/onBodyDoubleClick escape hatch for their outlier needs). Adds canvas search-match highlighting: CanvasSearchContext threads SearchOverlay's live query into NodeMarkdown, reusing the existing Document View highlight plugin so every visible node's body highlights matches as you type. Adds a node filter-by-kind/status control in ViewPopover's own FILTER section, backed by two new sceneStore fields (filterKinds/filterStatuses) composed into toFlowNodes' existing dimming mechanism - required wiring isDimmed into 10 node-kind branches that never carried it (only the 5 branch-focus kinds did), since the filter needed to dim every content kind, not just those five. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…aces Adds a first-run onboarding wizard (OnboardingDialog.tsx) that auto-opens once on a fresh machine, checks real provider readiness via the composer's existing route.available signal, offers a small bundled sample workspace (backend/api/intents_onboarding.py's loadSampleWorkspace - a fixed, deterministic 3-node fixture, no LLM call, reused by both the wizard and the canvas's own new empty-state hint), and gets out of the way. Persisted via a new hasCompletedOnboarding setting, reopenable later from Settings. Adds a non-blank empty-canvas hint in SceneCanvas.tsx, offering the same sample-workspace load when the scene has zero nodes. Adds 3 Settings surfaces this ADR's own stage table called for: - A working provider-mode switcher (ProviderModeSwitch), wiring the frontend to the setProviderMode intent that already existed but nothing called - placed on each of the 3 per-mode pages rather than reviving the cramped toolbar control removed earlier this ADR. Corrects that removal's own now-stale doc comment in AppBar.tsx. - An MCP Servers page (list/add/remove/enable), plus the setMcpServers intent and wire-contract field ADR-007's own docstring had explicitly deferred to this stage - MCP server config was previously edit-the-JSON- by-hand only. - A read-only Resource Limits section surfacing the same backend-computed disclosure CodeExecutionApprovalPanel.tsx already shows mid-approval, now reachable ahead of time. Found and fixed a real bug in the process: ExecutionLimitsProvider only wrapped SceneCanvas, not the Settings dialog sibling, so the new section would have silently read blank text. The DPAPI/secret-visibility badge (ADR-004) was already fully shipped; confirmed via recon, no new work needed for it in this stage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
eslint-plugin-jsx-a11y's published peer range still tops out at ESLint ^9 (no ^10 support yet upstream, confirmed against the latest published version), so a plain `npm ci` - what CI actually runs, no --legacy-peer- deps - hits an ERESOLVE conflict against this repo's eslint@^10.8.0 and fails before any check ever runs. Locally this was masked by installing with --legacy-peer-deps, which was never carried into the CI workflow. Fixes it with a targeted `overrides` entry pinning just this one peer resolution to the project's actual eslint version, rather than relaxing peer-dependency enforcement for the whole install. Regenerated package-lock.json from a clean install; `npm ci` now succeeds and the full suite (1843 frontend tests, tsc, eslint) passes unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
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.
Problem
Several long-standing UI/UX gaps: no real light/dark theming, incomplete keyboard reachability on the canvas (node menus, focus indication), no
prefers-reduced-motionsupport, every node kind hand-rolling the same wrapper boilerplate, no in-canvas search highlighting or node filtering, and no first-run path for a fresh install (no onboarding, no sample content, a blank canvas with no explanation). Three Settings surfaces were also owed by earlier ADRs: a working provider-mode switcher (the intent existed, nothing called it), an MCP servers panel (ADR-007's own "Consequences" section deferred this here), and reachable resource-limit disclosure (previously only shown mid code-execution-approval).Change
Six stages, one branch:
aria-liveregion announcing streaming/run-status transitions. Also broughteslint-plugin-jsx-a11yin as a green gate, fixing every violation it surfaced.prefers-reduced-motionrespected across CSS transitions and React Flow's own imperative viewport pans/zooms.NodeShell.tsx, a shared wrapper now used by 15 of 16 node kinds (the wrapper div, both handles, the context-menu wiring, and the collapse gate, previously duplicated in every*NodeView.tsx); canvas search-match highlighting (the same highlight plugin Document View uses, now live on every node card as you type in the search overlay); and a node filter-by-kind/status control in the View popover.Test plan
python -m pytest -q— 2418 passed, 17 skipped, 5 failed (all intest_native_dialogs.py, a pre-existing/environmentalpywebviewissue unrelated to this branch — confirmed via empty diff on that file and its test).npx vitest run— 1843 passed across 77 files.npx tsc --noEmit -p .— clean.npx eslint .— 0 errors (43 pre-existing warnings, none introduced).python contracts/codegen.py --check— generated wire-contract artifacts in sync.pytest tests/test_undo_classification_gate.py— every new intent classified.npm run build+ bundle-size gate — clean.