Round floating surface corners - #92
Conversation
🕙 Outdated review — superseded by a newer review below🤖 Michael review · approve · confidence 5/5Safe to merge. The semantic radius tokens are applied consistently across the shared menu, popover, modal, and responsive-sheet primitives, while caller overrides and the spin-off menu's full-bleed layout remain intact. The two failing installer checks concern a missing OpenCode engine and are unrelated to this frontend-only diff; type-check and tests pass.
|
Light-mode radius comparisonAll pairs use the same light theme; only corner geometry changes.
The phone pair forces |
🕙 Outdated review — superseded by a newer review below🤖 Michael review · approve · confidence 5/5Safe to merge. The new commit only merges current
|
A bare `#3` chipped as a PR reference in prose. The first fix required 4+ digits, which is right for tella-fusion (#5648) and wrong for every repo numbered under a thousand — opensession is at #92, tella-mac at #14. Measured over 120k transcript entries: 4+ digit `#numbers` are almost all PRs, while 1-3 digit ones are mostly stream and step indices (`stream #0`), CSS hex colours (`color: #333`), and rankings (`#29`). The legit short form carries its own evidence — `PR #92` is how the small-numbered repos are actually written. So a short number now links when something other than its digits says PR: a `PR`/`PRs` cue in front of it, a qualifier (`backstage#92`), or a PR the session list already knows for that repo. The cue is matched by the tokenizer (which cannot see behind its own match position) but re-emitted as prose, so the chip stays labelled `#92` rather than repeating the word next to the PR icon.
The renderer can place `opensession#92` at any length, but a bare `#92` is a guess it declines to make for short numbers — `#3`, `#333` and `#29` are far more often a step, a hex colour or a ranking. Writing the repo id is the cheap half of that contract, so ask for it in the run instructions instead of leaning entirely on heuristics. Goes in the existing "Session UI rendering" section, so every run gets it.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🤖 OS review · comment · confidence 4/5Safe to merge if retaining the newer, larger phone-sheet geometry is intentional. The popup and desktop modal token changes hold together, but the merged diff no longer achieves the stated unification of phone sheets with the modal token.
|
![open-session-os-tella-dev[bot]](https://avatars.githubusercontent.com/u/65127121?s=60&v=4){kind=small}
| ? "inset-0 h-dvh max-h-none rounded-none bg-surface pb-[env(safe-area-inset-bottom)] pt-[env(safe-area-inset-top)] shadow-none" | ||
| : "inset-x-0 bottom-0 max-h-[94dvh] rounded-t-[calc(var(--sheet-radius,34px)*var(--rf))] bg-surface pb-[env(safe-area-inset-bottom)] shadow-[0_-12px_40px_rgba(0,0,0,0.35)]" | ||
| : "left-1/2 top-1/2 max-h-[85vh] w-[92vw] max-w-[30rem] -translate-x-1/2 -translate-y-1/2 rounded-[calc(18px*var(--rf))] bg-raised smooth-shadow-ring-lg", | ||
| : "left-1/2 top-1/2 max-h-[85vh] w-[92vw] max-w-[30rem] -translate-x-1/2 -translate-y-1/2 rounded-modal bg-raised smooth-shadow-ring-lg", |
There was a problem hiding this comment.
🟠 P2 — Confirm that phone sheets should remain outside the modal token
The merge changes only the desktop branch to rounded-modal; the adjacent phone branch remains rounded-t-[calc(var(--sheet-radius,34px)*var(--rf))]. This is exercised by real phone consumers such as SidebarCustomizeDialog and ProfileSection. With the current --rf values, an ordinary phone sheet resolves to about 26.2px in Safari/Firefox and 45.9px in Chrome, while rounded-modal resolves to about 17.0px and 29.7px respectively. Consequently the current PR no longer delivers its stated goal of unifying phone sheets on the modal token; the previously reviewed implementation used rounded-t-modal, so this behavior was lost during the merge. If preserving main's newer sheet geometry is intentional, update the PR scope and visual-verification claims; otherwise restore rounded-t-modal on the phone-sheet branch.
2 participants
Summary
rounded-popupandrounded-modaldesign tokens instead of reusing button and generic scale cornersThis is radius-only and intentionally separate from #90's shadow work.
Visual comparison
Light-mode before/after pairs for menus, popovers, dialogs, and the Safari/Firefox phone fallback
Verification
bun run typecheckbun test src/frontend(251 passing)tailwind-mergechecks for full and top-only radius overrides22.1pxwithcorner-shape: roundStarted by Kent de Bruin in this Michael session