doc updates #7
doc updates #7
Conversation
WalkthroughREADME.md was expanded and reorganized: the intro now emphasizes core design foundations; a large Design Tokens & Primitives section with structured token categories and responsive token taxonomy was added; a comprehensive CSS/SCSS reset example and accessibility notes were included. No code or public API exports changed. Changes
Sequence Diagram(s)(omitted) — changes are documentation-only and do not modify control flow or runtime interactions. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
29-36: Inconsistent default theme (two “Default” labels).Only one default should be marked. If Light is default, remove “(Default)” from Dark.
Apply:
-Light (Default) | `light` +Light (Default) | `light` ... -Dark (Default) | `dark` +Dark | `dark`
🧹 Nitpick comments (6)
README.md (6)
18-20: Clarify CSS import usage alongside SCSS.Provide a CSS example and a option to reduce ambiguity and discourage @import in production.
Apply:
@use '@new-ui/foundations'; // Use `@import` for CSS +/* CSS (bundlers): */ +/* @import "@new-ui/foundations"; */ + +/* Plain HTML via link: */ +/* <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@new-ui/foundations@<version>/dist/index.css"> */
46-47: Fix nested list indentation (markdownlint MD007).Use 2-space indent for sub-items.
Apply:
- - `js-` for targeting JavaScript-specific functionality. + - `js-` for targeting JavaScript-specific functionality.
131-132: Wording nit: “order” → “outline”.“Focus order” is a different concept (tab order). This token appears to describe line thickness for outlines.
Apply:
-**`--border-width-02`** | Used for the selection and focus order +**`--border-width-02`** | Used for selection and focus outline
26-27: Close the HTML snippet for completeness.Tiny clarity improvement for copy-paste.
Apply:
-<html data-new-ui-theme="light"> +<html data-new-ui-theme="light"></html>
189-190: Pin CDN version; avoid @latest for reproducibility.Docs should encourage deterministic builds.
Apply:
-// https://cdn.jsdelivr.net/npm/@new-ui/reset@latest/dist/index.css +// Prefer pinning: https://cdn.jsdelivr.net/npm/@new-ui/reset@1.2.3/dist/index.css
234-238: Minor reset enhancement: inherit line-height and color.Improves consistency with typography tokens.
Apply:
input, textarea, button, select { font-family: inherit; font-size: inherit; + line-height: inherit; + color: inherit; margin: 0; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
46-46: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (3)
README.md (3)
4-4: Strong, focused intro.Clear scope and audience; reads well.
50-121: Design tokens section is comprehensive and well-structured.Tables are scannable; taxonomy reads cleanly.
Also applies to: 128-179
180-181: Confirmed: --lh- tokens are present in the distributed CSS.*
dist/index.css defines --lh-mobile-* and --lh-desktop-* variables (e.g. --lh-desktop-body-xl), so the README note is accurate.
| html { | ||
| -moz-text-size-adjust: none; | ||
| -webkit-text-size-adjust: none; | ||
| text-size-adjust: none; | ||
| vertical-align: baseline; | ||
| } | ||
|
|
There was a problem hiding this comment.
Accessibility: avoid disabling text size adjustment; remove no-op.
text-size-adjust: none can hinder user scaling on mobile. Also vertical-align on html has no effect.
Apply:
html {
- -moz-text-size-adjust: none;
- -webkit-text-size-adjust: none;
- text-size-adjust: none;
- vertical-align: baseline;
+ /* Allow user scaling; keep defaults */
+ /* text-size-adjust: 100%; */
}📝 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.
| html { | |
| -moz-text-size-adjust: none; | |
| -webkit-text-size-adjust: none; | |
| text-size-adjust: none; | |
| vertical-align: baseline; | |
| } | |
| html { | |
| /* Allow user scaling; keep defaults */ | |
| /* text-size-adjust: 100%; */ | |
| } |
🤖 Prompt for AI Agents
In README.md around lines 195 to 201, the css block on the html selector
disables user text scaling and contains a no-op vertical-align; remove the
vendor-prefixed and unprefixed text-size-adjust: none rules (or replace with
text-size-adjust: 100% if you need to explicitly allow scaling) and delete the
vertical-align: baseline declaration so users can scale text on mobile and the
no-op property is not present.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
README.md (4)
42-47: Fix nested list indent to satisfy markdownlint (MD007).Current indent uses 4 spaces; use 2 spaces.
- - `c-` for UI components. - - `l-` for layout-related styles. - - `u-` for utilities. - - `is-` and `has-` for state-based classes. - - `js-` for targeting JavaScript-specific functionality. + - `c-` for UI components. + - `l-` for layout-related styles. + - `u-` for utilities. + - `is-` and `has-` for state-based classes. + - `js-` for targeting JavaScript-specific functionality.
18-20: Clarify SCSS vs CSS import with separate examples.The inline comment is easy to miss; show both forms explicitly.
-```scss -@use '@new-ui/foundations'; // Use `@import` for CSS -``` +```scss +// SCSS +@use '@new-ui/foundations'; +``` + +```css +/* CSS */ +@import url('@new-ui/foundations/dist/index.css'); +/* or via <link>: +<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@new-ui/foundations@latest/dist/index.css" /> +*/ +```
31-36: Two “Default” themes listed; pick one or clarify initial default.Having both Light and Dark marked “(Default)” is confusing.
- Dark (Default) | `dark` + Dark | `dark`If Dark is actually the initial default, switch the “(Default)” label accordingly.
130-132: Wording nit: likely “focus ring” (or border), not “focus order”.Assuming this describes width for focus outlines.
-**`--border-width-02`** | Used for the selection and focus order +**`--border-width-02`** | Used for selection and the focus ring
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
46-46: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (2)
README.md (2)
89-93: Typo fix confirmed:--link-visitedis correct.Looks good; resolves the earlier misspelling.
195-201: Accessibility: don’t disable text scaling; remove no-op.Blocking user zoom hurts accessibility;
vertical-alignonhtmlis a no-op.Apply:
html { - -moz-text-size-adjust: none; - -webkit-text-size-adjust: none; - text-size-adjust: none; - vertical-align: baseline; + /* Allow user scaling; keep defaults */ + /* text-size-adjust: 100%; */ }
Summary by CodeRabbit