[WEB-4736] dev: propel button#7746
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new Button component (with variants and sizes), Storybook stories, an index re-export, includes it in tsdown build entries, and exposes it via a new package export ("./button") in packages/propel/package.json. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Consumer
participant Build as Bundler
participant PropelPkg as @propel package
participant Runtime as React
App->>Build: import { Button } from "@propel/button"
Build->>PropelPkg: resolve exports["./button"]
PropelPkg-->>Build: ./dist/button/index.js
Build->>App: bundle Button
App->>Runtime: render <Button variant="PRIMARY" size="MD" />
Runtime-->>App: mounted button element
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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. Comment |
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new Button component to the propel design system package. The implementation includes a React component with multiple variants and sizes, proper TypeScript types, and comprehensive Storybook documentation.
- Adds a new Button component with 5 variants (primary, secondary, outline, ghost, destructive) and 3 sizes (sm, md, lg)
- Implements proper TypeScript interfaces and enum types for variant and size options
- Includes comprehensive Storybook stories demonstrating all variants, sizes, and component states
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/propel/tsdown.config.ts | Adds button entry point to build configuration |
| packages/propel/src/button/index.ts | Exports Button component and types |
| packages/propel/src/button/button.tsx | Main Button component implementation with variants and sizes |
| packages/propel/src/button/button.stories.tsx | Storybook stories for all button variants and configurations |
| packages/propel/package.json | Adds button export path to package exports |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/propel/package.json (1)
21-21: Public subpath export added — LGTM.Optional: consider adding "types" conditions to subpath exports in a follow-up (applied consistently across all entries) for crisper TS resolution.
I can draft a repo-wide export map with { types, import, require } conditions if you want.
packages/propel/src/button/button.tsx (3)
33-47: Type the style maps for exhaustiveness and safer indexing.Apply this diff:
-const buttonVariants = { +const buttonVariants: Record<TButtonVariant, string> = { [EButtonVariant.PRIMARY]: "bg-custom-primary-100 text-white hover:bg-custom-primary-200 focus:bg-custom-primary-200", [EButtonVariant.SECONDARY]: "bg-custom-background-100 text-custom-text-200 border border-custom-border-200 hover:bg-custom-background-90 focus:bg-custom-background-90", [EButtonVariant.OUTLINE]: "border border-custom-primary-100 text-custom-primary-100 bg-transparent hover:bg-custom-primary-100/10 focus:bg-custom-primary-100/20", [EButtonVariant.GHOST]: "text-custom-text-200 hover:bg-custom-background-90 focus:bg-custom-background-90", [EButtonVariant.DESTRUCTIVE]: "bg-red-500 text-white hover:bg-red-600 focus:bg-red-600", }; -const buttonSizes = { +const buttonSizes: Record<TButtonSize, string> = { [EButtonSize.SM]: "px-3 py-1.5 text-xs font-medium", [EButtonSize.MD]: "px-4 py-2 text-sm font-medium", [EButtonSize.LG]: "px-6 py-2.5 text-base font-medium", };
26-31: Children can be optional; className is already in ButtonHTMLAttributes.Apply this diff:
export interface ButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> { variant?: TButtonVariant; size?: TButtonSize; - className?: string; - children: React.ReactNode; + // className is already included in the base interface + children?: React.ReactNode; }
18-25: Reduce duplication in type aliases.Using the enum type directly simplifies maintenance.
Apply this diff:
-export type TButtonVariant = - | EButtonVariant.PRIMARY - | EButtonVariant.SECONDARY - | EButtonVariant.OUTLINE - | EButtonVariant.GHOST - | EButtonVariant.DESTRUCTIVE; -export type TButtonSize = EButtonSize.SM | EButtonSize.MD | EButtonSize.LG; +export type TButtonVariant = EButtonVariant; +export type TButtonSize = EButtonSize;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/propel/package.json(1 hunks)packages/propel/src/button/button.stories.tsx(1 hunks)packages/propel/src/button/button.tsx(1 hunks)packages/propel/src/button/index.ts(1 hunks)packages/propel/tsdown.config.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/propel/src/button/button.tsx (1)
packages/propel/src/button/index.ts (2)
ButtonProps(2-2)Button(1-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)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (4)
packages/propel/tsdown.config.ts (1)
7-7: Button entry wired into build — LGTM.Including "src/button/index.ts" ensures the module is bundled and d.ts emitted with the rest.
packages/propel/src/button/button.tsx (1)
33-41: Verify or define destructive design tokens. Nocustom-danger-*tokens were found—check your Tailwind or design-token config for existing “danger/destructive” colors; if they’re missing, add them and then update the [EButtonVariant.DESTRUCTIVE] entry to use those tokens instead of hardcoded red values.packages/propel/src/button/button.stories.tsx (2)
26-119: Stories cover variants, sizes, disabled — LGTM.Good breadth for visual regressions and docs.
1-1: Import path is correct and consistent in packages/propel; no changes needed.
* dev: button added to propel * dev: button story added * chore: propel config updated --------- Co-authored-by: sriram veeraghanta <veeraghanta.sriram@gmail.com>
Description
This PR introduces the button component in propel.
Media
Summary by CodeRabbit
New Features
Documentation
Chores