feat(runner): DEV-2155 icon system — tabler-icons + seti-ui file icons - #91
Merged
Merged
Conversation
…cons One icon surface for the redesign, so T2/T3/T4 don't each invent their own. tabler-icons ships as a real dependency, re-exported from src/icons/ui.tsx under stable local names through a wrapper that pins the design's 16px/2px rendering (tabler defaults to 24) and marks glyphs aria-hidden. Twenty icons, all but sun/moon and the two toggled counterparts read straight off Figma layer names in section 18.1. No Vite config needed: the scanner crawls the aliased shell source and pre-bundles the package on its own — measured faster than an explicit optimizeDeps.include (741ms vs 1029ms to first paint on a cleared .vite). seti-ui has no React distribution, so scripts/sync-seti-icons.mjs generates src/icons/generated/seti.ts from a pinned commit: 29 icons, 38 suffixes, 6 exact filenames, ~22KB. Colour resolves through mapping.less + ui-variables.less rather than each SVG's baked fill — the two disagree upstream and only the mapping carries per-key colours (.test.ts is an orange typescript, .ts a blue one). Geometry is flattened to fill="currentColor" and coloured via CSS, so selection and muted states need no second code path. The generator fails rather than degrades: every curated key must resolve, every icon must have geometry, no transform, no non-<path> geometry, at most one distinct fill. It also replays the runtime resolver over 14 fixtures, which is what gates "unknown extension falls back to a generic icon" — editor-shell has no build step and the repo has no DOM test runner. Resolution order is exact basename, then longest dotted suffix (covering compound suffixes, dotfiles and plain extensions), then seti's generic default. ThemeToggle drops its hand-inlined sun/moon for imports from the new layer, as its own comment asked. The unicode glyphs elsewhere stay — swapping those is T2/T3/T4 restyle work. Refs DEV-2155, ADR-0024.
ADR-0024 records the three calls: tabler as a dependency, seti generated from a pinned commit, curated coverage with a generic fallback. ADR-0022 and theme.ts gain the generated seti palette as a third named exception to the colour-literal rule — upstream brand values, identical in both modes. Two items appended to the redesign plan's running log rather than resolved inline, per that table's convention: 4. The frames take file icons from seti's raw SVGs, baked fills and all (main.ts is #529bba, yml #9f74b3). Two rows — index.html and tsconfig.json — use the yellow ejs glyph where seti's mapping gives html/orange and tsconfig/blue. Needs a design nod. Everything else matches, package.json's generic json icon included. 5. T1's scope line lists "panel toggles" while the same document states the design has none. No such icon shipped. Refs DEV-2155.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ccb68ff. Configure here.
…resolves Bugbot caught it: every starter in catalog.json ships LICENSE.txt, and the generator reduced upstream's `.icon-partial` rules to exact-name lookups. The exact key "LICENSE" therefore never hit and those rows fell through to the `.txt` suffix, drawing seti's generic `default` glyph instead of the licence one. Upstream has two distinct match kinds and now so do we: `.icon-set` keys stay exact-name or suffix lookups, `.icon-partial` keys become an ordered substring table checked between the two. That position matters — `.txt` is a curated suffix, so a partial step after the suffix walk would change nothing. It also mirrors mapping.less, where the partial rules are stated after the extension rules and win on CSS order. Curated partials are one entry (LICENSE), and the generator resolves it against mapping.less like any other key, so upstream dropping it fails the run. Fixture 15 pins LICENSE.txt -> license. Substring matching is greedy the way upstream is — MY_LICENSE_NOTES.md gets the licence glyph too. That's seti's behaviour, kept deliberately rather than half-reimplemented. Refs DEV-2155, ADR-0024.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

