Card#41
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a foundational Card UI module ( Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
…ii/card rebase: Diubii/card onto main
* created cardCaption.tsx * Updated card-caption to use react-icons * Adjusted font sizes * Updated card-caption to use react-icons * Adjusted font sizes * chore: biome * feat: typography * aligned card.tsx between all cards * fix: homepage cards layout * chore: biome
* created cardCaption.tsx * Updated card-caption to use react-icons * Adjusted font sizes * Added path selection cards * chore: biome * Updated card-caption to use react-icons * Adjusted font sizes * Added path selection cards * chore: biome * rm: card-caption from this branch * fix: typography * fix: imports and home layout * remove: bg-background-blur
* feat: card-groups initial commit * fix: spacing between elements * chore: biome * fix: homepage layout for cards * remove: bg-background-blur * chore: biome
* feat: card-course-group initial commit * chore: biome * fix: make text and icons black * feat: added cards to homepage; feat: added bg-background-blur to the card * remove: bg-background-blur * remove: truncate class * feat: secondary variant * chore: biome * chore: biome
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/card-course-group.tsx (1)
42-57: Extract duplicated action styling into one variable.The two
CardActionbranches repeat the same class computation; extracting it will simplify future style changes.Proposed patch
export function CardCourseGroup({ @@ } & VariantProps<typeof cardCourseGroupVariants>) { + const actionClassName = cn("rounded-full p-3.75", secondary ? "bg-[`#51A2FF`]" : "bg-[`#74D4FF`]") + return ( <Card className={cn(cardCourseGroupVariants({ secondary }))}> @@ {hasWhatsapp && ( <CardAction gradient={false} - className={cn("rounded-full p-3.75", secondary ? "bg-[`#51A2FF`]" : "bg-[`#74D4FF`]")} + className={actionClassName} icon={IconWhatsApp} iconSize="normal" /> )} {hasTelegram && ( <CardAction gradient={false} - className={cn("rounded-full p-3.75", secondary ? "bg-[`#51A2FF`]" : "bg-[`#74D4FF`]")} + className={actionClassName} icon={IconTelegram} iconSize="normal" /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/card-course-group.tsx` around lines 42 - 57, The two CardAction render branches duplicate the same className computation; extract the computed class into a local variable (e.g., actionClass) before the JSX and use it for both CardAction components so future style changes are in one place. Compute actionClass using the existing cn call and the secondary prop logic (the same expression used now for className), then replace className={cn(...)} in both CardAction usages with className={actionClass}; keep all other props (gradient, icon, iconSize) unchanged and reference hasWhatsapp and hasTelegram as before.src/components/card-path-selection.tsx (1)
12-12: TODO left in component source.Line 12 still has an inline TODO (
add hover effect). If this is post-MVP work, consider opening a follow-up issue and referencing it here.If useful, I can draft the hover-state implementation and a small issue template for it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/card-path-selection.tsx` at line 12, Remove the inline TODO comment "//TODO: add hover effect" from the CardPathSelection component and either implement a minimal hover state now or replace the TODO with a one-line task reference to a created follow-up issue (e.g., "TODO: see ISSUE-123 for hover-state work") so the codebase contains no dangling todos; locate the comment in the CardPathSelection component (card-path-selection.tsx) and update the file to either implement the hover effect in the component's styles/JSX or annotate the TODO with the issue ID and a short note about scope (post-MVP).src/components/ui/card.tsx (1)
61-61: MakeiconSizeoptional in the prop type to match the default.
iconSizehas a runtime default of"normal"(line 58) but the type (line 61) requires it, making the API stricter than necessary.Proposed patch
-}: React.ComponentProps<"div"> & { icon: IconType; iconSize: "small" | "normal" | "large"; gradient?: boolean }) { +}: React.ComponentProps<"div"> & { icon: IconType; iconSize?: "small" | "normal" | "large"; gradient?: boolean }) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/card.tsx` at line 61, The prop type currently requires iconSize even though the component assigns a default of "normal"; make iconSize optional in the component's props type by changing the declaration for iconSize to iconSize?: "small" | "normal" | "large" so the runtime default (set near line 58) matches the TypeScript type—update the props object/type used in the function signature in src/components/ui/card.tsx (the iconSize prop) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ui/card.tsx`:
- Around line 71-73: The non-visual SVG that defines the gradient (the <svg>
using gradientId) is currently reachable by assistive tech; update that SVG to
be ignored by screen readers by adding aria-hidden="true" and
role="presentation" (and focusable="false" for IE/SVG compatibility) and remove
the <title> element so it won't be announced; locate the SVG in
src/components/ui/card.tsx around the gradientId usage and apply those
attributes and remove the title element.
---
Nitpick comments:
In `@src/components/card-course-group.tsx`:
- Around line 42-57: The two CardAction render branches duplicate the same
className computation; extract the computed class into a local variable (e.g.,
actionClass) before the JSX and use it for both CardAction components so future
style changes are in one place. Compute actionClass using the existing cn call
and the secondary prop logic (the same expression used now for className), then
replace className={cn(...)} in both CardAction usages with
className={actionClass}; keep all other props (gradient, icon, iconSize)
unchanged and reference hasWhatsapp and hasTelegram as before.
In `@src/components/card-path-selection.tsx`:
- Line 12: Remove the inline TODO comment "//TODO: add hover effect" from the
CardPathSelection component and either implement a minimal hover state now or
replace the TODO with a one-line task reference to a created follow-up issue
(e.g., "TODO: see ISSUE-123 for hover-state work") so the codebase contains no
dangling todos; locate the comment in the CardPathSelection component
(card-path-selection.tsx) and update the file to either implement the hover
effect in the component's styles/JSX or annotate the TODO with the issue ID and
a short note about scope (post-MVP).
In `@src/components/ui/card.tsx`:
- Line 61: The prop type currently requires iconSize even though the component
assigns a default of "normal"; make iconSize optional in the component's props
type by changing the declaration for iconSize to iconSize?: "small" | "normal" |
"large" so the runtime default (set near line 58) matches the TypeScript
type—update the props object/type used in the function signature in
src/components/ui/card.tsx (the iconSize prop) accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18c9061f-6b8d-49fc-9473-f3348ab7ba50
📒 Files selected for processing (5)
src/components/card-caption.tsxsrc/components/card-course-group.tsxsrc/components/card-course.tsxsrc/components/card-path-selection.tsxsrc/components/ui/card.tsx
|
Ci sarà da aggiungere l'hover effect di @BIA3IA |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/ui/card.tsx (1)
71-73:⚠️ Potential issue | 🟡 MinorFinish the decorative SVG accessibility cleanup.
Line 72 still includes a
<title>inside the non-visual gradient helper SVG. Since this SVG is decorative, remove the title and mark it presentational to avoid assistive-tech noise.Suggested patch
- <svg width="0" height="0" className="absolute" aria-hidden="true" focusable="false"> - <title>Icon gradient helper</title> + <svg width="0" height="0" className="absolute" aria-hidden="true" role="presentation" focusable="false"> <linearGradient id={gradientId} x1="0%" y1="100%" x2="0%" y2="0%">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/card.tsx` around lines 71 - 73, Remove the non‑visual <title> element from the decorative SVG and mark the SVG as presentational: delete the <title> line and add role="presentation" to the <svg> element (the element that uses gradientId in src/components/ui/card.tsx) while keeping aria-hidden="true" and focusable="false".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/ui/card.tsx`:
- Around line 71-73: Remove the non‑visual <title> element from the decorative
SVG and mark the SVG as presentational: delete the <title> line and add
role="presentation" to the <svg> element (the element that uses gradientId in
src/components/ui/card.tsx) while keeping aria-hidden="true" and
focusable="false".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c91ffba4-2ea5-4b78-b746-071cd3ffa9b0
📒 Files selected for processing (2)
src/components/card-course-group.tsxsrc/components/ui/card.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/card-course-group.tsx
toto04
left a comment
There was a problem hiding this comment.
due commentini tanto i componenti li ho già visti nelle singole PR
|
Okk, stasera procedo con le modifiche |
|
Fatto, scusate per il ritardo. Bisogna cambiare qualcosa riguardo la struttura della directory dei components? Vedo che Bianca sta creando nuove cartelle per i suoi, per ora io ho |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/ui/card.tsx (1)
44-53: Use heading semantics forCardTitleinstead of a genericdiv.
CardTitleis a structural title component; using a heading element improves navigation for assistive tech and document outline quality.Proposed refactor
-function CardTitle({ gradient = true, className, ...props }: React.ComponentProps<"div"> & { gradient?: boolean }) { +function CardTitle({ gradient = true, className, ...props }: React.ComponentProps<"h3"> & { gradient?: boolean }) { return ( - <div + <h3 data-slot="card-title" className={cn( `${gradient ? "bg-linear-to-b from-blue-secondary to-blue-primary bg-clip-text text-transparent" : ""} font-medium text-[1.5rem] leading-snug group-data-[size=sm]/card:text-base`, className )} {...props} - /> + /> ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/card.tsx` around lines 44 - 53, CardTitle currently renders a non-semantic div; change it to render a semantic heading (e.g., <h3>) to improve accessibility and document outline. Update the component signature from React.ComponentProps<"div"> to React.ComponentProps<"h3"> (or accept an `as` prop if you need flexible heading levels), replace the <div data-slot="card-title"> with the chosen heading element while preserving className, gradient logic, and spreading {...props}, and ensure any callers' props align with the new heading prop types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ui/card.tsx`:
- Around line 16-27: The Card root (<Glass ...>) lacks positioning so
CardHoverBackground (which uses absolute inset-0) won't anchor to it; update the
className passed to Glass in the Card component to include "relative" (ensure
the cn(...) string for the Glass element includes "relative") so
CardHoverBackground is positioned correctly relative to Card.
---
Nitpick comments:
In `@src/components/ui/card.tsx`:
- Around line 44-53: CardTitle currently renders a non-semantic div; change it
to render a semantic heading (e.g., <h3>) to improve accessibility and document
outline. Update the component signature from React.ComponentProps<"div"> to
React.ComponentProps<"h3"> (or accept an `as` prop if you need flexible heading
levels), replace the <div data-slot="card-title"> with the chosen heading
element while preserving className, gradient logic, and spreading {...props},
and ensure any callers' props align with the new heading prop types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b78a5cd5-14f3-419c-a53d-fa3182089ef6
📒 Files selected for processing (3)
src/components/card-caption.tsxsrc/components/card-path-selection.tsxsrc/components/ui/card.tsx
✅ Files skipped from review due to trivial changes (2)
- src/components/card-path-selection.tsx
- src/components/card-caption.tsx
|
Correggo i nomi dei commit perché mi ha tagliato via le parole dentro l'accento dei codeblock (`)
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/ui/card.tsx (1)
61-90: RemovechildrenfromCardActionprops type to prevent silent drops.
React.ComponentProps<"div">allowschildren, but this component always renders its own SVG and icon children, so caller-provided children would be silently ignored. All current usages are self-closing or empty tags, so tightening the API would have no impact but would clarify intent and prevent future misuse.Proposed fix
+type CardActionProps = Omit<React.ComponentProps<"div">, "children"> & { + icon: IconType + iconSize?: "small" | "normal" | "large" + gradient?: boolean +} + function CardAction({ className, icon: Icon, iconSize = "normal", gradient = true, ...props -}: React.ComponentProps<"div"> & { icon: IconType; iconSize?: "small" | "normal" | "large"; gradient?: boolean }) { +}: CardActionProps) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/card.tsx` around lines 61 - 90, The CardAction component's props type currently extends React.ComponentProps<"div"> which includes children and allows callers to pass children that will be ignored; update the CardAction props to exclude children by replacing the props type (used in the CardAction signature) with a custom type that omits "children" (e.g., Omit<React.ComponentProps<"div">, "children"> & { icon: IconType; iconSize?: "small" | "normal" | "large"; gradient?: boolean }) so the function CardAction no longer accepts children and intent is explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ui/card.tsx`:
- Around line 44-53: Change the props type for CardTitle to use the correct h3
intrinsic element instead of div: update the signature from
React.ComponentProps<"div"> & { gradient?: boolean } to
React.ComponentProps<"h3"> & { gradient?: boolean } (and if CardTitle
accepts/forwards refs, ensure any forwardRef or ref types are also updated to
HTMLHeadingElement). This fixes the intrinsic prop and ref type mismatch for the
CardTitle component.
---
Nitpick comments:
In `@src/components/ui/card.tsx`:
- Around line 61-90: The CardAction component's props type currently extends
React.ComponentProps<"div"> which includes children and allows callers to pass
children that will be ignored; update the CardAction props to exclude children
by replacing the props type (used in the CardAction signature) with a custom
type that omits "children" (e.g., Omit<React.ComponentProps<"div">, "children">
& { icon: IconType; iconSize?: "small" | "normal" | "large"; gradient?: boolean
}) so the function CardAction no longer accepts children and intent is explicit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 72d59b7e-5306-41e7-929c-9a866526c02f
📒 Files selected for processing (2)
src/components/card-caption.tsxsrc/components/ui/card.tsx
✅ Files skipped from review due to trivial changes (1)
- src/components/card-caption.tsx
Closes #14