feat(ui,app): chrome icon registry redraw + size API collapse (slice 08, issue #440)#491
Conversation
The folder icon was visually misleading for non-image dropped files. open-file matches the actual UI semantic — "this attachment is a file."
…callsites Slice 08 consolidates the two error/warning glyphs in the registry to a single warning entry. Updates the TextField error indicator + 3 test references to use the new name. Registry-side removal of alert-triangle lands in a follow-up commit.
Pre-clean for the size API collapse coming next. Removes size="small" (the most common existing value) and size="normal/medium/large" attributes from all <Icon> callsites. Behavior unchanged: with the prop still typed in icon.tsx, omitted size still defaults to "normal" for now.
…se size API to 16px Replaces 16 chrome icons with redrawn versions from the imagegen v4 round (agent, warning, automation, remote-control, server, prompt, photo, code, code-lines, chevron-down, file-tree, glasses, cloud-upload, circle, close-small, check-small), derives chevron-up/left/right by rotating chevron-down around (10,10), and ports 3 workshop-only icons (archive, arrow-down-to-line, github) into the registry. Removes the alert-triangle entry (now collapsed into warning). Also collapses the multi-size Icon API to a single 16×16 size: - IconProps.size and the data-size data attribute are dropped. - icon.css collapses the four size rules to a single 16px declaration. - Stories drop the Sizes variant; the docs block at the top of icon.tsx now describes the 4-keyshape system + spec link. - markdown.tsx no longer sets data-size when synthesizing icon nodes. Issue: #440
Potrace-derived icons emit nested subpaths (outer + inner) to express holes. The default nonzero rule fills both as solid, which broke the todo dock pending circle into a filled disc. Setting fill-rule on [data-slot="icon-svg"] restores correct rendering for all donut-shaped glyphs (circle, warning, glasses, file-tree, archive, github, etc.) without affecting stroke-only paths (fill="none" ignores fill-rule).
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (45)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request standardizes the Icon component by removing the size property and enforcing a fixed 16x16 pixel dimension across the application. It also renames the alert-triangle icon to warning in the TextField component and associated tests, and updates several icon references for improved consistency. I have no feedback to provide.
…ta-size
The slice-08 size API collapse removed [data-size] from rendered icons,
which broke the todo-toggle smoke that asserted on
[data-component="icon"][data-size]. Restoring data-size as a test hook
would re-introduce the contract we just dropped, so the Icon root now
exposes a stable identity hook (data-icon={name}) and the spec asserts
[data-icon="circle"] / [data-icon="circle-check"] under the pending and
completed todo items. Stroke-only icons inherit the same hook with no
visual change.
|
Thanks for the review. Addressed in P1 (e2e selector contract) — fixed. Rather than restoring
P2 (before/after screenshots) — acknowledged as a follow-up. The External API note ( While here, also pushed |
|
P2 deliverable per #440: before/after light + dark These are rendered via a self-contained Playwright script that parses the Hosted on a sibling Icon registry — light
Icon registry — dark
Surfaces — light
Surfaces — dark
Notes:
|
|
Re: review round 2 — confirming the open items. P2 (visual verification) — closed. Light + dark before/after screenshots posted as a separate comment (#issuecomment-4396640684). Coverage: full registry diff (86 keys, side by side) and four high-traffic surfaces (TextField error, todo dock pending/completed/in-progress, chevron family, composer attachments). PNGs are hosted on a sibling External API note — re-confirmed CI: |
Summary
Slice 08 of issue #440 redraws the chrome icon registry from imagegen v4 sheets, collapses the multi-size Icon API to a single 16×16 render, and adjusts a few callsites that depended on the old API.
alert-triangletowarningto match its semantic role at TextField error and permission-dock callsites; updates the rename test accordingly.Iconsize prop. The component now always renders 16×16; the four-tier API (small | normal | medium | large) is removed from props, CSS, stories, markdown helpers, and 33 callsites (59 attributes stripped). Where the previous size was visually load-bearing (a few settings rows and the home folder-add chip) the icon is now smaller; this is a deliberate part of the chrome simplification.image-attachments.tsxto theopen-fileicon for non-image attachments instead of the genericfolder.fill-rule: evenoddon[data-slot="icon-svg"]. Potrace emits nested subpaths to express holes; the default nonzero rule was filling donut glyphs (circle, warning, glasses, file-tree, archive, github, etc.) as solid discs. The fix is one CSS line and is inert for stroke-only paths.Test plan
bun --cwd packages/ui run typecheckcleanbun --cwd packages/app run typecheckcleanbun testin packages/ui (icon registry rename, text-field state matrix, inputs state matrix) — all icon-related tests pass; 2 pre-existing UI failures (apply-patch-file, session-diff) are unrelated to this slice and reproduce on HEAD without these changespackages/ui/src/components/icon.stories.tsx(Gallery story) for any glyph that still looks off at 16px