feat(shadcn): #52 swap search AnimalCard/Progress/Form Button#59
Closed
fray-cloud wants to merge 5 commits into
Closed
feat(shadcn): #52 swap search AnimalCard/Progress/Form Button#59fray-cloud wants to merge 5 commits into
fray-cloud wants to merge 5 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
…50) Header: - Drops daisyUI navbar/bg-neutral/text-neutral-content/btn-ghost and rebuilds as a plain Tailwind flex row using shadcn Button variant="ghost" for the title link - bg-neutral-900 / text-neutral-100 token pair preserves the dark contrast originally supplied by daisyUI's neutral theme tokens Sidebar: - Deleted. The component was pulled only via the new-component/ barrel export and rendered nowhere (no consumers after the catch-all bridge and SiteRouter were removed in #35). - new-component/index.ts drops `export * from './sidebar'`. - nx affected -t lint test build --exclude web-e2e green - daisyUI plugin still present; coexists with shadcn through #54 Refs #50
Count.tsx: - daisyUI `avatar` + nested `div.skeleton` → shadcn Avatar/AvatarImage and Skeleton. Loading state is now an explicit Skeleton branch instead of a class toggle, so there's no more empty .avatar box while loading. - The two text rows (name, totalCount) likewise use Skeleton during isLoading. - "use client" added (was missing; Count uses hooks). Contents.tsx: - `<button className="btn btn-link btn-primary">` → shadcn <Button variant="link">. router.push wiring unchanged. - nx affected -t lint test build --exclude web-e2e green - daisyUI plugin still present Refs #51
AnimalCard.tsx: - daisyUI `card bg-base-100 shadow-xl` + `card-body` → shadcn Card + CardContent - `"use client"` added (was missing; useLike is a Zustand hook) - Inner `<table className="table table-sm">` rewritten without daisyUI table classes — plain <table> with Tailwind padding/ font utilities SearchView.tsx: - react-infinite-scroller loader `<progress className="progress"/>` → shadcn <Progress className="h-2" /> Form.tsx: - Submit `<button className="btn btn-sm">` → shadcn <Button size="sm" type="submit"> - nx affected -t lint test build --exclude web-e2e green - daisyUI plugin still present Refs #52
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideMigrates key web UI pieces from daisyUI/native elements to shadcn-style Radix-based components, wires up supporting Tailwind theme tokens/utilities, and replaces ad‑hoc skeleton/loading UIs with reusable primitives. Class diagram for new shadcn-style UI primitivesclassDiagram
class cn {
+cn(inputs) string
}
class ButtonVariants {
<<utility>>
+buttonVariants(options) string
}
class ButtonProps {
+boolean asChild
+string variant
+string size
+string className
}
class Button {
<<ReactComponent>>
+render(props ButtonProps)
}
class Card {
<<ReactComponent>>
+render(className, children, otherProps)
}
class CardHeader {
<<ReactComponent>>
+render(className, children, otherProps)
}
class CardTitle {
<<ReactComponent>>
+render(className, children, otherProps)
}
class CardDescription {
<<ReactComponent>>
+render(className, children, otherProps)
}
class CardContent {
<<ReactComponent>>
+render(className, children, otherProps)
}
class CardFooter {
<<ReactComponent>>
+render(className, children, otherProps)
}
class Avatar {
<<ReactComponent>>
+render(className, children, otherProps)
}
class AvatarImage {
<<ReactComponent>>
+render(className, otherProps)
}
class AvatarFallback {
<<ReactComponent>>
+render(className, children, otherProps)
}
class Progress {
<<ReactComponent>>
+render(className, value, otherProps)
}
class Skeleton {
<<ReactComponent>>
+render(className, otherProps)
}
%% dependencies on cn
Button ..> cn : uses
Card ..> cn : uses
CardHeader ..> cn : uses
CardTitle ..> cn : uses
CardDescription ..> cn : uses
CardContent ..> cn : uses
CardFooter ..> cn : uses
Avatar ..> cn : uses
AvatarImage ..> cn : uses
AvatarFallback ..> cn : uses
Progress ..> cn : uses
Skeleton ..> cn : uses
%% button variants utility
Button ..> ButtonVariants : uses
%% group card subcomponents
Card "1" o-- "*" CardHeader
Card "1" o-- "*" CardTitle
Card "1" o-- "*" CardDescription
Card "1" o-- "*" CardContent
Card "1" o-- "*" CardFooter
%% avatar subcomponents
Avatar "1" o-- "*" AvatarImage
Avatar "1" o-- "*" AvatarFallback
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 773b656
☁️ Nx Cloud last updated this comment at |
1 task
![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 the new
CardTitleandCardDescriptioncomponents, the generic element types (HTMLDivElement) don’t match the HTML attributes (HTMLHeadingElement/HTMLParagraphElement), and they render<div>s instead of semantic headings/paragraphs; consider aligning the types and DOM elements (e.g.,h3/p) for correctness and accessibility. - The custom
Progresscomponent is used as the infinite scroll loader without avalue, which makes the indicator always render at 0% width; if you want a visible loading state, either provide a value or add an indeterminate/animated variant for the loader case. - In
CountList, each item still usesuuidv4()as the React key, which changes on every render; consider using a stable identifier fromresult.data(e.g.,sido.orgCd) to avoid unnecessary remounting and skeleton flicker.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the new `CardTitle` and `CardDescription` components, the generic element types (`HTMLDivElement`) don’t match the HTML attributes (`HTMLHeadingElement`/`HTMLParagraphElement`), and they render `<div>`s instead of semantic headings/paragraphs; consider aligning the types and DOM elements (e.g., `h3`/`p`) for correctness and accessibility.
- The custom `Progress` component is used as the infinite scroll loader without a `value`, which makes the indicator always render at 0% width; if you want a visible loading state, either provide a value or add an indeterminate/animated variant for the loader case.
- In `CountList`, each item still uses `uuidv4()` as the React key, which changes on every render; consider using a stable identifier from `result.data` (e.g., `sido.orgCd`) to avoid unnecessary remounting and skeleton flicker.
## Individual Comments
### Comment 1
<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 (bug_risk):** The `CardDescription` generics also mix element types and should be aligned with the rendered element.
`CardDescription` mirrors the `CardTitle` issue: the ref is typed as `HTMLDivElement`, props as `React.HTMLAttributes<HTMLParagraphElement>`, but the component renders a `<div>`. Please either render a `<p>` and keep `HTMLParagraphElement`, or keep the `<div>` and change the props to `React.HTMLAttributes<HTMLDivElement>` so the types match the actual element.
</issue_to_address>
### Comment 2
<location path="apps/web/src/new-component/ui/progress.tsx" line_range="21-22" />
<code_context>
+ {...props}
+ >
+ <ProgressPrimitive.Indicator
+ className="h-full w-full flex-1 bg-primary transition-all"
+ style={{ transform: `translateX(-${100 - (value || 0)}%)` }}
+ />
+ </ProgressPrimitive.Root>
</code_context>
<issue_to_address>
**suggestion:** The `Progress` indicator defaults to an empty bar when no value is passed, which may not match its current usage.
Because the transform uses `value || 0`, an undefined `value` becomes `translateX(-100%)`, fully hiding the indicator. In `SearchView`, `<Progress className="h-2" />` is rendered without `value`, so users get no visible loading state. Please either default `value` to `100` for this case or introduce an explicit indeterminate/animated variant for no-`value` usage.
Suggested implementation:
```typescript
>(({ className, value = 100, ...props }, ref) => (
```
```typescript
className="h-full w-full flex-1 bg-primary transition-all"
style={{ transform: `translateX(-${100 - value}%)` }}
```
</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 (bug_risk): The CardDescription generics also mix element types and should be aligned with the rendered element.
CardDescription mirrors the CardTitle issue: the ref is typed as HTMLDivElement, props as React.HTMLAttributes<HTMLParagraphElement>, but the component renders a <div>. Please either render a <p> and keep HTMLParagraphElement, or keep the <div> and change the props to React.HTMLAttributes<HTMLDivElement> so the types match the actual element.
Comment on lines
+21
to
+22
| className="h-full w-full flex-1 bg-primary transition-all" | ||
| style={{ transform: `translateX(-${100 - (value || 0)}%)` }} |
There was a problem hiding this comment.
suggestion: The Progress indicator defaults to an empty bar when no value is passed, which may not match its current usage.
Because the transform uses value || 0, an undefined value becomes translateX(-100%), fully hiding the indicator. In SearchView, <Progress className="h-2" /> is rendered without value, so users get no visible loading state. Please either default value to 100 for this case or introduce an explicit indeterminate/animated variant for no-value usage.
Suggested implementation:
>(({ className, value = 100, ...props }, ref) => ( className="h-full w-full flex-1 bg-primary transition-all"
style={{ transform: `translateX(-${100 - value}%)` }}
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 #58.
AnimalCard.tsx
card bg-base-100 shadow-xl+card-body→ shadcnCard+CardContent"use client"added (was missing;useLikeis a Zustand hook)table table-sm hoverclasses; plain<table>with Tailwind padding +font-mediumSearchView.tsx
react-infinite-scrollerloader<progress className="progress"/>→ shadcn<Progress className="h-2" />Form.tsx
<button className="btn btn-sm">→ shadcn<Button size="sm" type="submit">Closes #52.
Test plan
npx nx affected -t lint test build --exclude web-e2egreen🤖 Generated with Claude Code
Summary by Sourcery
Adopt shadcn-style UI primitives and theming across the web app and replace existing daisyUI elements in search and header views.
New Features:
cnutility for merging Tailwind class names.Enhancements:
Build:
Chores: