docs: expand registry documentation pages#342
Conversation
Review — 1 finding (1 blocking)BLOCKING
WARN
VERIFIED CLEAN
VALIDATION
Authorship note: this PR is authored by the authenticated account, so if GitHub refuses a formal |
|
|
||
| ## Tailwind setup | ||
|
|
||
| Make sure your app imports the shared CSS variables and scans the files where copied components live. The exact Tailwind setup depends on your app, but the minimum shape is: |
There was a problem hiding this comment.
Blocking: this installation section does not include the actual VLLNT UI styling contract. A user can follow the package path above (pnpm add @vllnt/ui, import Button) and then only apply @import "tailwindcss";, but the package exports @vllnt/ui/styles.css and @vllnt/ui/tailwind-preset, and existing docs/architecture describe the preset as what maps VLLNT semantic variables to Tailwind tokens. Please split package vs registry setup and show the required CSS/preset/content-scanning guidance (or clearly mark the @import "tailwindcss"; snippet as Tailwind v4-only and include the Tailwind 3 equivalent), otherwise the public install docs can produce unstyled/missing semantic utilities for consumers.
|
@bntvllnt Review update for PR #342 (head c2128c3): no source-level blocking findings found on this pass. What I re-checked:
Validation run locally on this pass:
Existing PR/CI evidence:
Remaining merge gate:
Verdict: code/docs review is clean; merge readiness depends on clearing the canceled Vercel deployment status. |
bntvllnt
left a comment
There was a problem hiding this comment.
@bntvllnt Review verdict: REQUEST_CHANGES recommended — changelog blocker
Head reviewed: c2128c3db3fd02f22a960c7c11bab0b4d5ab9393
BLOCKING
- The PR adds user-facing public docs pages/routes but does not update
packages/ui/CHANGELOG.mdunder[Unreleased].docs/agents/PR_PLAYBOOK.mdrequires a changelog entry for user-facing changes.
WARNING
- The new docs detail route renders a page-level
<h1>while each new MDX page also starts with its own# ...heading, producing duplicate top-level headings on detail pages. Not necessarily a functional blocker, but worth cleaning up for page structure/SEO/accessibility.
STATUS
- GitHub Actions/code checks are green and
Vercel – storybookis SUCCESS. Vercel – ui.vllnt.ai: FAILUREis eligible for the PM stale-Vercel reviewer-triage waiver only; it is not merge/release approval.- Validation gap: I did not rerun full local workspace gates; this review used the live GitHub status rollup plus changed-file inspection.
| { label: frontmatter.title }, | ||
| ]} | ||
| /> | ||
| <h1 className="text-4xl font-semibold mb-4"> |
There was a problem hiding this comment.
Warning: this route-level h1 is followed by MDX pages that also start with # ..., so each docs detail page renders duplicate top-level headings. Consider making one of them non-h1 or removing duplicate MDX headings.
|
Preview ready · pr-342-ui-registry
Inspect
|
|
@bntvllnt Review verdict: REQUEST_CHANGES recommended — changed registry files fail the repo lint rules. Head reviewed: BLOCKING
VERIFIED CLEAN
VALIDATION
|
Summary
@vllnt/ui/styles.css, use@vllnt/ui/tailwind-preset, and scan app source paths; registry installs keep copied component paths in Tailwind scanning;@vllnt/ui/themes/default.cssis variables-only and not a full stylesheet substitute.Validation
Notes:
pnpm buildstill emits the existing Next.js multiple-lockfile worktree warning.pnpm test:oncestill emits existing jsdom navigation, ShareDialog description, and InteractiveTimeline duplicate-key warnings while passing all tests.Closes #248