From 2f127e6a85e2f9e7cbf72b5a6e61a8362ae3826d Mon Sep 17 00:00:00 2001 From: Viet Nguyen Date: Fri, 14 Aug 2026 23:48:05 +0000 Subject: [PATCH] fix(ui): a hover that never moved anything, and a selection that vanished under the pointer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewing the two interface changes that just landed, against the browser rather than against the source. Three of them were doing nothing, and the same reason runs through all three: a class cannot outrank an inline style, and Tailwind's `ring` is not a ring. **`.lift` moved nothing.** It set `transform: translateY(-2px)`, and both cards it is on are Motion elements — Motion animates by writing `transform` into the inline style, which no class can beat. Worse, the `transition: transform` beside it *did* apply to the inline transform Motion rewrites every frame, so the library's entrance animation had a 180 ms lag chasing itself. Moved to the `translate` property: its own slot, composes with whatever `transform` holds, and Motion never touches it. Measured after the change — `translate: 0px -2px` on hover, on a plain button and on a Motion element alike. **The chosen agent lost its outline under the pointer.** `.lift:hover` sets `box-shadow`, which beat the selected card's `shadow-[0_0_0_1px_accent]`, so pointing at the selected agent deselected it visually. The first attempt was `ring-1 ring-accent`, and the browser said no: Tailwind v4 implements a ring *as* a box-shadow, and the computed style came back with the same two shadows and no accent anywhere. `outline` is a separate property and survives both. Confirmed at rest and on hover: `rgb(15, 115, 80) solid 1px` in each. **`Ticker` re-counted on every change.** Its own doc says a number that changes because the user filtered the screen should snap, because they are comparing it against what was there a moment ago — and the code counted from zero every time, which on the analytics range switch turns a comparison into a wait. Counted once now, and the flag is set on completion rather than on start so React's development double-invoke does not eat the only run. Also: `Recent`'s cards were lifting through `whileHover={{ y: -2 }}` — a spring and a rAF loop per hovered card, to move something two pixels. They use the same `.lift` as everything else now, which costs nothing while the pointer is elsewhere. The token test grew `outline-` alongside `text-`, `bg-`, `border-` and `ring-`, since that is now a colour this interface names. Full gate on this tree: `cargo fmt --check`, `clippy --workspace --all-targets --features bundled -D warnings`, 1312 Rust tests, 298 web tests, all 18 browser suites, and 44 screenshots in both schemes with no overflow and contrast AA everywhere. --- apps/web/src/components/library/Recent.tsx | 13 ++++++++----- apps/web/src/components/ui/Ticker.tsx | 16 +++++++++++++--- apps/web/src/screens/AgentsScreen.tsx | 10 +++++++++- apps/web/src/styles/theme.css | 16 ++++++++++++---- apps/web/src/styles/theme.test.ts | 12 ++++++------ 5 files changed, 48 insertions(+), 19 deletions(-) diff --git a/apps/web/src/components/library/Recent.tsx b/apps/web/src/components/library/Recent.tsx index f5a25e0..99629bf 100644 --- a/apps/web/src/components/library/Recent.tsx +++ b/apps/web/src/components/library/Recent.tsx @@ -8,7 +8,7 @@ import { cn } from "../../lib/cn"; import { formatDuration } from "../../lib/duration"; import { useEngine } from "../../lib/engine-context"; import { LibraryClient, dayLabel, localDay, type MeetingSummary } from "../../lib/library"; -import { GENTLE, listItem, stagger } from "../../lib/motion"; +import { listItem, stagger } from "../../lib/motion"; /** * The last few things in the vault, as cards. @@ -84,11 +84,14 @@ export function Recent({ > {entries.map((entry) => ( - onOpen(entry)} - whileHover={{ y: -2 }} - transition={GENTLE} className="border-line bg-bg-soft lift flex w-full items-center gap-3 rounded-[var(--radius-card)] border p-2.5 text-left" > {/* A note has no waveform because it was typed; it gets the pen instead, at the same @@ -129,7 +132,7 @@ export function Recent({ ))} )} - + ))} diff --git a/apps/web/src/components/ui/Ticker.tsx b/apps/web/src/components/ui/Ticker.tsx index c46b5de..83049b3 100644 --- a/apps/web/src/components/ui/Ticker.tsx +++ b/apps/web/src/components/ui/Ticker.tsx @@ -28,6 +28,7 @@ import { useI18n } from "../../i18n/context"; */ export function Ticker({ value, className }: { value: string; className?: string }) { const node = useRef(null); + const counted = useRef(false); const still = useReducedMotion(); const { locale } = useI18n(); @@ -39,18 +40,27 @@ export function Ticker({ value, className }: { value: string; className?: string useEffect(() => { const element = node.current; if (!element) return; - if (target === null || still) { + // Counted once, then never again for the life of this element. + // + // The analytics screen switches range — today, seven days, thirty — and the figure it holds + // changes under a person who is comparing it against the one that was there a second ago. + // Rolling that from zero every time turns a comparison into a wait, and it is the animation + // equivalent of a page reload. Arrival is the only moment counting says anything true. + if (target === null || still || counted.current) { element.textContent = value; return; } - // From zero, not from the previous number: this only ever runs on first arrival, and "0 → 12" - // is the shape of something being counted. const running = animate(0, target, { duration: Math.min(0.9, 0.25 + target * 0.02), ease: [0.16, 1, 0.3, 1], onUpdate: (at) => { element.textContent = format.format(Math.round(at)); }, + // On completion rather than on start, so an animation that was torn down before it finished + // — React's development double-invoke does exactly this — is allowed to run again. + onComplete: () => { + counted.current = true; + }, }); return () => running.stop(); }, [target, value, still, format]); diff --git a/apps/web/src/screens/AgentsScreen.tsx b/apps/web/src/screens/AgentsScreen.tsx index 3a4755b..9231566 100644 --- a/apps/web/src/screens/AgentsScreen.tsx +++ b/apps/web/src/screens/AgentsScreen.tsx @@ -194,8 +194,16 @@ export function AgentsScreen() { onClick={() => void open(each.slug)} className={cn( "rounded-card bg-bg-soft lift border p-4 text-left", + // An outline for the chosen card, not a shadow and not a ring. + // + // `.lift:hover` sets `box-shadow`, and a class beats a utility of lower specificity, + // so hovering the selected agent replaced its accent edge with the ordinary hover + // shadow and the selection vanished under the pointer. Swapping to `ring-1` did not + // help — Tailwind v4 implements a ring *as* a box-shadow, which the browser confirmed + // by reporting the same two shadows and no accent at all. `outline` is a separate + // property and survives both. each.slug === chosen - ? "border-accent shadow-[0_0_0_1px_var(--color-accent)]" + ? "border-accent outline-accent outline-1" : "border-line hover:border-fg-faint", )} > diff --git a/apps/web/src/styles/theme.css b/apps/web/src/styles/theme.css index ff00adc..7b22148 100644 --- a/apps/web/src/styles/theme.css +++ b/apps/web/src/styles/theme.css @@ -360,23 +360,31 @@ body { * that jumps under the pointer is a card that moves out from under a click. */ .lift { transition: - transform 0.18s var(--ease-out), + translate 0.18s var(--ease-out), box-shadow 0.18s var(--ease-out), border-color 0.18s var(--ease-out); } .lift:hover { - transform: translateY(-2px); + /* `translate`, not `transform`. + * + * Both cards this is on are Motion elements, and Motion animates by writing `transform` into the + * inline style — which no class can outrank. A `transform: translateY(-2px)` here did nothing at + * all on either of them, and the `transition: transform` beside it was worse than nothing: it + * applied to the inline transform Motion rewrites every frame, so the library's entrance + * animation had a 180ms lag chasing it. `translate` is its own property, composes with whatever + * `transform` holds, and Motion never touches it. */ + translate: 0 -2px; box-shadow: var(--shadow-pop); border-color: var(--color-line-strong); } .lift:active { - transform: translateY(0); + translate: 0 0; transition-duration: 0.08s; } @media (prefers-reduced-motion: reduce) { .lift:hover, .lift:active { - transform: none; + translate: none; } } diff --git a/apps/web/src/styles/theme.test.ts b/apps/web/src/styles/theme.test.ts index b662a1b..876b298 100644 --- a/apps/web/src/styles/theme.test.ts +++ b/apps/web/src/styles/theme.test.ts @@ -91,8 +91,8 @@ describe("colour utilities", () => { const defined = new Set([...css.matchAll(/--color-([a-z0-9-]+):/g)].map((m) => m[1]!)); /** - * The four prefixes that are colours often enough to be worth checking, and the values they take - * that are *not* colours. + * The prefixes that are colours often enough to be worth checking, and the values they take that + * are *not* colours. * * Tailwind overloads these: `text-sm` is a size, `bg-cover` is a fit, `border-b` is an edge. The * list is static, it is Tailwind's rather than ours, and a false positive costs one line here — @@ -109,8 +109,8 @@ describe("colour utilities", () => { // border- ...["0", "2", "4", "8", "x", "y", "s", "e", "t", "r", "b", "l"], ...["solid", "dashed", "dotted", "double", "hidden", "collapse", "separate"], - // ring- - ...["1", "3", "inset", "offset"], + // ring- and outline- + ...["1", "3", "inset", "offset", "dashed-2"], // Tailwind's own palette, used directly in a few places where a literal is honest. ...["white", "black", "transparent", "current", "inherit"], ]); @@ -127,7 +127,7 @@ describe("colour utilities", () => { return out; } - it("has a value behind every text-, bg-, border- and ring- colour the interface names", () => { + it("has a value behind every text-, bg-, border-, ring- and outline- colour it names", () => { const files = sources("../"); expect(files.length, "no sources found — this test is checking nothing").toBeGreaterThan(20); @@ -160,7 +160,7 @@ describe("colour utilities", () => { // The name only, without the opacity suffix: `text-accent/60` is `accent`. The lookbehind is // what stops `--avatar-text-c` and `--color-bg-elevated` reading as utilities. for (const [, name] of source.matchAll( - /(?