docs(dense): restore dropped Icon and Carousel dense/zh best-practice bullets - #4676
Conversation
meta-cla
Bot
added
the
CLA Signed
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR Modified ComponentsCarousel (@astryxdesign/core) · View in Storybook
Icon (@astryxdesign/core) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: 1 accessibility violation(s) found — 1 serious. Icon - 1 issue(s)
Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
| description: 'Icons are small visual symbols that represent actions, objects, or concepts. They improve scannability and reinforce meaning alongside text. Supports both direct SVG components and semantic icon names that adapt to the active theme.', | ||
| bestPractices: [ | ||
| { guidance: true, description: 'Use semantic icon names when available; they adapt to theme changes automatically.' }, | ||
| { guidance: true, description: 'Libraries can augment the icon map with their own keys: registerIcons({\'richtext:bold\': <MyIcon />}) accepts arbitrary keys (not just IconName). Resolve via getExtendedIcon(key, fallback), which prefers a theme-registered icon and falls back to a bundled default: the seam that makes library icons theme-overridable.' }, |
There was a problem hiding this comment.
The preferred way to do this is via theme
There was a problem hiding this comment.
Good call, and the code agrees with you: registerIcons() already warnOnces that defineTheme({ icons }) is the theme-scoped way to swap a glyph, so recommending the global registry as the seam for library icons contradicted our own runtime guidance.
Rewrote both the English and dense bullets to lead with defineTheme({icons}) (scoped to the active <Theme>, shallow-merged through extends) and reframed registerIcons() as an app-bootstrap escape hatch rather than a theming seam.
I kept it to what's on main today. Once #4647 lands, the slot mechanism (componentIcons + ComponentIconSlotMap augmentation) is the better answer for the library case specifically, and I'm happy to follow up with a bullet for that then. Also worth noting the same registerIcons-as-seam prose exists in RichTextEditor.doc.mjs and in the getExtendedIcon JSDoc if you want those cleaned up too.
85f2a8b to
d57f4b5
Compare
josephfarina
changed the title
The English Icon docs gained a best-practices bullet in #4551 covering registerIcons/getExtendedIcon for library icon augmentation, but the docsDense overlay was not updated to match. Because the dense overlay merges by count and position, the missing bullet caused a silent 11-vs-10 parity gap: the dense CLI output dropped the augmentation guidance. Add the compressed dense bullet at the matching position so `astryx component Icon --lang dense` renders all 11 best practices with the same guidance flags as English. Also straighten one em dash in the English bullet to a colon per the doc prose style. Found during Night Watch doc review.
The English bestPractices gained a hasLoop bullet in #4629 but neither docsDense nor docsZh was updated, so both overlays fell one bullet short and the CLI silently dropped the hasLoop guidance under --lang dense/zh. Also backfill the pre-existing missing built-in-navigation bullet in the zh overlay. Both overlays now match the English bullet count and per-position guidance flags (8: 5 do, 3 don't). Found during Night Watch doc review.
registerIcons() warns at runtime that defineTheme({icons}) is the
theme-scoped way to swap a glyph, so documenting the global registry as
the seam for library icons contradicted the library's own guidance.
Point both the English and dense best practices at the theme, and keep
registerIcons framed as an app-bootstrap escape hatch.
Co-authored-by: Cursor <cursoragent@cursor.com>
d57f4b5 to
5c705bd
Compare
2 participants
What
Restore dropped translation-overlay best-practice bullets so
astryx component <Name> --lang dense|zhrenders the full guidance instead of silently falling back to English.Icon.doc.mjs):docsDensewas missing onebestPracticesbullet (11 English vs 10 dense) after feat(lab): add composable RichTextEditorToolbar #4551 added registerIcons/getExtendedIcon guidance to the English docs only. Restored the bullet and straightened one em dash in the English bullet to a colon.Carousel.doc.mjs): feat(carousel): add hasLoop wrap-around and handleRef imperative control #4629 added ahasLoopbullet to the EnglishbestPracticesbut not todocsDenseordocsZh, so both overlays were one bullet short and dropped the hasLoop guidance. Restored it in both. Also backfilled a pre-existing missing built-in-navigation bullet in the zh overlay so it reaches full parity.Both files now match English bullet count and per-position
guidanceflags. Verified with the dense-coverage audit (0 bullet drifts) and by rendering--lang denseand--lang zhfor both components.Checklist
pnpm --filter @astryxdesign/core typecheck:docspassestsc --project packages/cli/tsconfig.template-docs.json --noEmitpasses--lang denseoutput verified for Icon and Carousel--lang zhoutput verified for CarouselNight Watch — Doc Reviewer