docs(ui): add design system guidelines#2252
docs(ui): add design system guidelines#2252cylewaitforit wants to merge 5 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Co-authored-by: Alex Savelyev <91429106+alexdln@users.noreply.github.com>
cylewaitforit
force-pushed
the
sb-design-guidelines
branch
from
9929234 to
0bd8a72
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughExtracted the Unocss theme from the config into a new exported Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/design-system-guidelines.mdx (1)
19-19: Minor: Missing period at end of sentence.📝 Suggested fix
-Text and input fields should not exceed **768px** in width +Text and input fields should not exceed **768px** in width.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 903b1231-571d-4bfb-bcbc-c29d267176b3
📒 Files selected for processing (6)
.storybook/main.tsapp/colors.mdxapp/design-system-guidelines.mdxapp/typography.mdxuno.config.tsuno.theme.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/typography.mdx (1)
37-56: Deduplicate the repeated typography scale array.The
fontSizeslist is duplicated in both largeTypesetblocks, which makes future token updates easy to miss in one place.♻️ Suggested refactor
export const wind4FontSizes = { 'xs': '0.75rem', 'sm': '0.875rem', 'base': '1rem', 'lg': '1.125rem', 'xl': '1.25rem', '2xl': '1.5rem', '3xl': '1.875rem', '4xl': '2.25rem', '5xl': '3rem', } + +export const fullTypographyScale = [ + wind4FontSizes['5xl'], // 48px + wind4FontSizes['4xl'], // 36px + wind4FontSizes['3xl'], // 30px + wind4FontSizes['2xl'], // 24px + wind4FontSizes['xl'], // 20px + wind4FontSizes['lg'], // 18px + wind4FontSizes['base'], // 16px + wind4FontSizes['sm'], // 14px + wind4FontSizes['xs'], // 12px + theme.text['2xs'].fontSize, // 11px + theme.text['3xs'].fontSize, // 10px + theme.text['4xs'].fontSize, // 9px + theme.text['5xs'].fontSize, // 8px +] @@ <Typeset fontFamily={theme.font.sans} - fontSizes={[ - wind4FontSizes['5xl'], // 48px - wind4FontSizes['4xl'], // 36px - wind4FontSizes['3xl'], // 30px - wind4FontSizes['2xl'], // 24px - wind4FontSizes['xl'], // 20px - wind4FontSizes['lg'], // 18px - wind4FontSizes['base'], // 16px - wind4FontSizes['sm'], // 14px - wind4FontSizes['xs'], // 12px - theme.text['2xs'].fontSize, // 11px - theme.text['3xs'].fontSize, // 10px - theme.text['4xs'].fontSize, // 9px - theme.text['5xs'].fontSize, // 8px - ]} + fontSizes={fullTypographyScale} fontWeight={400} sampleText="The quick brown fox jumps over the lazy dog" /> @@ <Typeset fontFamily={theme.font.mono} - fontSizes={[ - wind4FontSizes['5xl'], // 48px - wind4FontSizes['4xl'], // 36px - wind4FontSizes['3xl'], // 30px - wind4FontSizes['2xl'], // 24px - wind4FontSizes['xl'], // 20px - wind4FontSizes['lg'], // 18px - wind4FontSizes['base'], // 16px - wind4FontSizes['sm'], // 14px - wind4FontSizes['xs'], // 12px - theme.text['2xs'].fontSize, // 11px - theme.text['3xs'].fontSize, // 10px - theme.text['4xs'].fontSize, // 9px - theme.text['5xs'].fontSize, // 8px - ]} + fontSizes={fullTypographyScale} fontWeight={400} sampleText="const greeting = 'Hello, World!';" />Also applies to: 71-90
|
|
||
| ## Line Height | ||
|
|
||
| Base line height is set to **1.6** for optimal readability. This provides comfortable spacing for multi-line text blocks while maintaining density for UI components. |
There was a problem hiding this comment.
Use the standard hyphenated term “line-height”.
At Line 154, “Base line height” reads better as “Base line-height” in typography docs.
📝 Suggested wording tweak
-Base line height is set to **1.6** for optimal readability. This provides comfortable spacing for multi-line text blocks while maintaining density for UI components.
+Base line-height is set to **1.6** for optimal readability. This provides comfortable spacing for multi-line text blocks while maintaining density for UI components.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Base line height is set to **1.6** for optimal readability. This provides comfortable spacing for multi-line text blocks while maintaining density for UI components. | |
| Base line-height is set to **1.6** for optimal readability. This provides comfortable spacing for multi-line text blocks while maintaining density for UI components. |
There was a problem hiding this comment.
I have no preference on this. If others feel this is a good suggestion we can change to the hyphenated "line-height" here.
There was a problem hiding this comment.
@cylewaitforit, totally fair! The main rationale is just that line-height mirrors the CSS property name, which can make the docs feel more consistent with technical references. That said, "line height" (unhyphenated) is equally valid prose — it's a genuinely minor point. Happy to leave it as-is if there's no strong preference from the team. 🐇
🧠 Learnings used
Learnt from: cylewaitforit
Repo: npmx-dev/npmx.dev PR: 1964
File: .storybook/preview-head.html:1-6
Timestamp: 2026-03-06T17:45:06.525Z
Learning: In the npmx.dev project, the `.storybook/preview-head.html` file sets `background-color` on `.docs-story` via the `--bg` CSS custom property to fix the Autodocs canvas background. This is an intentional workaround for a known Storybook bug (storybookjs/storybook#30928) where `addon-themes` and `parameters.docs.theme` do not apply to the individual story canvas iframes in the Autodocs tab. Removing this file causes the autodocs pages to not display correctly.
|
Can we use a separate directory for these stories? I have a feeling that this will conflict with the blog-like pages now or in the future |
Yep! I thought having it colocated would make it easier to find and I didn’t think nuxt would ever read the files since they are Could move them into the Storybook will also use the directory in the auto-naming for the sidebar. However if the preference is to keep them at the top of the sidebar with welcome, we can always override it in the |
ghostdevv
left a comment
ghostdevv
left a comment
There was a problem hiding this comment.
Could we put the new mdx files somewhere else? feels weird having them just be in the root of app. I think they're for use with storybook right? I can't run storybook locally for some reason to check 🤔
Otherwise LGTM!
Yep! I thought having it colocated would make it easier to find and I didn’t think nuxt would ever read the files since they are .mdx but can definitely move them. Could move them into the Storybook will also use the directory in the auto-naming for the sidebar. However if the preference is to keep them at the top of the sidebar with welcome, we can always override it in the main.ts or in the files themselves in the meta section. Locally storybook seems to be tripping on some import path alias in some other files. I believe unrelated to this change since I see it on the main branch. It is still building in CI though. So these changes can be seen in the storybook link in the chromatic workflow. https://698b88d5157d89f1f33a6c21-fizlefsgpu.chromatic.com/?path=/docs/guidelines |
3 participants
🔗 Linked issue
🧭 Context
Codifies design system guidelines from @alexdln into the storybook docs.
📚 Description
Adds storybook doc pages for:
Moves theme from
uno.config.tsintouno.theme.tsin order for it to be consumed in the storybook docs.