Give CmIcon its own stylesheet, and import what App.vue actually uses - #20
Merged
Merged
Conversation
Two separate bugs, both hiding the same two components in the showcase. CmIcon's generated SVG carries no width, height, fill, or stroke of its own -- the outline shapes (regular/light/thin) have neither attribute at all, because in the source they were painted by the wrapper element they sat inside, not by themselves. icon.js's extraction regex kept only viewBox from that wrapper and dropped everything else, discovered by rendering the same icon through the original VueForge component and diffing the two <svg> tags directly rather than guessing: the source carried width/height via a CSS-variable fallback, fill=none unconditionally, and stroke/stroke-width/stroke-linecap/stroke-linejoin for every variant but solid, with stroke-width 2/1.5/1 for regular/light/thin. None of that needed to move into the generator or either adapter -- the classes it needs (cm-icon, cm-icon--regular, and so on) were already being rendered -- it only needed a stylesheet, which never existed. Separately, App.vue used <CmIcon> and <CmThemeSwitch> in its template without importing either from @codemonster-ru/ui-vue. Vue does not resolve template tags against a global namespace; a name not imported in <script setup> is treated as an unresolved custom element, so both were silently invisible regardless of the CSS fix. Caught by cross-checking every component tag the template uses against the import list, not just the two that had already been reported missing. Verified against the running dev server rather than assumed: confirmed the fix over HMR, then confirmed the rendered page directly.
…omponents from the CSS bundle The theme switch was flipping data-cm-theme and the token values behind it correctly, but nothing on the page ever repainted, for two independent reasons: - examples/ui-showcase/src/main.ts hand-picked one CSS import per demoed component and never imported foundation.css (or styles.css), which is what paints :root/body from --cm-color-background-canvas / --cm-color-text-primary in the first place. - examples/ui-showcase/src/showcase.css had its own body rule reading --cm-color-surface-default and --cm-color-text-default -- neither name exists in this design system, so it always fell through to its own hardcoded #fff/#111 fallback, on top of the cascade, regardless of theme. Verified live: drove the running dev server over the Chrome DevTools protocol and clicked the theme switch's own radios. Before the fix, body background stayed rgb(255, 255, 255) whether data-cm-theme was absent, "dark", or "light". After, it tracks the token (oklch(0.214 0.01 260) dark, oklch(0.978 0.005 260) light) in both directions. Fixing main.ts's import surfaced a second, unrelated bug: packages/css/src/styles.css -- the "everything" bundle -- never @imported icon.css or theme-switch.css (pre-existing), and was missing app-shell.css and document-layout.css too (a gap from this same round of fixes). Diffing dist/components/*.css against styles.css's own @import list found both. Added a test that enumerates the built component directory and asserts each file is reachable from styles.css, so a future component can't go missing the same way a spot-check list wouldn't have caught. That in turn exposed the same class of bug one layer down: the Razor asset sync script (scripts/ci/sync-code-monster-ui-razor-assets.mjs) never vendored those four component stylesheets for PHP consumers at all -- styles.css importing them made check:ui-razor-assets fail on an undeclared @import, which is exactly what it's for. Added the four missing entries and re-ran sync:ui-razor-assets to regenerate the vendored assets and manifest. showcase.css's now-redundant body rule is removed outright rather than fixed in place: reset.css already zeroes its margin and document.css already supplies its color/background/font-family from real tokens, so restating any of it here could only drift again.
You asked me to check the whole page rather than just the theme switch. A systematic sweep of every var(--cm-*) reference in packages/css/src against every custom property this package or ui-tokens actually defines found nine more, in six files, none of which crashed anything -- each one silently fell back to the property's initial value, which usually still renders something, just not the intended something: - theme-switch.css: --cm-space-3xs and --cm-space-xs (gap and padding collapsed to 0, which is why "Light System Dark" rendered as one unbroken run of text with no pill highlighting visible), and --cm-focus-ring-offset (the focus outline sat flush against the label instead of offset from it). This design system's spacing scale is numeric (--cm-space-0 through --cm-space-16, per docs/css/utilities.md) -- there never was a 3xs/xs scale for these to resolve against. - app-shell.css: --cm-space-md (the sidebar/content grid had no gap between columns). - document-layout.css: --cm-space-lg (same, for its content grid) and --cm-container-2xl (fell through to its own 80rem fallback, which happens to be narrower than the 96rem --cm-breakpoint-2xl that container.css's own --cm-container--2xl class resolves to). - nav-menu.css and admin-layout.css: --cm-motion-easing-standard, four times -- the real token is --cm-motion-ease-standard (every other component's transitions use it); the extra "ing" made four transitions instant instead of eased. - nav-menu.css, menu-bar.css, stepper.css: --cm-color-text-tertiary, four times -- there is no "tertiary" step in the text color scale, only primary/secondary/muted/disabled/placeholder. --cm-color-text-muted is the token this codebase already uses for the same kind of de-emphasized text elsewhere (date-picker's outside-month days, data-table's sort indicator). - admin-layout.css: --cm-color-background-overlay for the mobile sidebar scrim -- the real token is --cm-color-background-backdrop, the same one dialog.css and drawer.css already use for their own backdrops. Verified live, not just read: drove the running dev server over the Chrome DevTools protocol again. The theme switch's fieldset went from `gap: normal; padding: 0px` to `gap: 4px; padding: 4px`, and each option label from `padding: 0px` to `padding: 4px 8px`, with real spacing between their bounding rects. Also gave the App Shell demo's collapse button real classes (`cm-button cm-button--ghost cm-button--sm`) -- it was a bare <button> with none, so it was rendering as the browser's default gray/outset control (confirmed via the same CDP session: `background: rgb(107, 107, 107)`, `border: 2px outset rgb(255, 255, 255)`, 16px Segoe UI) rather than anything from this design system. Added packages/css/__tests__/token-references.test.mjs: it extracts every custom property this package or ui-tokens defines, then asserts every var(--cm-*) reference without a fallback names one of them. This is the check that would have caught all nine on its own, and now runs on every test:ui. Re-ran sync:ui-razor-assets since packages/razor/resources/assets vendors these files' built content directly. None of the six touched components are covered by the frozen vueforge-cross-platform visual baseline (checked contracts/cross-platform-visual-baselines.json's 12 case IDs directly), so there's nothing there to re-approve. npm run verify passes in full.
…age's dist packages/css/__tests__/token-references.test.mjs read packages/tokens/dist/tokens.css to check ui-css's own custom-property references. That's a real cross-package dependency, and scripts/ci/test-without-dist.sh -- the pre-push hook's isolation check -- wipes every workspace's dist and then runs each workspace's own `npm test` independently. ui-css's test script only builds ui-css, so with packages/css sorting before packages/tokens, the guard ran against a tokens/dist that had just been moved out of the way and failed on ENOENT, blocking the push outright. `npm run verify` never caught this because it runs `build` (all workspaces) before `test:ui` -- tokens/dist always existed by the time this test ran there. The pre-push hook is stricter on purpose: it is what proves each workspace's own test script is self-sufficient from clean source, which this test wasn't. Moved it to scripts/ci/css-token-references.test.mjs, wired as check:css-token-references and appended to check:ui-contracts (already part of verify's test:ui, already running after build) -- the same place check:ui-razor-assets lives for the same reason: it also reads across the css/tokens/razor package boundary. Logic and assertions are unchanged. Reran scripts/ci/test-without-dist.sh directly and npm run verify in full; both pass.
You asked whether the Admin Layout demo looked normal -- it didn't. Checked live: every layout demo's .ui-showcase__frame (App shell, Document layout, Admin layout, Admin shell, Setup layout) rendered at exactly 815px, matching the browser viewport's innerHeight (813px) to the pixel, not a coincidence. Each of those five layout components sets min-block-size: 100dvb/100vh on its own root, which is correct for what they actually are: a full page. .ui-showcase__frame gave them nothing to be inline preview-sized against -- overflow: hidden and resize: vertical with no starting block-size clip and resize nothing, so each layout just grew to fill the entire viewport, one after another, turning five compact previews into five near-empty full-screen sections. This couldn't be "the same as it looked in VueForge": the page's own intro text says these layouts are demonstrated here specifically because the frozen VueForge showcase never had them -- it used separate full-page routes per component, not one scrolling page of framed previews, so this failure mode is specific to this showcase's page architecture. Gave the frame an explicit block-size: 20rem for resize (already present) to shrink from, and fixed its border-radius reference from --cm-radius-md, which doesn't exist in this design system, to the real --cm-radius-surface. Verified live over CDP: all five frames now render at 320px instead of 815px. npm run verify passes in full.
…s leftovers The showcase demoed 13 components/layouts -- the ones that had no VueForge ancestor, back when a separate frozen VueForge showcase also existed and already covered the rest. That frozen showcase is gone (sunset earlier), so the other 36 components in @codemonster-ru/ui-vue had no live demo anywhere in this repository at all: Accordion, Alert, Avatar, Badge, Breadcrumbs, Button, Card, Checkbox, CommandPalette, Container, DataTable, DatePicker, Dialog, Divider, Dropdown, Drawer, Field, Fieldset, Grid, IconButton, Input, Inline, Link, Menu, Popover, ProgressBar, ProgressSpinner, Radio, Select, Section, Skeleton, Stack, Switch, Table, Tabs, Textarea, Tooltip. Added a demo section for each, following the existing pattern (heading, one-sentence note, minimal live example) and reading each component's actual prop types and templates rather than guessing at usage -- CmField/CmFieldset's slot-scoped composition matches the pattern already documented in docs/getting-started.md and docs/components/fieldset.md. Grid, Inline, Stack, Container, and Section needed no bespoke showcase CSS: their own stylesheets already set gap/padding/background, confirmed by reading each one rather than assuming. The handful of demos that do need a visible box (Container, Grid's cells, Stack's cells) use this design system's own @codemonster-ru/ui-utilities classes instead of new one-off showcase rules, so the showcase now also demonstrates that package for the first time. Verified every section live over the Chrome DevTools protocol, not just by reading the code: zero console warnings across all 50 sections, and confirmed the interactive ones actually work by scripting real clicks -- Dialog/Drawer/CommandPalette open and close, Dropdown/Popover toggle aria-expanded, Accordion expands a second item, Tooltip shows after its delay and hides on blur. That check caught a real bug before it shipped: the Tabs demo initially rendered zero tabs. CmTabs silently drops any item lacking either a `content` string or a matching panel slot (by design -- warnCm, not a thrown error), and the demo's tab items had neither. Added `content` to each and reverified three tabs render and switch correctly. Rewrote the showcase's own intro text and README, which both still described a showcase that only covered VueForge's gaps -- no longer true now that it covers the complete component set. npm run verify passes in full.
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.
What and why
You looked at the running showcase and asked if it was normal. It wasn't — two separate bugs, both hiding the same two components (
CmIcon,CmThemeSwitch).Bug 1:
.cm-iconhad no stylesheet at allCmIcon's SVG carries nowidth,height,fill, orstroke— the outline shapes (regular/light/thin) have neither attribute at all. In the source they were painted by the wrapper<svg>they sat inside, not by themselves.Checked against the source rather than guessed: rendered the same icon through the original
codemonster-ru/vueforgecomponent and diffed the two<svg>tags directly. The wrapper carriedwidth/heightvia a CSS-variable fallback,fill="none"unconditionally, andstroke/stroke-width/stroke-linecap/stroke-linejoinfor every variant butsolid(stroke-width2/1.5/1 for regular/light/thin).icon.js's extraction regex kept onlyviewBoxfrom that tag and silently dropped everything else.None of that needed to move into the generator or either adapter — the classes it needs (
cm-icon,cm-icon--regular, and so on) were already being rendered on both platforms. It only needed a stylesheet, which never existed. Addedpackages/css/src/components/icon.css.Bug 2:
App.vueused two components it never imported<CmIcon>and<CmThemeSwitch>were in the template but missing from the<script setup>import list. Vue does not resolve template tags against a global namespace — an unimported name is treated as an unresolved custom element, so both were silently invisible regardless of the CSS fix. This is why the first fix alone didn't show anything: the components weren't rendering at all yet.Cross-checked every component tag the template actually uses against the import list, not just the two already reported missing — all 13 now match (5 from
ui-layouts, 8 fromui-vue).Bug 3: the theme switch changed nothing you could see
Asked separately whether light/dark looked supported — it wasn't, for two independent reasons:
main.tshand-picked one CSS import per demoed component and never importedfoundation.css(orstyles.css), which is what paints:root/bodyfrom--cm-color-background-canvas/--cm-color-text-primaryin the first place.CmThemeSwitchwas writingdata-cm-themeand the token values behind it correctly the whole time; nothing on the page ever read them.showcase.csshad its ownbodyrule reading--cm-color-surface-defaultand--cm-color-text-default— neither name exists in this design system, so it always fell through to its own hardcoded#fff/#111fallback, on top of the cascade, regardless of theme.Verified live rather than assumed: drove the running dev server over the Chrome DevTools protocol and clicked the theme switch's own radios. Before the fix,
bodybackground stayedrgb(255, 255, 255)whetherdata-cm-themewas absent,dark, orlight. After, it tracks the token in both directions (oklch(0.214 0.01 260)dark,oklch(0.978 0.005 260)light).Bug 4 (found while fixing #3): the CSS bundle silently drops components
Switching
main.tsto importstyles.css— the "everything" bundle — surfaced thatstyles.cssnever@importedicon.cssortheme-switch.css(pre-existing), and was also missingapp-shell.cssanddocument-layout.css(introduced earlier this same round of fixes). A spot-check test listed a subset of imports by name, so a missing component never failed anything. Added a test that enumerates the builtcomponents/directory and asserts every file is reachable fromstyles.css.That exposed the same class of bug one layer down:
scripts/ci/sync-code-monster-ui-razor-assets.mjsnever vendored those four component stylesheets for PHP consumers at all —styles.cssimporting them madecheck:ui-razor-assetscorrectly fail on an undeclared@import. Added the four missing entries and re-ransync:ui-razor-assetsto regenerate the vendored assets and manifest.Bug 5 (you asked me to check everything else): nine custom-property references named tokens that don't exist
A systematic sweep of every
var(--cm-*)inpackages/css/srcagainst every custom property this package orui-tokensactually defines found nine more, in six files — none of them crashed, each silently fell back to the property's initial value:theme-switch.css:--cm-space-3xs/--cm-space-xs(gap and padding collapsed to0, which is why "Light System Dark" rendered as one unbroken run of text) and--cm-focus-ring-offset(should be--cm-focus-ring-width, matching every other component).app-shell.css/document-layout.css:--cm-space-md/--cm-space-lg(grid columns had no gap) and--cm-container-2xl(fell through to its own80remfallback instead of the real--cm-breakpoint-2xl, 96rem).nav-menu.css/admin-layout.css:--cm-motion-easing-standard— the real token is--cm-motion-ease-standard(every other component's transitions use it); four transitions were instant instead of eased.nav-menu.css/menu-bar.css/stepper.css:--cm-color-text-tertiary— there's no "tertiary" step in the text-color scale, only primary/secondary/muted/disabled/placeholder.--cm-color-text-mutedis what this codebase already uses for the same kind of de-emphasized text elsewhere.admin-layout.css:--cm-color-background-overlayfor the mobile sidebar scrim — the real token is--cm-color-background-backdrop, the same onedialog.css/drawer.cssuse for their own backdrops.Also gave the App Shell demo's collapse button real classes (
cm-button cm-button--ghost cm-button--sm) — it was a bare<button>with none, rendering as the browser's default gray/outset control instead of anything from this design system.Added
scripts/ci/css-token-references.test.mjs: it extracts every custom propertyui-css/ui-tokensdefine, then asserts everyvar(--cm-*)reference without a fallback names one of them — the check that would have caught all nine on its own. It lives at the repo root rather than insidepackages/css/__tests__because it reads a sibling package'sdist;packages/css's own workspacetestscript only builds itself, andscripts/ci/test-without-dist.sh(the pre-push hook's isolation check) wipes every workspace'sdistand runs each one's tests independently, so it has to run where cross-package reads are expected. Wired ascheck:css-token-references, appended tocheck:ui-contractsnext tocheck:ui-razor-assetsfor the same reason.None of the six touched components are covered by the frozen
vueforge-cross-platformvisual baseline (checkedcontracts/cross-platform-visual-baselines.json's 12 case IDs directly), so there's nothing there to re-approve.Bug 6 (you asked about Admin Layout specifically): every layout demo filled the whole viewport
Every layout demo's
.ui-showcase__frame(App shell, Document layout, Admin layout, Admin shell, Setup layout) rendered at exactly815px— the browser viewport'sinnerHeight(813px) to the pixel. Each of those five layout components setsmin-block-size: 100dvb/100vhon its own root, correct for what they actually are (a full page); the frame gave them nothing to be preview-sized against, sooverflow: hiddenandresize: verticalhad no starting size to clip or resize from — each layout just grew to fill the viewport.This isn't a regression from VueForge: the page's own intro text says these layouts are shown here specifically because the frozen VueForge showcase never had them (separate full-page routes per component there, not one scrolling page of framed previews) — so this failure mode is specific to this showcase's page architecture.
Gave
.ui-showcase__framean explicitblock-size: 20rem, and fixed itsborder-radiusreference from--cm-radius-md(doesn't exist) to the real--cm-radius-surface. Verified live over CDP: all five frames now render at320px.Verified against the real thing, throughout
Confirmed each CSS/import fix by inspecting the live dev server's HMR payload and, for the theme and spacing fixes, by scripting real clicks against the live page over CDP and reading back computed styles — not by reasoning about the CSS in the abstract.
Checks
npm run verifypasses in full, and so doessh scripts/ci/test-without-dist.sh(the pre-push hook's stricter per-workspace isolation check) run directly.Bug 7 (you asked for the demo to cover everything VueForge did): 36 components had no live demo at all
The showcase demoed 13 components/layouts — the ones with no VueForge ancestor, back when a separate frozen VueForge showcase also covered the rest. That frozen showcase is gone (sunset earlier), so the other 36 components in
@codemonster-ru/ui-vuehad no live demo anywhere in this repository: Accordion, Alert, Avatar, Badge, Breadcrumbs, Button, Card, Checkbox, CommandPalette, Container, DataTable, DatePicker, Dialog, Divider, Dropdown, Drawer, Field, Fieldset, Grid, IconButton, Input, Inline, Link, Menu, Popover, ProgressBar, ProgressSpinner, Radio, Select, Section, Skeleton, Stack, Switch, Table, Tabs, Textarea, Tooltip.Added a demo section for each, reading each component's actual prop types and templates rather than guessing —
CmField/CmFieldset's slot-scoped composition matches the pattern already documented indocs/getting-started.mdanddocs/components/fieldset.md. Grid/Inline/Stack/Container/Section needed no bespoke showcase CSS (their own stylesheets already set gap/padding/background); the few demos that need a visible box use this design system's own@codemonster-ru/ui-utilitiesclasses instead of new one-off rules, so the showcase now demonstrates that package too, for the first time.Verified every section live over CDP: zero console warnings across all 50 sections, and scripted real clicks to confirm the interactive ones actually work — Dialog/Drawer/CommandPalette open and close, Dropdown/Popover toggle
aria-expanded, Accordion expands a second item, Tooltip shows after its delay and hides on blur. That caught a real bug before it shipped: the Tabs demo initially rendered zero tabs, becauseCmTabssilently drops any item lacking either acontentstring or a matching panel slot (by design) — the demo's items had neither. Fixed and reverified.Rewrote the showcase's intro text and README, which both still described a showcase that only covered VueForge's gaps.