refactor(icons): repoint ui icon consumers - #6567
Conversation
🦋 Changeset detectedLatest commit: 42ad8a9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📚 Branch Preview Links🔍 Gen1 Visual Regression Test ResultsWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
Deployed to Azure Blob Storage: If the changes are expected, update the |
64a3740 to
faf07d0
Compare
8bf9cbe to
6085381
Compare
Rajdeepc
left a comment
There was a problem hiding this comment.
I dont see a changeset here but we are changing close-button rendering. Please update this to a fix or a refactor and add a changeset.
Some blocking items too. Let me know if you would like to discuss.
Coverage Report for CI Build 32155952033Coverage remained the same at 96.241%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats💛 - Coveralls |
8f53f17 to
662cefc
Compare
662cefc to
f32e145
Compare
That would be on consumers to adjust — we could clip it, but that would impact things like focus rings our hover states that extend beyond the bounds of the box. ✨ |
f32e145 to
dc065cb
Compare
f98ba05 to
b01bd3c
Compare
b01bd3c to
b058876
Compare
| Repoint `CloseButton`, `MessageSources`, and `ResponseStatus` off the pre-RFC `icon/elements/` module onto the internal `<swc-ui-icon>` element, matching the Accordion POC. No public API changes. | ||
|
|
||
| - Fixed a pre-existing bug where `swc-close-button`'s default (`m`) size had no `:host([size="m"])` rule, so its cross icon fell through to the `ui-icon-large` box size instead of `ui-icon-medium`. | ||
| - Visual change: `swc-close-button`'s cross icon now uses the RFC's fixed size-to-step map instead of its old ad hoc numbering. `s`/`m` now render the previously-unused `Cross75`/`Cross100` glyphs in place of the old `Cross200`/`Cross300` shapes, and `l`/`xl` may render a slightly different glyph than before. |
There was a problem hiding this comment.
Not a close button issue but this might be a phase 1 completeness gap. help me clarify that if we lock down on this any future component needing a step above 300 will hit the same wall silently. Not blocking but we should have clarity.
Should we generate the missing steps of 400/500/600 and extend UI_STEPS and keep close-button pixel-identical at l/`xl?
There was a problem hiding this comment.
Or should we deliberately cap S2 Ui scale to 300? We need a design sign off if we do this
@rubencarvalho @caseyisonit @5t3ph
There was a problem hiding this comment.
Hmmm, IDK - on the designs it does seem like XL uses 500. So it feels like we may need those? Or the steps are incorrectly mapped?
| <swc-badge variant="neutral" size=${size}> | ||
| <swc-icon size=${size} slot="icon" aria-hidden="true"> | ||
| ${iconForSize(Icons, 'Checkmark', size)} | ||
| ${Checkmark100Icon()} |
There was a problem hiding this comment.
Note: Here badge diff are expected.
| ? html` | ||
| <swc-icon slot="icon" aria-hidden="true"> | ||
| ${iconForSize(Icons, 'Checkmark', 'm')} | ||
| ${Checkmark100Icon()} |
There was a problem hiding this comment.
Note: Here badge diff are expected.
ade6228 to
ee9834e
Compare
ee9834e to
42ad8a9
Compare

Description
Repoints
CloseButton,MessageSources, andResponseStatusoff the pre-RFCicon/elements/module onto<swc-ui-icon>, matching the Accordion POC. Deletes the now-unusediconForSizeStorybook helper (Badge stories now callicon/elementsfunctions directly). Removes<swc-icon>'s stale@status internaltag. Addsclose-button's first VRT coverage (base + custom-properties).icon/elements/itself is kept for now, it still backs<swc-icon>'s public export and docs; full retirement is a later phase.Motivation and context
RFC Phase 1: repoint any component rendering a UI icon internally onto
<swc-ui-icon>, nounsafeSVGremaining.Related issue(s)
Screenshots (if appropriate)
No API changes. Note:
CloseButton's cross icon now uses the RFC's fixed size-to-step map instead of its old ad hoc numbering, sol/xlmay render a slightly different glyph. Check the newclose-button.vrt.tssnapshot against Golden Hash.MessageSourcesandResponseStatusalso switch their chevron from a custom10pxinline size to<swc-ui-icon icon="chevron" size="s">; this is a visual no-op (sresolves to the same rendered size) but has no dedicated VRT coverage yet, so please double-check both patterns' expand/collapse chevrons visually.Author's checklist
Reviewer's checklist
patch,minor, ormajorfeaturesManual review test cases
size, and under hover/focus/active, no distortion or regression.icon-slotoption (Checkmark,Cross,Alert), confirm render, no console errors.Device review
Accessibility testing checklist
accessible-labelonly, icon stays silent (aria-hiddenby default viaIconBase). Toggle buttons announce label +aria-expanded; chevron stays silent. No behavior change from before this PR.