T1 of the DEV-2027 redesign. One icon layer so T2 (chrome), T3 (sidebar) and T4 (tabs) don't each invent their own. Blocking for those three. Decisions recorded in ADR-0024.
What's in it
tabler-icons as a real dependency.
@tabler/icons-react@^3.46.0ineditor-shell, 20 icons re-exported fromsrc/icons/ui.tsxunder stable local names through a wrapper that pins the design's 16px/2px rendering (tabler defaults to 24px) and marks glyphsaria-hidden— labels belong on the enclosing button. All butsun/moonand the two toggled counterparts (layout-sidebar-left-collapse,chevron-right) were read straight off Figma layer names in section18.1. Sun and moon appear in no frame at all; they come from T0, which inlined them by hand and handed them here by name in a comment.seti-ui generated from source. No React distribution exists, so
scripts/sync-seti-icons.mjsreadsmapping.less,ui-variables.lessand the neededicons/*.svgfrom a pinned commit and emitssrc/icons/generated/seti.ts— 29 icons, 38 suffixes, 6 exact filenames, ~22KB. Committed output; CI installs--frozen-lockfileand never runs the generator.Colour comes from the mapping, not each SVG's baked fill. The two disagree upstream (
typescript.svgbakes#529BBAwhere@blueis#519aba;yml.svgbakes#9F74B3where@purpleis#a074c4), the mapping is what seti's own editor uses, and only the mapping carries per-key colour —.test.tsis an orange typescript,.tsa blue one. Geometry renders withfill="currentColor"and is coloured through CSScolor, so selection and muted states need no second code path.folderis the one exception: no mapping entry, so it borrows its own fill (#ababab, exactly what the frame measures).Resolution order: exact basename → longest dotted suffix (covers compound suffixes, dotfiles and plain extensions in one walk) → seti's generic
default. Exact-name matching is load-bearing, not decorative:72:16991givestsconfig.jsonandpnpm-lock.yamldistinct icons.One import surface: everything re-exported from the package barrel.
editor-shell/package.jsonhasmainonly, and both consumers alias the bare specifier straight at the barrel, so@handsontable/demo-editor-shell/iconswould not resolve.ThemeToggledrops its hand-inlined sun/moon, as its own comment asked. The unicode glyphs elsewhere (✎ ✕ + ↗ ▾ ▸ ✓) are deliberately untouched — swapping those is T2/T3/T4 restyle work.Calls made during implementation
optimizeDepsentry, against my own plan. A bareinclude: ["@tabler/icons-react"]fails outright — the app'snode_modulescan't reach a dependency of the shell. The nested"@handsontable/demo-editor-shell > @tabler/icons-react"form works but measured slower on a clearednode_modules/.vite: 1029ms vs 741ms with no config at all. Vite's scanner crawls the aliased shell source and pre-bundles the package itself. Config dropped..zip/.pdf/.mp4excluded. Their upstream icons are two-colour (a white<g>over a coloured body) and can't survive flattening tocurrentColor. No runner demo contains those types, and the generator now fails loudly rather than drawing a broken glyph if one is ever added.<path>, not one concatenated string. The generator's assertions fired four times while writing it:default.svgis0 0 1200 1000,vite.svgis-75 -105 560 554,tsconfig.svghas no viewBox at all (width/height instead),yml.svgneedsfill-rule="evenodd". Each icon carries its own viewBox and path list..gitignorerenders near-invisible. seti maps it to@ignore#41535b, which is ~1.6:1 on the dark sidebar. Upstream means it as "this file is ignored"; a demo's.gitignoreisn't.<FileIcon color>exists if T3 wants an override. Noted in ADR-0024 consequences.Two open items for design (appended to the plan's running log, not resolved here)
72:16991,main.tsis#529bbaandpnpm-lock.yamlis#9f74b3. Two rows deviate from seti's own mapping:72:17026(index.html) and72:17056(tsconfig.json) are both the layerejs 1, measured#d3c238, where the mapping giveshtml/orange andtsconfig/blue. Reads like a neighbouring sheet cell got grabbed. Everything else matches the frame exactly —package.json's generic json yellow included, since seti has nopackage.jsonentry and falls through to.jsonprecisely as drawn.Verification
pnpm install --frozen-lockfile,pnpm build,pnpm typecheckclean ·pnpm test67/67 ·pnpm e2e10 passed. First e2e run reused a stray server on 4173 and proved nothing (reuseExistingServer: !CI); redone after killing it.Generator re-run against the committed output produces zero diff.
Browser: dev and preview both exercised — the toggle click flips
tabler-icon-moon→tabler-icon-sunwith no page errors. A throwaway gallery rendered all 29 icons plus 25 resolution cases:App.test.tsorange TS,app.component.ts→ typescript,index.astro/Makefile/noext→default.Bundle 6684K → 6696K, so the barrel didn't collapse.
Automated gate, stated plainly: there is no vitest/jsdom anywhere in the repo and
editor-shellhas no build step, sopipeline/*.test.mjscannot import the resolver. The gate istsc --noEmitplus the generator's assertions and its 14-fixture replay of the resolution order — that replay is what backs "unknown extension falls back to a generic icon". Visual confirmation of file icons in a real tree lands with T3.Closes DEV-2155.
Note
Low Risk
Mostly additive shell infrastructure and docs; runtime UX change is limited to the theme toggle icons, with a new dependency and ~22KB generated icon data.
Overview
Adds the DEV-2027 T1 icon layer in
editor-shell: one barrel export for chrome and file-type glyphs, documented in ADR-0024 and wired into the redesign plan as shipped.UI chrome uses
@tabler/icons-react, re-exported fromsrc/icons/ui.tsxwith 16px / 2px defaults andaria-hidden.ThemeToggledrops its hand-inlined sun/moon in favor of those imports; other unicode stand-ins are intentionally unchanged until T2–T4.File types come from seti-ui via
scripts/sync-seti-icons.mjs(pinned upstream SHA) into committedsrc/icons/generated/seti.ts.<FileIcon />,<FolderIcon />, andresolveFileIcon()resolve paths (exact name → partial → longest suffix →default), with colours frommapping.lessand geometry ascurrentColor. ADR-0022 /theme.tsnow treat the generated seti palette as a third allowed colour-literal exception.Reviewed by Cursor Bugbot for commit 61c82ca. Bugbot is set up for automated code reviews on this repo. Configure here.