chore(icons): add workflow icons generator - #6572
Conversation
|
📚 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 |
9f272c0 to
5f7a3ac
Compare
d68ef6c to
5f7a3ac
Compare
fcc0c51 to
64a3740
Compare
5f7a3ac to
0ffc6bf
Compare
64a3740 to
faf07d0
Compare
0ffc6bf to
7ecaa90
Compare
faf07d0 to
e21e74a
Compare
7ecaa90 to
1f31550
Compare
e21e74a to
8bf9cbe
Compare
1f31550 to
a35a1a1
Compare
8bf9cbe to
6085381
Compare
a35a1a1 to
c350f1c
Compare
There was a problem hiding this comment.
Considering the real workflow set of 1096 icons, this emits 2192 modules. vite.config.ts's components/*/swc-*.ts glob would turn 1096 of them into rollup entries against the current 51; cem.config.js globs components/**/*.ts and yarn analyze runs ahead of every storybook and storybook:build; dist/custom-elements.json is already 869K and is a published export; and the npm script prettiers all 2192 on each run.
None of these needs solving in this PR, but it is the main input to packaging decision, so it'd be good to have the numbers written down before that phase starts rather than during it.
I don't see an automated tests here and the input grammar is unverified and this script is checking out by design so this will not execute in CI.
I will hold off this merge on the comments here and also on #6567 which has an open thread on close-button cross art.
| // Refuse to run without source SVGs. The source folder is git-ignored, so on a clean | ||
| // checkout this would otherwise delete the committed workflow icon art and write | ||
| // nothing, breaking every consumer. | ||
| if (files.length === 0) { |
There was a problem hiding this comment.
This throws when no source files exist, but the wipe at line 104 keys off names, which is only populated after SOURCE_NAME matches. So if the real A4U filenames don't match the inferred grammar, the sequence is: guard passes (files exist) → every file hits the console.warn skip → names is empty → every .ts in components/workflow-icons/ is deleted → nothing is written → exit code 0.
Please put that after the parse loop and keeping the current check too. I'd also make any skip fatal rather than a warning
|
|
||
| protected override render(): TemplateResult { | ||
| return html\` | ||
| <span class="swc-Icon">\${unsafeSVG(${functionName}())}</span> |
There was a problem hiding this comment.
Blocking: This reintroduces unsafeSVG, which #6552 removed as one of its stated goals, across all 1096 public elements.
I understand why but if you want the public substrate a string the generator already holds the cleaned SVG at this point, so it can emit both forms:
StarIcon(): string— unchanged, the public Lit-free substrate- a Lit
svgtemplate — internal, used by the generated element
|
|
||
| // A4U source filename: S2_Icon_<LogicalName>_20_N.svg (one fixed-size drawing per | ||
| // icon, no optical step). | ||
| const SOURCE_NAME = /^S2_Icon_(?<name>.+?)_20_N\.svg$/; |
There was a problem hiding this comment.
Is this verified? _20_ hardcodes a single size. That matches the RFC's workflow sizing model (one drawing scaled to a token box), but if A4U ships 20 and 24, everything else is silently skipped.
Could we get one real sample filename into the PR before we merge. I dont have anything to validate this against.
| } | ||
| } | ||
|
|
||
| export class ${elementClassName} extends IconBase { |
There was a problem hiding this comment.
This is missing JSDoc. Also Class and registration in one module drifts from the repo convention
| // deletions in source propagate. | ||
| mkdirSync(outDir, { recursive: true }); | ||
| // workflow-icons/ holds only generated modules, so clearing them all is safe. | ||
| for (const stale of globSync('*.ts', { cwd: outDir })) { |
There was a problem hiding this comment.
Two things here:
generate-ui-icons.mjs emits an index.ts with the UI_ICONS registry and the UiIconName type. This generator emits nothing enumerable, so there's no way to list the available icons programmatically, and no name union for consumers.
Also this deletes every .ts in the output directory. Either emit the index from the generator (matching the UI one), or scope the clear to the two generated filename patterns.
|
Going to close this out in favor of the generator work in #6562. |
2 participants
Description
Adds
scripts/generate-workflow-icons.mjs: converts A4U workflow SVGs into a per-icon function (StarIcon(): string) and a per-icon element (<swc-icon-star>extendingIconBase), reusing Phase 0's sharedicon-source/utils/. Staged inswc/components/workflow-icons/for now; packaging into@adobe/spectrum-wc-iconsis Phase 4. Filename regex (S2_Icon_<Name>_20_N.svg) is derived from a public sibling repo's manifest, not a confirmed real A4U sample, flagged as such in the file header. Validated against synthetic fixtures only; no real icon art is included or committed.Motivation and context
Implements Phase 3 of the icon RFC (SWC-2442): public workflow icons as a Lit-free function plus a custom element.
Related issue(s)
Screenshots (if appropriate)
N/A — build script only, no UI.
Author's checklist
Reviewer's checklist
patch,minor, ormajorfeaturesManual review test cases
S2_Icon_Star_20_N.svg,--iconPrimaryfill) intoicon-source/workflow/, runyarn generate:workflow-icons, confirm both output files are generated, fill is rewritten tovar(--swc-icon-color, currentColor), and lint/prettier pass. Delete the fixture and output after.icon-source/workflow/throws instead of deleting anything.Device review
Accessibility testing checklist
IconBase(existing, already-tested a11y contract: decorative by default,role="img"+aria-labelwhenaccessible-labelis set); no new behavior introduced.