-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[WEB-4731] feat: add baseui input component to propel package #7769
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
WalkthroughIntroduces a new Input component in @plane/propel with stories, adds corresponding barrel/export entries and build config wiring, updates package exports to expose "./input", and reorders a single import in a web editor file. No changes to existing public APIs beyond the new export. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Consumer
participant Input as Input (propel)
participant BaseInput as BaseInput (@base-ui-components/react/input)
Consumer->>Input: Render with props (mode, inputSize, hasError, type, ...)
Note right of Input: Compute className via cn<br/>merge defaults + conditionals
Input->>BaseInput: Forward props/ref (id, type, name, autoComplete, ...)<br/>className
BaseInput-->>Consumer: Rendered input element
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ 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 |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
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 adds a new Input component to the propel package, built on top of the @base-ui-components/react/input library with customizable styling and properties.
- Added a new Input component with support for different modes (primary, transparent, true-transparent), sizes (xs, sm, md), and states (error, disabled)
- Created comprehensive Storybook stories demonstrating all component variants and use cases
- Updated package configuration to export the new Input component
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/propel/src/input/index.tsx | Main Input component implementation with BaseUI integration and styling |
| packages/propel/src/input/input.stories.tsx | Comprehensive Storybook stories showcasing all Input variants |
| packages/propel/tsdown.config.ts | Added input module to build configuration |
| packages/propel/package.json | Added input export path for package consumers |
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: 3
🧹 Nitpick comments (3)
packages/propel/src/input/index.tsx (2)
27-49: Add accessibility hint for invalid inputs.Expose aria-invalid so screen readers catch the error state.
<BaseInput id={id} ref={ref} type={type} name={name} + aria-invalid={hasError || undefined} className={cn(
6-12: Don’t narrow autoComplete; avoid redeclaring className.
- InputHTMLAttributes already includes className; redeclaration is redundant.
- Narrowing autoComplete to "on" | "off" blocks valid tokens like "email", "username", etc. Keep it as the native string type.
export interface InputProps extends React.InputHTMLAttributes<HTMLInputElement> { mode?: "primary" | "transparent" | "true-transparent"; inputSize?: "xs" | "sm" | "md"; hasError?: boolean; - className?: string; - autoComplete?: "on" | "off"; + autoComplete?: React.InputHTMLAttributes<HTMLInputElement>["autoComplete"]; }packages/propel/src/input/input.stories.tsx (1)
126-129: Rename story “Number” to avoid shadowing global Number-export const Number = createStory({ +export const NumberField = createStory({Prevents confusion and satisfies the linter.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/core/components/pages/version/editor.tsx(1 hunks)packages/propel/package.json(1 hunks)packages/propel/src/input/index.tsx(1 hunks)packages/propel/src/input/input.stories.tsx(1 hunks)packages/propel/tsdown.config.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/propel/src/input/input.stories.tsx (1)
packages/propel/src/input/index.tsx (1)
Input(55-55)
🪛 Biome (2.1.2)
packages/propel/src/input/input.stories.tsx
[error] 126-126: Do not shadow the global "Number" 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). (1)
- GitHub Check: Build and lint web apps
🔇 Additional comments (1)
apps/web/core/components/pages/version/editor.tsx (1)
7-7: No functional change; import order tweak is fine.Reorder aligns with local style; no impact.
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: 3
🧹 Nitpick comments (2)
packages/propel/src/input/input.tsx (2)
19-24: Don’t default autoComplete to "off".Hurts UX/password managers. Let browsers decide unless explicitly set.
- autoComplete = "off", + autoComplete,
53-53: Set displayName to “Input”.Improves DevTools/Storybook clarity.
-Input.displayName = "form-input-field"; +Input.displayName = "Input";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/propel/src/input/index.ts(1 hunks)packages/propel/src/input/input.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)
🔇 Additional comments (2)
packages/propel/src/input/index.ts (1)
1-1: Barrel export looks good.Simple and correct re-export to expose the component.
packages/propel/src/input/input.tsx (1)
28-49: Confirm BaseInput styling target.Please verify that BaseInput forwards className and aria-* to the actual element (not a wrapper), otherwise styles/a11y props may not apply as intended.
…t-propel-input
Description
Added input component to propel package with respective stories
Type of Change
Screenshots and Media (if applicable)
Summary by CodeRabbit