feat(icon): make swc-icon public with BYO SVG docs and private elements catalog#6415
feat(icon): make swc-icon public with BYO SVG docs and private elements catalog#6415TarunAdobe wants to merge 7 commits into
Conversation
🦋 Changeset detectedLatest commit: 63e87b5 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🔍 First Generation 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 |
a9752d9 to
919bf1d
Compare
Coverage Report for CI Build 28080828176Coverage remained the same at 96.246%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats💛 - Coveralls |
919bf1d to
f021bc4
Compare
| } as const satisfies Record<ButtonStaticColor, string>; | ||
|
|
||
| const addIconSvg = `<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 36 36" aria-hidden="true" focusable="false"><path d="M31.5 17H19V4.5a1 1 0 0 0-2 0V17H4.5a1 1 0 0 0 0 2H17v12.5a1 1 0 0 0 2 0V19h12.5a1 1 0 0 0 0-2z"/></svg>`; | ||
| const addIconSvg = `<swc-icon slot="icon" aria-hidden="true"><svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 36 36" aria-hidden="true" focusable="false"><path d="M31.5 17H19V4.5a1 1 0 0 0-2 0V17H4.5a1 1 0 0 0 0 2H17v12.5a1 1 0 0 0 2 0V19h12.5a1 1 0 0 0 0-2z"/></svg></swc-icon>`; |
There was a problem hiding this comment.
aria-hidden appears both on swc-icon and on the svg element, is that intentional?
It also appears in the svg for the Anatomy story below.
| label: 'Expand', | ||
| size: 'm', | ||
| }, | ||
| }; |
There was a problem hiding this comment.
It would be cool to add a control to the playground to be able to change the svg, I think we can add one that allows you to add your own svg code.
| <!-- After --> | ||
| <swc-button accessible-label="Add item"> | ||
| <svg slot="icon">...</svg> | ||
| <swc-icon slot="icon" aria-hidden="true"> |
There was a problem hiding this comment.
aria-hidden is also being set here, I'm not sure it's necessary
| // PLAYGROUND STORY | ||
| // ──────────────────── | ||
|
|
||
| export const Playground: Story = { |
There was a problem hiding this comment.
It would be cool if this internal icon Playground featured a select menu of available icons
| - Always provide a descriptive `label` for informative icons | ||
| - Use empty labels only for purely decorative icons | ||
| - Keep labels short and specific (e.g., "Expand" instead of "Icon") | ||
| - When an icon accompanies visible text, omit `label` and set `aria-hidden="true"` on the icon so screen readers do not announce duplicate content |
There was a problem hiding this comment.
This contradicts L59:
- When no label is provided, slotted SVGs are marked
aria-hidden="true"
|
@rise-erpelding Addressed these in follow-up: removed the manual aria-hidden guidance/examples so decorative usage now relies on omitting label, which matches the component behavior; added a public Playground SVG control so consumers can try their own inline markup; and added an internal icon-name select for the maintainer catalog Playground. |
882b91f to
7018e68
Compare
| </swc-icon> | ||
| ``` | ||
|
|
||
| **With Lit:** |
There was a problem hiding this comment.
I'm not sure what to think about this block. We say "with lit", but it could be with any templating language / vanilla HTML too.
rubencarvalho
left a comment
There was a problem hiding this comment.
Looks good, but I feel like the internal documentation needs a bit of cleanup 😄
There was a problem hiding this comment.
just this one cleanup right? everything else looks pretty minimal to me...
5t3ph
left a comment
There was a problem hiding this comment.
For discussion: should components like Button and Badge automatically assign the corresponding size attribute to the icon, or accept whatever icon size a consumer adds? Can be a follow-up, if changes are needed.
|
Thanks for the thoughtful review @5t3ph, I addressed the icon docs/accessibility updates in this PR: • Updated icon.mdx sizing guidance to emphasize matching component sizes (removed per-size use-case descriptions). As for the should components like Button and Badge automatically assign the corresponding size attribute to the icon, or accept whatever icon size a consumer adds? This need a broader discussion outside of this PR's scope |
Document go-public scope and remove @status internal from swc-icon JSDoc. Co-authored-by: Cursor <cursoragent@cursor.com>
Document concrete how-to for export boundary, docs split, and ship checklist. Co-authored-by: Cursor <cursoragent@cursor.com>
Ship public Icon docs, stories, migration guide, and tests while keeping the elements catalog internal. Update pattern imports and badge/tabs migration docs to show the swc-icon wrapper pattern. Co-authored-by: Cursor <cursoragent@cursor.com>
Apply reviewer feedback by removing manual aria-hidden usage in icon slot examples, clarifying icon sizing guidance, and managing decorative SVG focusability in IconBase. Co-authored-by: Cursor <cursoragent@cursor.com>
Drop the generated type-import guidance and subclassing caution from the migrated getting-started block. Co-authored-by: Cursor <cursoragent@cursor.com>
21f8b3d to
63e87b5
Compare
| To reference the \`${baseClassName}\` type, import it as a type-only import: | ||
|
|
||
| \`\`\`typescript | ||
| import type { ${baseClassName} } from '@adobe/spectrum-wc/components/${packageName}'; | ||
| \`\`\` | ||
|
|
||
| > The class is exposed primarily for type purposes. Extending it is possible, but the internal shape is not part of the public API — if you choose to subclass, you do so at your own risk and may need to adjust your code between releases. |
There was a problem hiding this comment.
why is this being removed?
|
|
||
| When an icon accompanies a text label, the icon is decorative and should be hidden from assistive technology. | ||
| Apply `aria-hidden="true"` to the `<swc-icon>` so screen readers only announce the label text. | ||
| Omit `label` on `<swc-icon>` so screen readers only announce the label text. |
There was a problem hiding this comment.
This reads confusing. should it saw "so screen readers only announce badge label text"?
| * <swc-badge variant="neutral" fixed="fill"> | ||
| * <sp-icon-checkmark slot="icon"></sp-icon-checkmark> | ||
| * <swc-icon slot="icon"> | ||
| * <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 10 10">...</svg> |
There was a problem hiding this comment.
can we use the approved adobe checkmark icon?
| <swc-badge aria-label="Approved"> | ||
| <sp-icon-checkmark-circle slot="icon"></sp-icon-checkmark-circle> | ||
| <swc-icon slot="icon"> | ||
| <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 10 10">...</svg> |
There was a problem hiding this comment.
Can we use only approved Adobe icons throughout?
| | `--mod-icon-*` custom properties | `--swc-icon-*` equivalents (see [Styling](#styling)) | | ||
| | Removed | Replacement | | ||
| | -------------------------------- | -------------------------------------------------------------------------------------------------- | | ||
| | `name` attribute | Slot an inline `<svg>`. See [step 2](#2-replace-name-or-src-with-slotted-svg) | |
There was a problem hiding this comment.
i think the icon factory function call out still remains true since that is what A4U ships if im not mistaken
caseyisonit
left a comment
There was a problem hiding this comment.
This has a number of a11y issues and is missing the a11y migration plan before proceeding.
nikkimk
left a comment
There was a problem hiding this comment.
Can we please back up so I can do an a11y migration analysis on icon and update the migration plan accordingly?
There was a problem hiding this comment.
there are other issues in this file that are overriding a hosts role and that should not be happening. @nikkimk is gonna do a fully a11y review like other migrations to call out the correct behavior.
5t3ph
left a comment
There was a problem hiding this comment.
Enhancements to add:
- remove detected
widthandheightattributes from the SVG (otherwise, prevents our size styling from working) - add this attribute and value to SVG if missing:
xmlns="http://www.w3.org/2000/svg" - add warnings if the SVG does not have a
viewBoxattribute

Description
Make
<swc-icon>a public 2nd-gen component: public Storybook docs, consumer migration guide, and npm exportboundary, while keeping the internal
elements/SVG catalog monorepo-only.Included in this PR
@status internalfromswc-iconJSDoc and keep it aligned with other public 2nd-gen components(
@sinceonly)Icononly fromcomponents/icon/index.tsso@adobe/spectrum-wc/iconstops leaking internal SVGfactories
icon.stories.ts,icon.mdx, andmigration-guide.mdxfor a BYO SVG contract with no publicelements/import pathicon.internal.*down to a maintainer-only catalog for dev Storybookelements/paths instead of a public packagepath
swc-iconJSDoc examples to inline SVG only@adobe/spectrum-wc<swc-icon slot="icon">...</swc-icon>in icon-slotexamples, matching the intended public icon path
Motivation and context
<swc-icon>already exists in 2nd-gen and is used internally by components and patterns, but it is stilltreated like an internal implementation detail. Consumers migrating from
<sp-icon>need a documented publicwrapper whose contract is simple: slot your own SVG.
This PR makes that public contract explicit.
It does not turn the internal
elements/SVG catalog into a public icon registry. Those factories remain amaintainer convenience for SWC development only.
It also aligns button docs with that direction so 2nd-gen documentation points consumers toward
swc-iconinstead of encouraging raw slotted SVG as the primary path.
Related issue(s)
Screenshots
N/A. This is primarily a docs, Storybook, and package export-boundary change.
Author's checklist
CONTRIBUTING.md) and [PULL_REQUESTS](https://github.com/adobe/spectrum-web-components/blob/main/
PULL_REQUESTS.md) documents.
TR/wai-aria-practices/)
after public story rename.)
Reviewer's checklist
patch,minor, ormajorfeatures
Manual review test cases
Public icon docs (production Storybook build)
yarn storybook:buildin2nd-gen/packages/swc<swc-icon>with slotted inline SVGelements/import guidance on the public pageInternal catalog (dev Storybook only)
yarn storybookin2nd-gen/packages/swcelements/reference still exist locally in dev StorybookConsumer migration guide
sp-icon→swc-icon, slot your own SVG, and no public@adobe/spectrum-wc/.../elementsimportpath
name,src,xxs/xxl,--mod-icon-*)Package export boundary
components/icon/index.tsexportsIcononly@adobe/spectrum-wc/icondoes not re-exportChevron100Iconor other internal factorieselements/importsButton docs alignment
<swc-icon>as the documented path<swc-icon slot="icon">...</swc-icon>Downstream patterns
@adobe/spectrum-wc/icon/elementsimports in sourceDevice review
Accessibility testing checklist
Icon a11y behavior is unchanged: slotted SVG receives image semantics when
labelis provided, and decorativeusage remains hidden from the accessibility tree when no label is present. Re-verify on the public docs
stories.
Keyboard
<swc-icon>is not focusable and does not create stray focus stopsScreen reader