Skip to content

refactor(gui): converge the visual system onto AppKit's metrics - #601

Open
AprilNEA wants to merge 4 commits into
masterfrom
refactor/gui-visual-system
Open

refactor(gui): converge the visual system onto AppKit's metrics#601
AprilNEA wants to merge 4 commits into
masterfrom
refactor/gui-visual-system

Conversation

@AprilNEA

Copy link
Copy Markdown
Owner

Summary

The GUI's design tokens described a system the values never actually delivered:
one token (surface_hover) covered three unrelated jobs, hover states had
drifted into four dialects, ~20 hand-painted controls were reachable by mouse
only, and the type ramp ran a rung larger and heavier than AppKit's, which is
most of what "the UI looks rough" meant — it read as a web page rather than a
Mac app.

Three commits, separable and independently revertible: converge the colour
layer and back the window with a native material; take the type ramp and chrome
down to AppKit's own metrics; then a handful of fixes that fell out of
measuring the result.

Two real defects surfaced while doing it, both pre-existing on master:

  • A stale agent was reported as unreachable. The handshake already knew which
    side was old (agent=13 gui=16 was in the log), but an older agent was folded
    into ConnectFailure::Unreachable, so the window advised "try reinstalling
    the app" while the socket was answering fine and the actual fix was to let the
    new agent take over.
  • moon.svg was on disk but absent from ACTION_ICONS, so Action::Sleep's
    icon silently failed to load in the binding picker — AppAssets::load only
    consults that table.

Changes

crates/openlogi-gui — colour tokens

  • Replace surface_hover with one neutral interaction wash at two strengths,
    wash / wash_strong. Both are alpha tints of the foreground, so they
    composite correctly over any surface instead of matching exactly one.
  • Add text_ghost (decorative marks) and border_strong (an emphasised edge).
    Call sites were reaching for text_muted — a text weight — in their place.
  • WashStyle::hover_wash puts the hover decision in one place, alongside the
    existing SelectableStyle.
  • Leader lines were a hardcoded grey authored for the dark theme and nearly
    invisible on light; they now resolve the palette at paint time, so an
    appearance flip repaints them.

crates/openlogi-gui — macOS window material

  • The main window opts into WindowBackgroundAppearance::Blurred, and
    platform::os::configure_window_material retargets the NSVisualEffectView
    gpui installs beneath its Metal layer at WindowBackground.
  • gpui's BlurredView strips the material's desktop tinting and saturation, so
    the colour comes from the GPUI side: Palette::backdrop is the theme
    background at BACKDROP_ALPHA (0.8) — a single translucent layer, with
    everything above it opaque so no two translucent surfaces stack.
  • New macOS-only dependency raw-window-handle (target-gated) to reach gpui's
    NSView. It is the version already in Cargo.lock as gpui's own dependency,
    so the resolve adds an edge, not a crate; both git pins are unchanged.

crates/openlogi-gui — control affordances

  • ControlStyle::control gives a hand-painted div a native cursor, a tab stop
    and a focus ring; press_wash is the pressed state. Applied to the ~20
    clickable surfaces that had none of it, and cursor_pointer (a web idiom) is
    gone from the crate.
  • Activation needed no wiring: gpui already maps enter/space to a focused
    element's click listeners and keeps the focus handle in element state under
    the element's id.
  • The focus ring is drawn as an outer shadow rather than a border, which would
    resize the element on focus and reflow tight rows.
  • Fixes en route: the mouse and keyboard key chips hovered darker than their
    resting state, and a delete affordance hovered to gpui::white(), vanishing
    on a light theme.

crates/openlogi-gui — type ramp and chrome

  • The ramp now is AppKit's text styles — title1 22, title2 17, headline/body
    13, subheadline 11 — and weight stops at SEMIBOLD, leaving hierarchy to the
    colour ramp. Leading ratios are deliberately unchanged: native chrome is small
    text with roomy leading, so density comes from the size, not from crowding.
  • Header 80 → 56, footer 50 → 34, and one notch off each spacing token.
  • Both *_VERTICAL_RESERVE constants are now derived from those values instead
    of a 224px literal, so the reclaimed height goes to the device model rather
    than staying budgeted for chrome that shrank. The window's minimum height
    follows the same arithmetic down to 640, and the pointer grid keeps its exact
    two-card fit at the 720px minimum by taking the 8px the narrower inset frees.
  • Card radius × 1.5× 2: at the theme's 6px control radius that moves
    cards from 9 to 12, far enough from the controls inside them that the nesting
    reads.

