Fix: draw component borders with outline instead of ring (Safari border rendering at non-100% zoom) - #30190
Conversation
github-actions
Bot
added
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Jest test CoverageUI tests summary
|
…drop dead ring class, fix import order - OwnerStackOverflow: drop tw:outline-hidden — it set outline-style:none and erased the outline that now draws the tooltip border (all browsers). - avatar-company-icon: remove dead unprefixed ring-[1.67px] from the 2xl size. It generates no CSS under prefix(tw), but contradicts the no-rings rule and would spring to life if the prefix bug is fixed. - ManageMenuButton: run organize-imports so the added borderAfter import is sorted, fixing the UI Checkstyle lint-src failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code Review ✅ Approved 1 resolved / 1 findingsReplaces Tailwind ring utilities with outline styles to resolve Safari rendering issues at non-100% zoom levels. The fix also addresses an issue where outline-hidden erroneously erased tooltip borders. ✅ 1 resolved✅ Bug: outline-hidden erases the new outline border on owners tooltip
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
🔴 Playwright Results — 1 test failure(s)✅ 4549 passed · ❌ 1 failed · 🟡 20 flaky · ⏭️ 99 skipped
Genuine Failures (failed on all attempts)❌
|
Code Review ✅ Approved 1 resolved / 1 findingsReplaces Tailwind ring utilities with outline styles to resolve Safari rendering issues at non-100% zoom levels. The fix also addresses an issue where outline-hidden erroneously erased tooltip borders. ✅ 1 resolved✅ Bug: outline-hidden erases the new outline border on owners tooltip
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
Fixes #30188
What's the problem
In Safari, component borders thin out — and at some zoom levels vanish entirely — at any zoom other than 100%. It looks Safari-only, but isn't quite.
The design system never drew these borders with a real
border. It used Tailwind'sring-*, which compiles to abox-shadow, and WebKit does not pixel-snap box-shadows. At a fractional device pixel ratio (Safari reportsdevicePixelRatio: 1.7at 85% zoom on Retina) a 1px ring cannot occupy a whole number of device pixels, so it is antialiased across two columns and renders washed out or not at all.borderandoutlineare snapped and never degrade.Chromium degrades rings too — it just never falls far enough to notice, which is why only Safari gets reported. Peak border darkness sampled from rendered pixels across a 50–150% zoom sweep (42 = full strength for the token, 0 = invisible):
borderringoutlineWhat changed
ring-*is replaced withoutline(orborder) as the border primitive across the design system — ~120 sites in 85 files — with no intended visual change at 100% zoom.Why
outlineand notborder: aborderoccupies layout. On controls whose height is content-driven it would add 2px and make them grow on focus (1px → 2px).outlineis layout-neutral, and an inset outline is pixel-identical to the ring it replaces.Three patterns, by situation:
outlineis freetw:outline-1 tw:-outline-offset-1 tw:outline-<token>outlineis already the focus ring (buttons, checkbox, radio, toggle, tags, tabs, slider)::afteroverlay (borderAfterinutils/tailwindClasses) — an element gets exactly one outline, and these must show border and focus ring simultaneouslyring-0)outline-0/after:outline-0— must flip with the core change, or the suppressor becomes a no-op and borders reappearOffsets matter:
ring-inset→-outline-offset-1; noring-inset→ offset 0 (a non-inset ring draws outward). Getting this wrong shifts an edge by 1px.Verification
outline-*counterpart dereferences to the same value — 41 tokens, zero mismatches.ring-*andoutline-*use different theme namespaces, so this was not a given.ringvs::afteroutline, identical geometry): WebKit @2x differs by 1 pixel at delta 1/255 — visually identical at working zoom. WebKit @1.7 differs at max delta 42 — that difference is the bug being fixed.tsc, ESLint, Prettier all clean. The two ring-coupled tests inDataQualityDashboard.test.tsxpass unmodified.Rings deliberately left in place
Each would be a visual change to convert, so they aren't:
ring-offset-*halos (color-picker-field,icon-picker-field) —ring-offset-colorfills the gap;outline-offsetleaves it transparent. Not reproducible. Also 2–3 rings compete for one outline there.Avatarconsumers (OwnerReveal,OwnerStackOverflow) —Avataralready uses an outline; a second would clobber it. Wants Avatar'scontrastBorderAPI instead.ring-secondarywith no ring-width (AgentCard,RunHistoryDrawer) — renders nothing today; converting would make an invisible border appear.ring-0onCard(OntologyExplorer,DataQualityDashboard) —Carduses a real border and never had a ring, so it is already inert. Converting would kill Card's focus outline.Docs
docs/colors.md§2.3.1 documents the rule, the measurements, the ring→outline translation table, and the gotchas that make this easy to get wrong:outline-hiddenerases an outline border; unlayered LESSoutline: nonebeats Tailwind utilities regardless of specificity;transition-shadowdoes not animate an outline; and overriding a Button's border needsafter:outline-*, sinceoutline-*silently sets its focus colour instead. Summarised in bothCLAUDE.mdfiles and added to the anti-pattern cheat sheet so the pattern isn't reintroduced.Reviewer notes
::aftercomponent — that's the whole reason for the overlay approach and the main regression risk.::beforewas already taken onbutton.tsx/social-button.tsx(inner gradient), hence::after.Before:
After:
Greptile Summary
This PR replaces ring-based component edges with pixel-snapped outlines or borders.
Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
Reviews (5): Last reviewed commit: "Merge branch 'main' into fix-ring-border..." | Re-trigger Greptile
Context used (3)