fix: render builds status filter dashes correctly in Safari#459
Conversation
Replace the CSS `border-dashed` rings (which Safari renders with uneven, sparse dashes on rounded elements) with the registry `StatusIcon` SVG, and sync `StatusIcon` from the e2b registry to pick up `pathLength`, which evenly distributes the dash pattern. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The button's `[&_svg]:text-icon-tertiary` (a descendant selector) overrode the rings' `text-*` color, so all rings rendered gray. The old CSS used `border-*` color, which that rule never touched. Mark the ring colors `!important` so each status keeps its color. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — clear Safari rendering fix with narrow scope; the inline findings are cosmetic nits and a pre-existing issue, none blocking.
Extended reasoning...
Overview
This PR fixes a Safari-only rendering defect in the builds status filter where border-dashed on rounded-full elements produced uneven, sparse dashes. The fix swaps the CSS-dashed <div> for the registry StatusIcon SVG (which has cross-browser-consistent dash distribution) and syncs StatusIcon to add pathLength={32} for even dash spacing. Two files changed; -space-x-1.5 becomes -space-x-2 to compensate for the slightly larger SVG. Before/after screenshots are included.
Security risks
None. This is a pure visual/CSS change with no auth, data handling, or input parsing involved.
Level of scrutiny
Low. The change is small, self-contained, well-commented (the author explicitly documents both the Safari workaround and the ! override), and has clear before/after screenshots demonstrating the fix. The only behavioral risk is a 1px size shift, which is mitigated by the spacing tweak.
Other factors
The inline findings are all minor: (1) size={17} is silently clamped to 16px by the button's [&_svg]:size-4 — purely a documentation/clarity issue with no visible effect; (2) a sibling log-level-filter has the same pre-existing Safari defect, but is explicitly out of this PR's scope; (3) the JSDoc base64 preview encodes pathLength=36 while the JSX uses 32 — dev-facing only, end users never see the preview. None of these warrant blocking merge.
…view
- status-filter: the button's `[&_svg]:size-4` clamped `size={17}` to 16px;
apply `size-[17px]!` so the ring renders at the intended ~14px size.
- log-level-filter: swap the same broken `border-dashed` ring for StatusIcon,
fixing the identical Safari defect in the sibling filter.
- status-icon: sync the JSDoc base64 preview to `pathLength=32` to match the
rendered JSX (was 36).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop the `size-[17px]!` override and inherit the button's `[&_svg]:size-4` (16px) for both the builds status filter and the log-level filter rings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Always render all three status rings; unselected ones use the muted `fill` color and selected ones keep their status highlight color, so ring positions stay stable as filters toggle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 participants
Fixes ENG-4333. The builds Status filter drew its dashed status rings with a CSS
border-dashedon rounded elements, which Safari renders with uneven, sparse dashes (Chrome was fine).StatusIconSVG instead of a CSS dashed border, keeping abg-bgbackdrop so the front ring masks the one behind it in the overlap.StatusIconfrom the e2b registry to pick uppathLength, which evenly distributes the dash pattern around the circle.🤖 Generated with Claude Code