refactor(gui): converge the visual system onto AppKit's metrics - #601
Open
AprilNEA wants to merge 4 commits into
Open
refactor(gui): converge the visual system onto AppKit's metrics#601AprilNEA wants to merge 4 commits into
AprilNEA wants to merge 4 commits into
Conversation
…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.
Greptile SummaryThe PR aligns the GUI’s visual system with AppKit metrics and improves native accessibility behavior.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| 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
…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.
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.
Summary
The GUI's design tokens described a system the values never actually delivered:
one token (
surface_hover) covered three unrelated jobs, hover states haddrifted 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:
side was old (
agent=13 gui=16was in the log), but an older agent was foldedinto
ConnectFailure::Unreachable, so the window advised "try reinstallingthe app" while the socket was answering fine and the actual fix was to let the
new agent take over.
moon.svgwas on disk but absent fromACTION_ICONS, soAction::Sleep'sicon silently failed to load in the binding picker —
AppAssets::loadonlyconsults that table.
Changes
crates/openlogi-gui— colour tokenssurface_hoverwith one neutral interaction wash at two strengths,wash/wash_strong. Both are alpha tints of the foreground, so theycomposite correctly over any surface instead of matching exactly one.
text_ghost(decorative marks) andborder_strong(an emphasised edge).Call sites were reaching for
text_muted— a text weight — in their place.WashStyle::hover_washputs the hover decision in one place, alongside theexisting
SelectableStyle.invisible on light; they now resolve the palette at paint time, so an
appearance flip repaints them.
crates/openlogi-gui— macOS window materialWindowBackgroundAppearance::Blurred, andplatform::os::configure_window_materialretargets theNSVisualEffectViewgpui installs beneath its Metal layer at
WindowBackground.BlurredViewstrips the material's desktop tinting and saturation, sothe colour comes from the GPUI side:
Palette::backdropis the themebackground at
BACKDROP_ALPHA(0.8) — a single translucent layer, witheverything above it opaque so no two translucent surfaces stack.
raw-window-handle(target-gated) to reach gpui'sNSView. It is the version already inCargo.lockas gpui's own dependency,so the resolve adds an edge, not a crate; both git pins are unchanged.
crates/openlogi-gui— control affordancesControlStyle::controlgives a hand-painteddiva native cursor, a tab stopand a focus ring;
press_washis the pressed state. Applied to the ~20clickable surfaces that had none of it, and
cursor_pointer(a web idiom) isgone from the crate.
element's click listeners and keeps the focus handle in element state under
the element's id.
resize the element on focus and reflow tight rows.
resting state, and a delete affordance hovered to
gpui::white(), vanishingon a light theme.
crates/openlogi-gui— type ramp and chrome13, 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.
*_VERTICAL_RESERVEconstants are now derived from those values insteadof 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.
× 1.5→× 2: at the theme's 6px control radius that movescards from 9 to 12, far enough from the controls inside them that the nesting
reads.
crates/openlogi-gui— agent-link state, assets, motionAgentLink::OutdatedAgentmirrors theOutdatedGuistate that alreadyexisted, with its own frame and the same retry cadence. No wire change: the
version comparison was already there, only its reporting was lossy.
moon.svg, and add the test that would have caught it — theaction-icons/directory andACTION_ICONSmust agree.include_bytes!already covers the opposite direction and
ActionRingIcon::ALLcovers theenum-driven icons; a file nobody registered was the uncovered case.
set_reduce_motionwas never called, so gpui's flag sat at its default andanimations ran regardless of the system preference. Wired to
NSWorkspaceonmacOS and GSettings on Linux, before the first window opens.
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.mdBring the FFI inventory back in sync with the tree it documents: add
os.rsandoverlay.rs, and correct the two tray paths that moved tocrates/openlogi-agent.Testing
Run on the rebased tip. One caveat worth recording: the first post-rebase run
showed three
xtaskbundle-plist failures that turned out to be a stale testbinary cached from a since-deleted sibling worktree (its
CARGO_MANIFEST_DIRwas baked in);
cargo clean -p xtaskand they pass 20/20. Nothing in thisbranch 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:
alongside the framework buttons; and
AgentLink::OutdatedAgentframe, which needs a deliberately stale agentto trigger.
Fixes #567