Conversation
|
@kaberovnikita is attempting to deploy a commit to the OrcDev Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds a new WarcraftSwitch React component (Radix-based) with size, faction and disabled variants, documentation and registry entries, a dependency on Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
content/docs/components/index.mdx (1)
53-57: Switch card is out of alphabetical order.The existing cards follow alphabetical order (Avatar → Badge → … → Skeleton → Textarea → Tooltip). "Switch" should be placed between "Skeleton" and "Textarea" to stay consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/docs/components/index.mdx` around lines 53 - 57, The "Switch" Card component instance (Card title="Switch" description="Warcraft toggle with magical glow" href="/docs/components/switch") is out of alphabetical order; move that Card element so it appears between the Card for "Skeleton" and the Card for "Textarea" in the list of component cards to restore alphabetical ordering.components/ui/warcraftcn/switch.tsx (2)
25-39: Nit: inconsistent indentation infactionvariant block.The
sizeblock (lines 20-24) is indented at 6 spaces, but thefactionblock (lines 25-39) is indented at 8 spaces. Align them for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/warcraftcn/switch.tsx` around lines 25 - 39, Align the indentation of the faction variant block to match the size block: re-indent the entire "faction" object (the lines defining none, human, orc, elf, undead) so it uses the same 6-space indentation as the "size" block; ensure the keys ("faction", "none", "human", "orc", "elf", "undead") and their string values maintain the same relative indentation and line-wrapping style as the "size" variant for consistent formatting.
84-120: React 19:forwardRefis no longer required.Since the project targets React
^19.2.4,refcan be accepted as a regular prop.forwardRefstill works, so this is non-blocking, but you could simplify the component signature when convenient.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/warcraftcn/switch.tsx` around lines 84 - 120, The component currently uses React.forwardRef; simplify by turning WarcraftSwitch into a plain function component that accepts ref as a regular prop: remove React.forwardRef and its generic, change the signature to ( { className, size, faction, label, ref, ...props }: WarcraftSwitchProps ) => { ... }, keep passing ref to SwitchPrimitive.Root, and update the WarcraftSwitchProps type to include ref?: React.Ref<React.ComponentRef<typeof SwitchPrimitive.Root>> so type-checking remains correct..gitignore (1)
23-25: Nit: inconsistent comment style.Other section comments in this file use
# <space><text>(e.g.,# deps,# misc). This one omits the space.-#idea +# idea🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 23 - 25, The comment line in .gitignore uses inconsistent style ("#idea"); update it to match other section comments by adding a space so it reads "# idea" above the .idea entry, ensuring consistent comment formatting across the file.content/docs/components/meta.json (1)
14-15: Same ordering concern — "switch" should precede "textarea" and "tooltip" alphabetically.The pages list is otherwise alphabetically sorted. Place
"switch"between"skeleton"and"textarea"for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/docs/components/meta.json` around lines 14 - 15, The list in content/docs/components/meta.json is out of alphabetical order: move the "switch" entry so it appears between "skeleton" and "textarea" (i.e., order ... "skeleton", "switch", "textarea", "tooltip") to restore consistent alphabetical sorting of the pages array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/ui/warcraftcn/switch.tsx`:
- Line 89: The outer <label> unconditionally sets "cursor-pointer", which
overrides the Switch's disabled cursor; update the wrapping label in the Switch
component so the pointer class is conditional on the control not being disabled
(e.g. use a ternary or clsx to add "cursor-pointer" only when !disabled), or
replace it with group-based styling like "group" on the label and
"group-disabled:cursor-not-allowed" on the interactive element; adjust the
label's className and/or Switch root class (the disabled prop handling in the
component) accordingly so disabled switches show the not-allowed cursor.
- Around line 99-106: The glow span never sees a data-state attribute because
only SwitchPrimitive.Root (the parent button) receives it; add the CSS "group"
utility to the SwitchPrimitive.Root element and change the span's conditional
class from data-[state=checked]:opacity-100 to
group-data-[state=checked]:opacity-100 so the span becomes visible when the
parent switch is checked (leave the rest of the span classes intact).
In `@content/docs/components/switch.mdx`:
- Around line 44-49: Update the example code block for the WarcraftSwitch usage:
fix the typo in the label prop from "Humam" to "Human" and remove the
unnecessary leading indentation in the fenced tsx block so the import and return
lines are left-aligned; locate the example referencing WarcraftSwitch and adjust
the label prop and block indentation accordingly.
In `@registry.json`:
- Around line 293-310: The registry entry for the component named "switch" is
missing the dependencies field; update the "switch" registry object (the entry
that lists files including "components/ui/warcraftcn/switch.tsx") to add a
dependencies array that includes "@radix-ui/react-switch" so consumers
installing the "Warcraft Switch" get Radix auto-installed; ensure the dependency
key and version scheme match other entries in registry.json (e.g., how "tooltip"
and "dropdown-menu" declare their Radix deps).
---
Nitpick comments:
In @.gitignore:
- Around line 23-25: The comment line in .gitignore uses inconsistent style
("#idea"); update it to match other section comments by adding a space so it
reads "# idea" above the .idea entry, ensuring consistent comment formatting
across the file.
In `@components/ui/warcraftcn/switch.tsx`:
- Around line 25-39: Align the indentation of the faction variant block to match
the size block: re-indent the entire "faction" object (the lines defining none,
human, orc, elf, undead) so it uses the same 6-space indentation as the "size"
block; ensure the keys ("faction", "none", "human", "orc", "elf", "undead") and
their string values maintain the same relative indentation and line-wrapping
style as the "size" variant for consistent formatting.
- Around line 84-120: The component currently uses React.forwardRef; simplify by
turning WarcraftSwitch into a plain function component that accepts ref as a
regular prop: remove React.forwardRef and its generic, change the signature to (
{ className, size, faction, label, ref, ...props }: WarcraftSwitchProps ) => {
... }, keep passing ref to SwitchPrimitive.Root, and update the
WarcraftSwitchProps type to include ref?: React.Ref<React.ComponentRef<typeof
SwitchPrimitive.Root>> so type-checking remains correct.
In `@content/docs/components/index.mdx`:
- Around line 53-57: The "Switch" Card component instance (Card title="Switch"
description="Warcraft toggle with magical glow" href="/docs/components/switch")
is out of alphabetical order; move that Card element so it appears between the
Card for "Skeleton" and the Card for "Textarea" in the list of component cards
to restore alphabetical ordering.
In `@content/docs/components/meta.json`:
- Around line 14-15: The list in content/docs/components/meta.json is out of
alphabetical order: move the "switch" entry so it appears between "skeleton" and
"textarea" (i.e., order ... "skeleton", "switch", "textarea", "tooltip") to
restore consistent alphabetical sorting of the pages array.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.gitignorecomponents/ui/warcraftcn/switch.tsxcontent/docs/components/index.mdxcontent/docs/components/meta.jsoncontent/docs/components/switch.mdxpackage.jsonregistry.json
54498f7 to
3b4531a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
registry.json (1)
293-311: Registry dependency lists"radix-ui"rather than the specific primitive package.The component imports directly from
@radix-ui/react-switch, but the registry entry declares"dependencies": ["radix-ui"]. While the umbrellaradix-uipackage does pull in all primitives, theradix-uilatest version (1.4.3) is a low-level UI library that re-exports all primitives — consumers who install only via the registry entry may end up with the umbrella rather than the specific scoped package that the import path resolves against. The previous review comment on this thread suggested"@radix-ui/react-switch"for precision.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@registry.json` around lines 293 - 311, Update the registry entry for the component named "switch" so its dependencies list references the exact primitive package used by the code: replace "radix-ui" with the scoped package "@radix-ui/react-switch" in the "dependencies" array (the entry that includes the component files under "components/ui/warcraftcn/switch.tsx" and "components/ui/warcraftcn/styles/warcraft.css") to match the import path used by the component.components/ui/warcraftcn/switch.tsx (1)
89-89: Outer<label>retainscursor-pointerwhen the switch is disabled.The
cursor-pointerclass is unconditional on the<label>, overriding thedisabled:cursor-not-allowedset onSwitchPrimitive.Root. A disabled switch should show the not-allowed cursor across the entire interactive area.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/warcraftcn/switch.tsx` at line 89, The outer <label> currently has an unconditional "cursor-pointer" which overrides the "disabled:cursor-not-allowed" on SwitchPrimitive.Root; update the label in the Switch component to apply cursor classes conditionally (mirror the disabled prop) instead of always using "cursor-pointer" — e.g., use the component's disabled prop (from Switch or SwitchProps) and set the label's className to include "cursor-not-allowed" when disabled and "cursor-pointer" when not (or use clsx) so the label respects SwitchPrimitive.Root's disabled state.
🧹 Nitpick comments (2)
.gitignore (1)
23-23: Nit: comment is missing a space after#
#ideais a valid but oddly formatted comment. For consistency with the rest of the file, it should be# ideaor# JetBrains IDE.✏️ Proposed fix
-#idea +# idea🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore at line 23, The comment line "#idea" in .gitignore is missing a space after the hash; update that comment to follow the file's style (e.g., change "#idea" to "# idea" or a clearer label like "# JetBrains IDE") so the comment formatting is consistent with the rest of the file.components/ui/warcraftcn/switch.tsx (1)
25-39: Inconsistent indentation insideswitchVariants.The
factionkey (Line 25) is indented with 8 spaces whilesizeuses 6, and the faction values are indented with 12 spaces. This doesn't affect runtime behaviour but breaks the visual alignment of the CVA config.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/warcraftcn/switch.tsx` around lines 25 - 39, Indentation in the switchVariants CVA object is inconsistent: the "faction" key and its values are misaligned compared to "size"; update the indentation of the "faction" key and each of its value lines (human, orc, elf, undead) to match the project's existing style used for "size" so the object keys and multi-line string values align consistently within the switchVariants declaration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/ui/warcraftcn/switch.tsx`:
- Around line 99-106: The glow span is always visible because animate-pulse
overrides opacity-0; gate the animation to only run when the switch is checked
by moving the animate-pulse class behind the group-data selector. Update the
span in components/ui/warcraftcn/switch.tsx so it uses
group-data-[state=checked]:animate-pulse (and keep
group-data-[state=checked]:opacity-100) instead of a global animate-pulse,
ensuring opacity-0 hides the glow when unchecked.
---
Duplicate comments:
In `@components/ui/warcraftcn/switch.tsx`:
- Line 89: The outer <label> currently has an unconditional "cursor-pointer"
which overrides the "disabled:cursor-not-allowed" on SwitchPrimitive.Root;
update the label in the Switch component to apply cursor classes conditionally
(mirror the disabled prop) instead of always using "cursor-pointer" — e.g., use
the component's disabled prop (from Switch or SwitchProps) and set the label's
className to include "cursor-not-allowed" when disabled and "cursor-pointer"
when not (or use clsx) so the label respects SwitchPrimitive.Root's disabled
state.
In `@registry.json`:
- Around line 293-311: Update the registry entry for the component named
"switch" so its dependencies list references the exact primitive package used by
the code: replace "radix-ui" with the scoped package "@radix-ui/react-switch" in
the "dependencies" array (the entry that includes the component files under
"components/ui/warcraftcn/switch.tsx" and
"components/ui/warcraftcn/styles/warcraft.css") to match the import path used by
the component.
---
Nitpick comments:
In @.gitignore:
- Line 23: The comment line "#idea" in .gitignore is missing a space after the
hash; update that comment to follow the file's style (e.g., change "#idea" to "#
idea" or a clearer label like "# JetBrains IDE") so the comment formatting is
consistent with the rest of the file.
In `@components/ui/warcraftcn/switch.tsx`:
- Around line 25-39: Indentation in the switchVariants CVA object is
inconsistent: the "faction" key and its values are misaligned compared to
"size"; update the indentation of the "faction" key and each of its value lines
(human, orc, elf, undead) to match the project's existing style used for "size"
so the object keys and multi-line string values align consistently within the
switchVariants declaration.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.gitignorecomponents/ui/warcraftcn/switch.tsxcontent/docs/components/index.mdxcontent/docs/components/meta.jsoncontent/docs/components/switch.mdxpackage.jsonregistry.json
✅ Files skipped from review due to trivial changes (1)
- content/docs/components/switch.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- content/docs/components/index.mdx
3b4531a to
afcc544
Compare
afcc544 to
77cfc96
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/ui/warcraftcn/switch.tsx (1)
94-94: Labelcursor-pointerstill overridesdisabled:cursor-not-allowedon the switch.The wrapping
<label>unconditionally appliescursor-pointer, so whendisabledis passed the browser shows a pointer cursor over the label text area instead ofnot-allowed. This was flagged in a previous review and remains unaddressed.🐛 Proposed fix
- <label className="flex items-center gap-3 cursor-pointer"> + <label className={cn("flex items-center gap-3", props.disabled ? "cursor-not-allowed" : "cursor-pointer")}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/warcraftcn/switch.tsx` at line 94, The wrapping <label> in the Switch component currently hardcodes "cursor-pointer" which overrides the disabled state; update the label's className to apply "cursor-pointer" only when the component/prop is not disabled (or alternatively apply "disabled:cursor-not-allowed" and remove the unconditional "cursor-pointer"), referencing the Switch component's label element and the disabled prop so the browser shows "not-allowed" when disabled; ensure any utility class logic (e.g., conditional class concat in the Switch component) reflects this change.
🧹 Nitpick comments (1)
components/ui/warcraftcn/switch.tsx (1)
78-83: ConsiderReact.ComponentPropsinstead ofComponentPropsWithoutRef+ manualref.In React 19,
refis a regular prop andReact.ComponentProps<T>already includes it, so the manualref?field on the interface is redundant.♻️ Proposed simplification
export interface WarcraftSwitchProps - extends React.ComponentPropsWithoutRef<typeof SwitchPrimitive.Root>, + extends React.ComponentProps<typeof SwitchPrimitive.Root>, VariantProps<typeof switchVariants> { label?: string - ref?: React.Ref<React.ComponentRef<typeof SwitchPrimitive.Root>> }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/warcraftcn/switch.tsx` around lines 78 - 83, The WarcraftSwitchProps interface currently extends React.ComponentPropsWithoutRef<typeof SwitchPrimitive.Root> and also adds a manual ref?: React.Ref<...>; replace the pair with a single extension using React.ComponentProps<typeof SwitchPrimitive.Root> to let React include ref automatically, i.e., extend React.ComponentProps<typeof SwitchPrimitive.Root> alongside VariantProps<typeof switchVariants>, keep the optional label, and remove the manual ref field; update any consumers of WarcraftSwitchProps if needed to rely on the built-in ref prop.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/ui/warcraftcn/switch.tsx`:
- Around line 1-4: Add the Next.js client directive by inserting "use client" as
the very first line of this file so the Radix Switch import (SwitchPrimitive)
and any components that use React context/hooks render on the client; ensure the
string literal is the first statement before imports (affecting the exported
Switch/SwitchProps components that use `@radix-ui/react-switch`).
---
Duplicate comments:
In `@components/ui/warcraftcn/switch.tsx`:
- Line 94: The wrapping <label> in the Switch component currently hardcodes
"cursor-pointer" which overrides the disabled state; update the label's
className to apply "cursor-pointer" only when the component/prop is not disabled
(or alternatively apply "disabled:cursor-not-allowed" and remove the
unconditional "cursor-pointer"), referencing the Switch component's label
element and the disabled prop so the browser shows "not-allowed" when disabled;
ensure any utility class logic (e.g., conditional class concat in the Switch
component) reflects this change.
---
Nitpick comments:
In `@components/ui/warcraftcn/switch.tsx`:
- Around line 78-83: The WarcraftSwitchProps interface currently extends
React.ComponentPropsWithoutRef<typeof SwitchPrimitive.Root> and also adds a
manual ref?: React.Ref<...>; replace the pair with a single extension using
React.ComponentProps<typeof SwitchPrimitive.Root> to let React include ref
automatically, i.e., extend React.ComponentProps<typeof SwitchPrimitive.Root>
alongside VariantProps<typeof switchVariants>, keep the optional label, and
remove the manual ref field; update any consumers of WarcraftSwitchProps if
needed to rely on the built-in ref prop.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.gitignorecomponents/ui/warcraftcn/switch.tsxcontent/docs/components/index.mdxcontent/docs/components/meta.jsoncontent/docs/components/switch.mdxpackage.jsonregistry.json
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- content/docs/components/switch.mdx
- content/docs/components/index.mdx
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/ui/warcraftcn/switch.tsx (2)
1-4: Missing"use client"directive.This was flagged in a previous review and remains unresolved.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/warcraftcn/switch.tsx` around lines 1 - 4, The file is missing the React Server Components client directive; add the string directive "use client" as the very first line of components/ui/warcraftcn/switch.tsx (before any imports) so the Switch component and imports like React and SwitchPrimitive from "@radix-ui/react-switch" are treated as client-side code.
94-94: Outer<label>unconditionally appliescursor-pointereven when the switch is disabled.This was flagged in a previous review and remains unresolved.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/warcraftcn/switch.tsx` at line 94, The outer <label> in components/ui/warcraftcn/switch.tsx unconditionally includes "cursor-pointer" which should be omitted when the switch is disabled; update the label's className in the Switch component (where the label is rendered) to conditionally include "cursor-pointer" only when the component is not disabled (e.g., use a ternary or classNames utility referencing the component's disabled prop or isDisabled state) so the cursor does not appear clickable for disabled switches.
🧹 Nitpick comments (1)
components/ui/warcraftcn/switch.tsx (1)
10-10: Preferfocus-visible:overfocus:for the focus ring.
focus:ring-2triggers the ring on every focus event including mouse clicks. Keyboard-only focus rings should usefocus-visible:to avoid the ring on mouse interaction.♻️ Proposed refactor
- "focus:outline-none focus:ring-2 focus:ring-amber-500/50", + "focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-amber-500/50",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/warcraftcn/switch.tsx` at line 10, The focus ring utility in the Switch component's class string uses focus:outline-none and focus:ring-2/focus:ring-amber-500/50 which shows the ring on mouse clicks; update those utilities to use focus-visible:outline-none and focus-visible:ring-2 with focus-visible:ring-amber-500/50 (locate the class string in components/ui/warcraftcn/switch.tsx where focus:outline-none focus:ring-2 focus:ring-amber-500/50 appears and replace the focus: variants with focus-visible:) so the ring only appears for keyboard focus.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/ui/warcraftcn/switch.tsx`:
- Around line 27-38: The checked-state gradients for the faction variants in
switchVariants (the human, orc, elf, undead variant strings) are inheriting the
base via-zinc-900 stop under Tailwind v4, so update each faction's checked-state
classes to explicitly clear the middle stop by adding
data-[state=checked]:via-transparent to the human, orc, elf and undead variant
strings in switchVariants; this ensures the checked gradient becomes a two-stop
(from → to) rather than a three-stop (from → zinc → to) under Tailwind v4.
---
Duplicate comments:
In `@components/ui/warcraftcn/switch.tsx`:
- Around line 1-4: The file is missing the React Server Components client
directive; add the string directive "use client" as the very first line of
components/ui/warcraftcn/switch.tsx (before any imports) so the Switch component
and imports like React and SwitchPrimitive from "@radix-ui/react-switch" are
treated as client-side code.
- Line 94: The outer <label> in components/ui/warcraftcn/switch.tsx
unconditionally includes "cursor-pointer" which should be omitted when the
switch is disabled; update the label's className in the Switch component (where
the label is rendered) to conditionally include "cursor-pointer" only when the
component is not disabled (e.g., use a ternary or classNames utility referencing
the component's disabled prop or isDisabled state) so the cursor does not appear
clickable for disabled switches.
---
Nitpick comments:
In `@components/ui/warcraftcn/switch.tsx`:
- Line 10: The focus ring utility in the Switch component's class string uses
focus:outline-none and focus:ring-2/focus:ring-amber-500/50 which shows the ring
on mouse clicks; update those utilities to use focus-visible:outline-none and
focus-visible:ring-2 with focus-visible:ring-amber-500/50 (locate the class
string in components/ui/warcraftcn/switch.tsx where focus:outline-none
focus:ring-2 focus:ring-amber-500/50 appears and replace the focus: variants
with focus-visible:) so the ring only appears for keyboard focus.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.gitignorecomponents/ui/warcraftcn/switch.tsxcontent/docs/components/index.mdxcontent/docs/components/meta.jsoncontent/docs/components/switch.mdxpackage.jsonregistry.json
🚧 Files skipped from review as they are similar to previous changes (5)
- package.json
- registry.json
- content/docs/components/meta.json
- content/docs/components/index.mdx
- content/docs/components/switch.mdx
|
@TheOrcDev pr ready for check |
closes #38
Added switch component. I hope you will like my ui design!
Summary by CodeRabbit
New Features
Documentation
Chores