-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[WEB-4737] dev: propel pill component #7743
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
WalkthroughAdds a new Pill UI component with variants and sizes, exposes it via a new Changes
Sequence Diagram(s)sequenceDiagram
actor Dev as Developer
participant App as App Code
participant Propel as @propel/pill
participant Pill as Pill Component
participant DOM as Browser DOM
participant Util as cn()
Dev->>App: import { Pill } from "@propel/pill"
App->>Propel: Resolve export "./pill"
Propel-->>App: dist/pill/index.js
App->>Pill: <Pill variant size className>children</Pill>
Pill->>Util: cn(base, variantClass, sizeClass, className)
Util-->>Pill: merged class string
Pill->>DOM: render <span> with classes and children
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new Pill component to the propel design system. The component provides a flexible way to display small status indicators or labels with different visual variants and sizes.
- Added a reusable Pill component with variant and size customization options
- Included comprehensive Storybook stories demonstrating all variants and use cases
- Updated build configuration and package exports to include the new component
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 | Added pill component to build configuration |
| packages/propel/src/pill/pill.tsx | Implemented main Pill component with variants and sizes |
| packages/propel/src/pill/pill.stories.tsx | Created comprehensive Storybook stories for component documentation |
| packages/propel/src/pill/index.ts | Added component exports |
| packages/propel/package.json | Updated package exports to include pill component |
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/propel/src/pill/pill.tsx (2)
35-48: Type the lookup tables to catch missing keys at compile-time.Add explicit Record typings for stronger safety.
-const pillVariants = { +const pillVariants: Record<TPillVariant, string> = { [EPillVariant.DEFAULT]: "bg-custom-background-90 text-custom-text-200 border border-custom-border-200", [EPillVariant.PRIMARY]: "bg-custom-primary-100/10 text-custom-primary-100 border border-custom-primary-100/20", [EPillVariant.SUCCESS]: "bg-green-50 text-green-700 border border-green-200", [EPillVariant.WARNING]: "bg-amber-50 text-amber-700 border border-amber-200", [EPillVariant.ERROR]: "bg-red-50 text-red-700 border border-red-200", [EPillVariant.INFO]: "bg-blue-50 text-blue-700 border border-blue-200", }; -const pillSizes = { +const pillSizes: Record<TPillSize, string> = { [EPillSize.SM]: "px-2 py-0.5 text-xs", [EPillSize.MD]: "px-2.5 py-1 text-sm", [EPillSize.LG]: "px-3 py-1.5 text-base", };
55-61: Consider using class-variance-authority for variants/sizes.You already depend on
cva; adopting it here would align with other components and centralize defaults.packages/propel/src/pill/pill.stories.tsx (2)
53-58: Rename theErrorstory to avoid shadowing the globalError.Prevents linter complaints and accidental confusion.
-export const Error: Story = { +export const ErrorVariant: Story = { args: { variant: EPillVariant.ERROR, children: "Error", }, };
4-21: Stronger typing withsatisfies Meta<typeof Pill>.This narrows
metawithout over-constraining.-const meta: Meta<typeof Pill> = { +const meta = { title: "Components/Pill", component: Pill, parameters: { layout: "centered", }, tags: ["autodocs"], argTypes: { variant: { control: "select", options: Object.values(EPillVariant), }, size: { control: "select", options: Object.values(EPillSize), }, }, -}; +} satisfies Meta<typeof Pill>;packages/propel/src/pill/index.ts (1)
1-2: Export enums and helper types from the barrel for a complete public API.Consumers will likely import variants/sizes; re-export them here.
-export { Pill } from "./pill"; -export type { PillProps } from "./pill"; +export { Pill, EPillVariant, EPillSize } from "./pill"; +export type { PillProps, TPillVariant, TPillSize } from "./pill";
📜 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/pill/index.ts(1 hunks)packages/propel/src/pill/pill.stories.tsx(1 hunks)packages/propel/src/pill/pill.tsx(1 hunks)packages/propel/tsdown.config.ts(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
packages/propel/src/pill/pill.stories.tsx
[error] 53-53: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ 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)
🔇 Additional comments (5)
packages/propel/package.json (2)
29-29: Export wired correctly for the new entry.The subpath export
"./pill"points to the built entry and matches tsdown’s new entry. LGTM.
29-29: Confirm TS type resolution for subpath.If your TS version is <5.3, consider adding types mapping (typesVersions or per-export "types") so
@plane/propel/pillresolvesdist/pill/index.d.ts. Otherwise, no action.packages/propel/src/pill/pill.tsx (1)
50-68: Solid, idiomatic forwardRef implementation.Defaults, class merging, and prop passthrough look good.
packages/propel/src/pill/pill.stories.tsx (1)
1-1: Verify Storybook types import path.Most setups import types from
@storybook/reacteven with the Vite builder; if this repo standardizes on@storybook/react-vitefor types, keep it. Otherwise consider switching for consistency.packages/propel/tsdown.config.ts (1)
15-15: Build entry added — good.Including
src/pill/index.tsensures JS and DTS emission for the new subpath.
* dev: pill component added to propel * dev: pill story added * chore: propel config updated * chore: code refactor
Description
This PR introduces the toolbar component in propel.
Media
Summary by CodeRabbit