crates/openlogi-gui — agent-link state, assets, motion

  • AgentLink::OutdatedAgent mirrors the OutdatedGui state that already
    existed, with its own frame and the same retry cadence. No wire change: the
    version comparison was already there, only its reporting was lossy.
  • Register moon.svg, and add the test that would have caught it — the
    action-icons/ directory and ACTION_ICONS must agree. include_bytes!
    already covers the opposite direction and ActionRingIcon::ALL covers the
    enum-driven icons; a file nobody registered was the uncovered case.
  • set_reduce_motion was never called, so gpui's flag sat at its default and
    animations ran regardless of the system preference. Wired to NSWorkspace on
    macOS and GSettings on Linux, before the first window opens.
  • Tooltips for the two controls that had no accessible name at all: the
    carousel's prev/next chevrons and the DPI preset's remove button. The other
    icon buttons already carry visible labels.

i18n

Five new keys across all 20 locales (403 each, order-identical). Non-English
values are best-effort pending Crowdin, per the locale rules.

crates/openlogi-gui/src/platform/AGENTS.md

Bring the FFI inventory back in sync with the tree it documents: add os.rs and
overlay.rs, and correct the two tray paths that moved to crates/openlogi-agent.

Testing

cargo fmt --all -- --check          # clean
cargo clippy --workspace --all-targets -- -D warnings   # clean
cargo test --workspace              # 25 test binaries, no failures
cargo test -p openlogi-gui i18n     # locale parity, 403 keys

Run on the rebased tip. One caveat worth recording: the first post-rebase run
showed three xtask bundle-plist failures that turned out to be a stale test
binary cached from a since-deleted sibling worktree (its CARGO_MANIFEST_DIR
was baked in); cargo clean -p xtask and they pass 20/20. Nothing in this
branch was involved.

Exercised in the running dev app against the mock agent (openlogi-agent-mock):
window opens, connects, polls, and quits cleanly with no panics or unexpected
warnings. Not runtime-tested on hardware — no physical device was attached,
and every device shown came from the mock.

Two things this branch changes that were not visually verified by the author
and are worth a look during review:

  • the resulting Tab order, now that ~20 hand-painted controls joined the cycle
    alongside the framework buttons; and
  • the AgentLink::OutdatedAgent frame, which needs a deliberately stale agent
    to trigger.

Fixes #567

…acOS material

Three related pieces of the visual system, restored onto master from the
mock-agent working tree.

Colour tokens: `surface_hover` served three unrelated jobs (hover, armed
resting state, recessed well), so hover states had drifted into four
dialects. Replace it with one neutral interaction wash at two strengths —
`wash` / `wash_strong`, alpha tints of the foreground so they composite
correctly over any surface instead of matching exactly one. Add
`text_ghost` for decorative marks and `border_strong` for an emphasised
edge; both had call sites reaching for `text_muted`, a text weight, in
their place. `WashStyle::hover_wash` puts the hover decision in one place.

macOS material: the main window opts into `WindowBackgroundAppearance::
Blurred`, and `platform::os::configure_window_material` retargets the
`NSVisualEffectView` gpui installs beneath its Metal layer at
`WindowBackground`. gpui's `BlurredView` strips the material's desktop
tinting and saturation, so the colour comes from the GPUI side instead:
`Palette::backdrop` is the theme background at `BACKDROP_ALPHA`, a single
translucent layer with everything above it opaque.

Control affordances: ~20 hand-painted clickable surfaces had no press
feedback, no focus ring and no tab stop — reachable by mouse only.
`ControlStyle::control` adds a native cursor, a tab stop and a focus ring
drawn as an outer shadow (a border would resize the element on focus and
reflow tight rows). Activation needs no wiring: gpui already maps enter /
space to a focused element's click listeners, and keeps the focus handle
in element state under the element's id.

Fixes en route: leader lines were a hardcoded grey authored for the dark
theme and invisible-ish on light; the mouse and keyboard key chips
hovered *darker* than their resting state; a delete affordance hovered to
`gpui::white()`, vanishing on a light theme.
The colour tokens, the window material and the control affordances all
landed without changing what the app looks like at rest, because none of
them touched the values that actually carry the look. This does.

Type: the ramp now *is* AppKit's text styles — title1 22, title2 17,
headline / body 13, subheadline 11 — where it previously ran a rung
larger and topped out at Bold. Leading ratios are untouched on purpose:
native chrome is small text with roomy leading, so density comes from the
size, not from crowding lines. Weight stops at SEMIBOLD, leaving
hierarchy to the colour ramp.

Chrome: an 80 px header read as a page masthead. 56 px header, 34 px
footer, and one notch off each spacing token. Both `*_VERTICAL_RESERVE`
constants are now derived from those values rather than a 224 px literal,
so the reclaimed height goes to the device model instead of staying
budgeted for chrome that shrank; the window's minimum height follows the
same arithmetic down to 640. The pointer grid keeps its exact two-card
fit at the 720 px minimum by taking the 8 px the narrower inset frees.

