feat(shadcn): #49 add button/card/skeleton/avatar/progress#56
Closed
fray-cloud wants to merge 2 commits into
Closed
feat(shadcn): #49 add button/card/skeleton/avatar/progress#56fray-cloud wants to merge 2 commits into
fray-cloud wants to merge 2 commits into
Conversation
Handcrafted shadcn init to preserve the front/* alias and Nx layout. Scaffolding only — no call-site swaps; daisyUI plugin remains active. - apps/web/components.json — shadcn schema 1, style default, rsc true, baseColor neutral, cssVariables, aliases mapped onto front/* (components → front/new-component, ui → front/new-component/ui, utils → front/lib/utils, lib → front/lib, hooks → front/hooks), iconLibrary lucide - apps/web/src/lib/utils.ts — cn helper (clsx + tailwind-merge) - apps/web/src/new-component/ui/ directory created (empty; #49 fills) - apps/web/tailwind.config.js — darkMode ['class'], theme.extend.colors wired to hsl(var(--*)) shadcn tokens, theme.extend.borderRadius, plugins keep `require('daisyui')` and add `tailwindcss-animate` - apps/web/app/globals.css — :root + .dark CSS variables under @layer base - package.json — class-variance-authority, clsx, tailwind-merge, tailwindcss-animate, lucide-react added - nx affected -t lint test build --exclude web-e2e green (4 projects, 10 tasks) - daisyUI classes still render identically (coexistence by design through #54) Refs #48
Five shadcn/ui components written directly into apps/web/src/new-component/ui/ to match the aliases configured in #48. Standard shadcn default-style implementations using cn(), class-variance-authority (button only), and Radix primitives for Avatar and Progress. No call-site swaps — files are unreferenced until #50–#54 consume them. - button.tsx — variants (default/destructive/outline/secondary/ghost/ link), sizes (default/sm/lg/icon), asChild via @radix-ui/react-slot - card.tsx — Card + CardHeader/Title/Description/Content/Footer - skeleton.tsx — animated muted placeholder - avatar.tsx — Radix Avatar/Image/Fallback ("use client") - progress.tsx — Radix Progress with translate-based indicator ("use client") - package.json — @radix-ui/react-slot, @radix-ui/react-avatar, @radix-ui/react-progress added - nx affected -t lint test build --exclude web-e2e green Refs #49
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideAdds initial shadcn/ui-style design system primitives (button, card, skeleton, avatar, progress) plus supporting Tailwind theme tokens and utilities, wired for class-based dark mode and Radix UI, without yet replacing any call sites. Class diagram for new shadcn UI primitives and utilitiesclassDiagram
class cn {
+cn(inputs) $Returns string$
}
class Button {
+asChild boolean
+variant string
+size string
+ButtonProps
}
class buttonVariants {
+buttonVariants(options) $Returns string$
}
class Card {
+Card
}
class CardHeader {
+CardHeader
}
class CardTitle {
+CardTitle
}
class CardDescription {
+CardDescription
}
class CardContent {
+CardContent
}
class CardFooter {
+CardFooter
}
class Avatar {
+Avatar
}
class AvatarImage {
+AvatarImage
}
class AvatarFallback {
+AvatarFallback
}
class Progress {
+value number
}
class Skeleton {
+Skeleton
}
class React {
}
class RadixSlot {
}
class RadixAvatarRoot {
}
class RadixAvatarImage {
}
class RadixAvatarFallback {
}
class RadixProgressRoot {
}
class RadixProgressIndicator {
}
class clsx {
}
class twMerge {
}
cn ..> clsx : uses
cn ..> twMerge : uses
Button ..> cn : uses
Button ..> buttonVariants : uses
Button ..> React : forwardRef
Button ..> RadixSlot : uses
buttonVariants ..> React : cva_variants
Card ..> cn : uses
Card ..> React : forwardRef
CardHeader ..> cn : uses
CardHeader ..> React : forwardRef
CardTitle ..> cn : uses
CardTitle ..> React : forwardRef
CardDescription ..> cn : uses
CardDescription ..> React : forwardRef
CardContent ..> cn : uses
CardContent ..> React : forwardRef
CardFooter ..> cn : uses
CardFooter ..> React : forwardRef
Avatar ..> cn : uses
Avatar ..> React : forwardRef
Avatar ..> RadixAvatarRoot : wraps
AvatarImage ..> cn : uses
AvatarImage ..> React : forwardRef
AvatarImage ..> RadixAvatarImage : wraps
AvatarFallback ..> cn : uses
AvatarFallback ..> React : forwardRef
AvatarFallback ..> RadixAvatarFallback : wraps
Progress ..> cn : uses
Progress ..> React : forwardRef
Progress ..> RadixProgressRoot : wraps
Progress ..> RadixProgressIndicator : wraps
Skeleton ..> cn : uses
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
View your CI Pipeline Execution ↗ for commit 75ff0cb
☁️ Nx Cloud last updated this comment at |
2 tasks
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
button.tsx,classNameis being passed intobuttonVariantsas a variant (buttonVariants({ variant, size, className })); to avoid unintended behavior and keepclassNamepurely for style overrides, callcn(buttonVariants({ variant, size }), className)instead. - In
card.tsx, theforwardRefgenerics and element types are inconsistent forCardTitleandCardDescription(e.g.,HTMLDivElementwithHTMLAttributes<HTMLHeadingElement>but rendering a<div>); align the generic element types, props, and rendered tags (e.g., use<h3>/HTMLHeadingElementand<p>/HTMLParagraphElement) to avoid type mismatch and improve semantics. - In
skeleton.tsx,React.HTMLAttributes<HTMLDivElement>is used without importing React; add aReactimport (or switch to theJSX.IntrinsicElements['div']type) so the file type-checks correctly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `button.tsx`, `className` is being passed into `buttonVariants` as a variant (`buttonVariants({ variant, size, className })`); to avoid unintended behavior and keep `className` purely for style overrides, call `cn(buttonVariants({ variant, size }), className)` instead.
- In `card.tsx`, the `forwardRef` generics and element types are inconsistent for `CardTitle` and `CardDescription` (e.g., `HTMLDivElement` with `HTMLAttributes<HTMLHeadingElement>` but rendering a `<div>`); align the generic element types, props, and rendered tags (e.g., use `<h3>`/`HTMLHeadingElement` and `<p>`/`HTMLParagraphElement`) to avoid type mismatch and improve semantics.
- In `skeleton.tsx`, `React.HTMLAttributes<HTMLDivElement>` is used without importing React; add a `React` import (or switch to the `JSX.IntrinsicElements['div']` type) so the file type-checks correctly.
## Individual Comments
### Comment 1
<location path="apps/web/src/new-component/ui/card.tsx" line_range="32-8" />
<code_context>
+));
+CardHeader.displayName = 'CardHeader';
+
+const CardTitle = React.forwardRef<
+ HTMLDivElement,
+ React.HTMLAttributes<HTMLHeadingElement>
+>(({ className, ...props }, ref) => (
+ <div
+ ref={ref}
</code_context>
<issue_to_address>
**issue:** Align CardTitle element type with its props typing for better semantics and type safety.
CardTitle’s props are typed as `React.HTMLAttributes<HTMLHeadingElement>` but it renders a `<div>` and uses `HTMLDivElement` as the ref. This inconsistency can cause misleading typing (e.g., heading-only attributes) and less semantic HTML. Please either render a heading element (`<h2>`/`<h3>`) and update the ref accordingly, or change the props to `React.HTMLAttributes<HTMLDivElement>` so the element and types match.
</issue_to_address>
### Comment 2
<location path="apps/web/src/new-component/ui/card.tsx" line_range="47-8" />
<code_context>
+));
+CardTitle.displayName = 'CardTitle';
+
+const CardDescription = React.forwardRef<
+ HTMLDivElement,
+ React.HTMLAttributes<HTMLParagraphElement>
+>(({ className, ...props }, ref) => (
+ <div
+ ref={ref}
</code_context>
<issue_to_address>
**issue:** Fix CardDescription props/ref typing to match the rendered element.
This renders a `<div>` but uses `React.HTMLAttributes<HTMLParagraphElement>` while the ref is `HTMLDivElement`, which is inconsistent. Please either switch the rendered element to `<p>` and update the ref type, or change the props type to use `HTMLDivElement` so everything matches.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const Card = React.forwardRef< | ||
| HTMLDivElement, | ||
| React.HTMLAttributes<HTMLDivElement> | ||
| >(({ className, ...props }, ref) => ( |
There was a problem hiding this comment.
issue: Align CardTitle element type with its props typing for better semantics and type safety.
CardTitle’s props are typed as React.HTMLAttributes<HTMLHeadingElement> but it renders a <div> and uses HTMLDivElement as the ref. This inconsistency can cause misleading typing (e.g., heading-only attributes) and less semantic HTML. Please either render a heading element (<h2>/<h3>) and update the ref accordingly, or change the props to React.HTMLAttributes<HTMLDivElement> so the element and types match.
| const Card = React.forwardRef< | ||
| HTMLDivElement, | ||
| React.HTMLAttributes<HTMLDivElement> | ||
| >(({ className, ...props }, ref) => ( |
There was a problem hiding this comment.
issue: Fix CardDescription props/ref typing to match the rendered element.
This renders a <div> but uses React.HTMLAttributes<HTMLParagraphElement> while the ref is HTMLDivElement, which is inconsistent. Please either switch the rendered element to <p> and update the ref type, or change the props type to use HTMLDivElement so everything matches.
Owner
Author
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
1 participant
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.
Summary
Stacked on #55. Five shadcn/ui components written directly into
apps/web/src/new-component/ui/(matches the alias configured in #48). Standarddefault-style implementations.button.tsx— variants (default/destructive/outline/secondary/ghost/link), sizes (default/sm/lg/icon),asChildvia@radix-ui/react-slot,class-variance-authoritycard.tsx—Card+CardHeader/CardTitle/CardDescription/CardContent/CardFooterskeleton.tsx—animate-pulse bg-mutedplaceholderavatar.tsx— RadixAvatar/Image/Fallback("use client")progress.tsx— RadixProgresswithtranslate-Xindicator ("use client")@radix-ui/react-slot,@radix-ui/react-avatar,@radix-ui/react-progressNo call-site swaps — files are unreferenced until #50–#54 consume them. daisyUI still live.
Closes #49.
Test plan
npx nx affected -t lint test build --exclude web-e2egreen🤖 Generated with Claude Code
Summary by Sourcery
Add initial shadcn-style UI foundation and components for the web app.
New Features:
Enhancements:
Build: