[WEB-4687] feat: propel switch#7629
Conversation
WalkthroughAdds a new Switch component and exposes it via a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Consumer
participant PropelSwitch as Propel.Switch
participant BaseRoot as BaseSwitch.Root
participant BaseThumb as BaseSwitch.Thumb
Consumer->>PropelSwitch: render({value, onChange, label, size, disabled})
Note right of PropelSwitch #EFEFEF: compute classes via cn (size/state)
PropelSwitch->>BaseRoot: props {checked=value, disabled, aria-label=label}
PropelSwitch->>BaseRoot: onCheckedChange = onChange
BaseRoot->>BaseThumb: render thumb (position/style from value+size+disabled)
BaseRoot-->>Consumer: invokes onChange(newValue)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/propel/src/switch.tsx (2)
14-16: Forward the ref to Root to enable focus management and form integrations.This wrapper currently swallows the ref. Forwarding it is a common requirement for DS components.
Apply this diff:
-const Switch: React.FC<IToggleSwitchProps> = (props) => { +const Switch = React.forwardRef<React.ElementRef<typeof BaseSwitch.Root>, IToggleSwitchProps>( + (props, ref) => { const { value, onChange, label, size = "sm", disabled, className } = props; return ( <BaseSwitch.Root + ref={ref} checked={value} disabled={disabled} onCheckedChange={onChange} className={cn( @@ - ); -}; + ); +});Also applies to: 17-17, 50-50
22-22: Use design tokens consistently for the off state (optional).Off state uses bg-gray-700 while other colors use custom tokens. Consider swapping to a custom token for consistency with the design system.
Example:
- "relative inline-flex flex-shrink-0 cursor-pointer rounded-full border border-custom-border-200 transition-colors duration-200 ease-in-out focus:outline-none", + "relative inline-flex flex-shrink-0 cursor-pointer rounded-full border border-custom-border-200 transition-colors duration-200 ease-in-out focus:outline-none", // size... - // state... - disabled - ? "cursor-not-allowed bg-custom-background-80" - : value - ? "bg-custom-primary-100" - : "bg-gray-700", + // state... + disabled + ? "cursor-not-allowed bg-custom-background-80" + : value + ? "bg-custom-primary-100" + : "bg-custom-background-90",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
packages/propel/src/switch.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
packages/propel/src/switch.tsx (3)
1-3: Consider marking this as a Client Component.If Propel is consumed in Next.js app-router (RSC) contexts, this interactive component should be compiled as a client component to avoid runtime errors.
If applicable, add the directive at the very top:
+ "use client";Please confirm whether Propel components are already wrapped/consumed under client boundaries in the host app; if so, this can be skipped.
52-54: LGTM: displayName and export.Setting a displayName helps React DevTools, and the named export is consistent with DS patterns.
2-2: Confirm existence of Radix-style subcomponents in @base-ui-components/react/switchI ran the recommended searches in our repo and observed:
- The only import of
@base-ui-components/react/switchis inpackages/propel/src/switch.tsx, with no other occurrences elsewhere.- There are no references to
Switch.RootorSwitch.Thumbin our codebase.- Our
cnhelper is defined inpackages/utils/src/common.tsandpackages/ui/src/utils/classname.tsxas expected.Since the absence of references here doesn’t prove whether the external package actually exports
Switch.RootandSwitch.Thumb, please verify directly against the package’s API:
- Inspect
node_modules/@base-ui-components/react/switch/index.d.ts(or its source repo) to confirm those subcomponent exports.- Alternatively, check the package’s published documentation to ensure it provides
Switch.RootandSwitch.Thumb.If those subcomponents aren’t exposed, the current imports/rendering will fail at compile- or runtime.
JayashTripathy
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/propel/src/switch/root.tsx (2)
14-20: Forward ref and pass-through native props for flexibility (id, name, onBlur, etc.).Exposing a
refand spreading unhandled props toBaseSwitch.Rootimproves integration in forms and with focus management. Keep the current API but extend it.You can adapt as follows (outside the exact lines shown, sketching the approach):
type SwitchRootElement = React.ElementRef<typeof BaseSwitch.Root>; type SwitchRootProps = React.ComponentPropsWithoutRef<typeof BaseSwitch.Root>; interface SwitchProps extends Omit<SwitchRootProps, "checked" | "onCheckedChange"> { value: boolean; onChange: (value: boolean) => void; label?: string; size?: "sm" | "md" | "lg"; className?: string; } const Switch = React.forwardRef<SwitchRootElement, SwitchProps>( ({ value, onChange, label, size = "sm", disabled, className, ...rest }, ref) => ( <BaseSwitch.Root ref={ref} checked={value} disabled={disabled} onCheckedChange={onChange} aria-label={label} className={cn(/* ...existing classes... */, className)} {...rest} > {label && <span className="sr-only">{label}</span>} {/* thumb */} </BaseSwitch.Root> ) ); Switch.displayName = "plane-ui-switch"; export { Switch }; export type { SwitchProps };Also applies to: 29-31, 45-46
15-20: A11y: prefer external visible label support via aria-labelledby in addition to aria-label.Current API only accepts a string
labeland setsaria-label. Many consumers render a visible text label adjacent to the switch. Consider supportingariaLabelledBy?: stringor passing arbitrary ARIA props through...rest(see forwardRef suggestion) so consumers can bind an external label element.Also applies to: 29-31
packages/propel/package.json (1)
25-26: Ensure CSS is retained and consider exporting Switch typesWe’ve confirmed that both exported files exist under
packages/propel/srcand that nosideEffectsfield is currently set inpackages/propel/package.json(it’s reported as “MISSING”). To avoid downstream bundlers tree-shaking away your CSS, add asideEffectsentry. You can opt for a broad or narrow scope:• In
packages/propel/package.json, add:{ "sideEffects": [ "**/*.css" ] }—or, to be more specific:
{ "sideEffects": [ "./src/styles/fonts/index.css" ] }
Optional: expose Switch props as a types entry
If you’d like to make
SwitchProps(or other types) directly importable, you can add atypessubpath export. For example, inpackages/propel/package.json’sexportsblock:"exports": { "./switch": "./src/switch/index.ts", + "./switch/types": "./src/switch/types.ts" }This lets consumers import props via:
import type { SwitchProps } from 'propel/switch/types'packages/propel/src/switch/index.ts (1)
1-1: LGTM: clean re-export.Simple and conventional surface. If you later export
SwitchProps, consider addingexport type { SwitchProps } from "./root";.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
apps/web/core/components/core/modals/existing-issues-list-modal.tsx(1 hunks)packages/propel/package.json(1 hunks)packages/propel/src/combobox/combobox.tsx(1 hunks)packages/propel/src/command/command.tsx(1 hunks)packages/propel/src/switch/index.ts(1 hunks)packages/propel/src/switch/root.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/propel/src/command/command.tsx
- packages/propel/src/combobox/combobox.tsx
- apps/web/core/components/core/modals/existing-issues-list-modal.tsx
🔇 Additional comments (1)
packages/propel/src/switch/root.tsx (1)
48-50: LGTM on displayName and export.Display name is helpful for DevTools; the named export matches the package export surface.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/propel/src/command/command.tsx (3)
7-7: Remove the no-op empty string in cn callcn("", className) is equivalent to cn(className) and adds noise.
Apply this diff:
- return <CommandPrimitive data-slot="command" className={cn("", className)} {...props} />; + return <CommandPrimitive data-slot="command" className={cn(className)} {...props} />;
22-24: Pass through className to List/Empty/Item (currently destructured but unused)className is being pulled off props and then dropped, which prevents consumers from styling these subcomponents and may trigger lint warnings.
Apply this diff:
function CommandList({ className, ...props }: React.ComponentProps<typeof CommandPrimitive.List>) { - return <CommandPrimitive.List data-slot="command-list" {...props} />; + return <CommandPrimitive.List data-slot="command-list" className={className} {...props} />; } function CommandEmpty({ ...props }: React.ComponentProps<typeof CommandPrimitive.Empty>) { - return <CommandPrimitive.Empty data-slot="command-empty" {...props} />; + return <CommandPrimitive.Empty data-slot="command-empty" {...props} />; } function CommandItem({ className, ...props }: React.ComponentProps<typeof CommandPrimitive.Item>) { - return <CommandPrimitive.Item data-slot="command-item" {...props} />; + return <CommandPrimitive.Item data-slot="command-item" className={className} {...props} />; }If styling should intentionally be locked down, remove className from the destructuring to avoid confusion instead.
Also applies to: 26-28, 30-32
4-4: Migratecnto@plane/utils— confirmed, address redundantcn("", className)patterns
- No lingering
import { cn } from "@plane/ui"inpackages/propel(✓).@plane/utils(inpackages/utils/src/common.ts) exportsmatching the prior signature (✓).export const cn = (...inputs: ClassValue[]) => twMerge(clsx(inputs));- Detected redundant usages where
cn("", className)can be simplified:Suggestion: replace
packages/propel/src/command/command.tsx(line 7)<CommandPrimitive … className={cn("", className)} … />packages/propel/src/table/core.tsx(line 26)<tbody … className={cn("", className)} … />cn("", className)withclassNameto improve readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/propel/src/command/command.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
JayashTripathy
changed the title
3 participants
Description
added switch to propel
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Chores