Material: `BACKDROP_ALPHA` 0.9 → 0.8. At 0.9 the bleed was invisible
against a dark desktop — the whole effect wasted. 0.8 still keeps the
muted text ramp clear of its contrast floor over a bright wallpaper.

The keyboard render test asserted its floor at a 500 px viewport, which
is no longer short enough to reach it now that less height is reserved.
The assertion is unchanged; its input is derived from the constants that
decide the threshold, so it cannot rot the same way again.
…l what was unlabelled

Five small things, four of them found by measuring rather than guessing.

A stale agent reported as unreachable. The handshake already knows which
side is old — `agent=13 gui=16` was in the log the whole time — but an
older agent was folded into `ConnectFailure::Unreachable`, so the window
said "Can't reach the background service … try reinstalling the app"
while the socket was answering fine and the fix was to let the new agent
take over. `AgentLink::OutdatedAgent` is the mirror of the `OutdatedGui`
state that already existed, with its own frame and the same retry
cadence. No wire change: the version comparison was already there.

`moon.svg` was on disk but missing from `ACTION_ICONS`, so
`Action::Sleep`'s icon silently failed to load in the binding picker —
`load` only consults that table. Registered it, and added the test that
would have caught it: the directory and the registry must agree.
`include_bytes!` already covers the other direction, and
`ActionRingIcon::ALL` covers the enum-driven icons; a file nobody
registered was the uncovered case.

`set_reduce_motion` was never called, so gpui's flag sat at its default
and animations ran regardless of the system preference. Wired to
`NSWorkspace` on macOS and GSettings on Linux, before the first window.

Card radius `× 1.5` → `× 2`: at the theme's 6px control radius that moves
cards from 9 to 12, far enough from the controls inside them that the
nesting reads.

Tooltips for the two controls that had no name at all: the carousel's
prev/next chevrons and the DPI preset's remove button. The rest of the
icon buttons already carry visible labels — the gap was two controls, not
a category.
Copilot AI lite review requested due to automatic review settings August 14, 2026 03:04

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Aug 14, 2026

Copy link
Copy Markdown

Greptile Summary

The PR aligns the GUI’s visual system with AppKit metrics and improves native accessibility behavior.

  • Consolidates palette, typography, spacing, window-material, and control-affordance behavior
  • Adds explicit stale-agent status handling and associated localized messaging
  • Initializes the operating system’s reduced-motion preference before creating the first window
  • Registers the missing sleep icon and adds asset-table coverage

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
crates/openlogi-gui/src/ipc_client.rs Adds a distinct older-agent connection outcome and now prevents the generic unreachable notification from overwriting the specialized frame while the mismatch persists.
crates/openlogi-gui/src/platform/os.rs Reads the platform reduced-motion preference synchronously before window creation, resolving the previously reported Linux startup race.
crates/openlogi-gui/src/main.rs Initializes reduced-motion state before opening windows and maps the new IPC outcome into the dedicated agent-link state.
crates/openlogi-gui/src/state.rs Extends the connection-state model with an explicit outdated-agent state for accurate recovery guidance.
crates/openlogi-gui/src/theme.rs Revises visual tokens, typography, spacing, and reusable interaction styles to match the new AppKit-oriented system.

Reviews (2): Last reviewed commit: "fix(gui): keep a protocol-mismatch frame..." | Re-trigger Greptile

Comment thread crates/openlogi-gui/src/ipc_client.rs
Comment thread crates/openlogi-gui/src/platform/os.rs Outdated
…ble"

A mismatched protocol never sets `client`, so both mismatch states leave it
at `None` — and the generic unreachable notice only checked that. Fifteen
seconds after reporting the specialised frame, the fast phase elapsed and
`GuiUpdate::Unreachable` talked over it with "try reinstalling the app",
exactly the wrong advice while the socket is answering. Guard the notice on
neither mismatch being live, and clear both when the socket actually goes
away so a stale agent that then disappears still degrades to unreachable.

This was not new: `OutdatedGui` has had the same hole since it was added.
Fixing the mechanism fixes both directions.

Read the GNOME reduce-motion preference synchronously, like the macOS arm.
Resolving it in the background returned before `gsettings` did, so the first
window — and its first animation — could be built while the flag still held
its default, ignoring the preference for the moment it matters most. One
subprocess before any window exists is not on a frame path.

Cross-platform fallout from the same change: `ControlStyle` / `WashStyle`
are only used by macOS-gated controls in the footer and the permissions
page, so their imports are gated to match, and gpui's `App::update` returns
the closure's value rather than a `Result` — the `.ok()` was macOS-invisible
because that arm is `#[cfg]`-ed out there.
Sign up for free to 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.

Gallery cards are not keyboard-focusable — devices cannot be opened without a mouse

2 participants