Feat/input group #486
Feat/input group #486
Conversation
|
@Bridgetamana is attempting to deploy a commit to the 8bitcn Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds two InputGroup component sets (standard and 8‑bit) with six composable subcomponents, documentation pages with examples, and a registry entry registering the 8‑bit input group component. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/ui/8bit/input-group.tsx(1 hunks)components/ui/input-group.tsx(1 hunks)content/docs/components/input-group.mdx(1 hunks)registry.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx,js,jsx}: Use explicit types for function parameters and return values when they enhance clarity
Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, nevervar
Alwaysawaitpromises in async functions - don't forget to use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors
Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks meaningfully - don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Keep functions focused and under reasonable cognitive complexity limits
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Don't useeval()or assign directly todocument.cookie
Avoid spread syntax in accumulators within loops
Use top-level regex literals instead of creating them in loops
Prefer specific imports over namespace imports
Files:
components/ui/input-group.tsxcomponents/ui/8bit/input-group.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx}: Preferunknownoveranywhen the type is genuinely unknown
Use const assertions (as const) for immutable values and literal types
Leverage TypeScript's type narrowing instead of type assertions
Files:
components/ui/input-group.tsxcomponents/ui/8bit/input-group.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{jsx,tsx}: Use function components over class components in React
Call hooks at the top level only, never conditionally
Specify all dependencies in hook dependency arrays correctly
Use thekeyprop for elements in iterables (prefer unique IDs over array indices)
Nest children between opening and closing tags instead of passing as props
Don't define components inside other components
AvoiddangerouslySetInnerHTMLunless absolutely necessary
Use proper image components (e.g., Next.js<Image>) over<img>tags
Use Next.js<Image>component for images
Usenext/heador App Router metadata API for head elements
Use Server Components for async data fetching instead of async Client Components
Use ref as a prop instead ofReact.forwardRefin React 19+
Files:
components/ui/input-group.tsxcomponents/ui/8bit/input-group.tsx
**/*.{html,jsx,tsx,vue,svelte,astro}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{html,jsx,tsx,vue,svelte,astro}: Use semantic HTML and ARIA attributes for accessibility: provide meaningful alt text for images, use proper heading hierarchy, add labels for form inputs, include keyboard event handlers alongside mouse events, use semantic elements instead of divs with roles
Addrel="noopener"when usingtarget="_blank"on links
Files:
components/ui/input-group.tsxcomponents/ui/8bit/input-group.tsx
🧬 Code graph analysis (1)
components/ui/input-group.tsx (2)
components/ui/8bit/input-group.tsx (6)
InputGroup(175-175)InputGroupAddon(176-176)InputGroupText(179-179)InputGroupButton(177-177)InputGroupInput(178-178)InputGroupTextarea(180-180)lib/utils.ts (1)
cn(4-6)
🔇 Additional comments (6)
registry.json (1)
1858-1876: Registering the 8-bit Input Group looks consistent with existing entries.Name, dependencies, and file targets follow the established 8-bit registry pattern; nothing blocking here.
content/docs/components/input-group.mdx (2)
35-184: Docs cover the main Input Group variants well.Previews for icon, text, button, and textarea layouts line up with the exposed 8-bit API and show realistic compositions.
62-80: Fix the Usage snippet to use a defined icon.The snippet uses
<SearchIcon />but doesn't import or define it, which will fail if copy-pasted. For consistency with the rest of the page (which uses lucide-react'sSearchicon), update the example to import and useSearch:```tsx -import { - InputGroup, - InputGroupAddon, - InputGroupButton, - InputGroupInput, - InputGroupText, - InputGroupTextarea, -} from "@/components/ui/8bit/input-group"; +import { Search } from "lucide-react"; +import { + InputGroup, + InputGroupAddon, + InputGroupButton, + InputGroupInput, + InputGroupText, + InputGroupTextarea, +} from "@/components/ui/8bit/input-group";<InputGroup> <InputGroupInput placeholder="Search..." /> <InputGroupAddon> - <SearchIcon /> + <Search className="size-4" /> </InputGroupAddon> </InputGroup>components/ui/8bit/input-group.tsx (2)
24-172: Retro-specific layout and font handling are cohesive.Conditional
retrofont application on the group, input, and textarea plus the addon alignment variants give you flexible 8-bit layouts while keeping props simple.
1-23: Verify React type imports in TypeScript configuration and full file.The review comment flags a potential missing React import for
React.HTMLAttributes. However, verification cannot be completed due to repository access limitations.The concern may be valid depending on:
- Whether
tsconfig.jsonconfiguresjsx: "react-jsx"(new JSX transform doesn't require React import) orjsx: "react"(requires explicit import)- The actual complete file content, particularly the
InputGroupButtonPropsdefinition mentioned but not shown in the code snippet- The project's TypeScript strict mode settings
Additionally, the code snippet provided only covers lines 1-18, but the review scope is lines 1-23, so the full context is incomplete.
components/ui/input-group.tsx (1)
1-168: Base Input Group implementation looks solid and idiomatic.Good use of
cvafor root/addon variants, thin wrappers aroundButton,Input, andTextarea, and consistentdata-slotattributes for composition—no issues from a typing or layout perspective.
| }) | ||
|
|
||
| type InputGroupButtonSize = "xs" | "icon-xs" | "sm" | "icon-sm" | ||
| type InputGroupButtonVariant = "default" | "destructive" | "outline" | "secondary" | "ghost" | "link" | ||
|
|
||
| interface InputGroupButtonProps | ||
| extends Omit<React.ComponentProps<typeof Button>, "size"> { | ||
| size?: InputGroupButtonSize | ||
| variant?: InputGroupButtonVariant | ||
| } |
There was a problem hiding this comment.
Match the base input-group implementation by omitting both size and variant from Button props.
InputGroupButtonProps should follow the same pattern as InputGroupProps (defined at line 70) by omitting both "size" and "variant" from Button's props before redefining them with constrained types:
-interface InputGroupButtonProps
- extends Omit<React.ComponentProps<typeof Button>, "size"> {
+interface InputGroupButtonProps
+ extends Omit<React.ComponentProps<typeof Button>, "size" | "variant"> {
size?: InputGroupButtonSize
variant?: InputGroupButtonVariant
}This ensures consistent type narrowing across the input-group component hierarchy and prevents type conflicts with the Button component's variant property.
🤖 Prompt for AI Agents
In components/ui/8bit/input-group.tsx around lines 104 to 113,
InputGroupButtonProps currently only omits "size" from
React.ComponentProps<typeof Button>; update it to omit both "size" and "variant"
from Button props (matching InputGroupProps at line 70), then redeclare the
constrained size and variant types as shown so the prop types are narrowed
consistently and avoid conflicts with Button's own variant prop.
|
Looking good @Bridgetamana ! Can you add please InputGroup component here: |
| "path": "components/ui/8bit/input-group.tsx", | ||
| "type": "registry:component", | ||
| "target": "components/ui/8bit/input-group.tsx" | ||
| "name": "character-sheet", |
There was a problem hiding this comment.
This doesn't look good :)
Character sheet going in middle of input group JSON
2 participants
This PR adds the input group component same way as shadcn's
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.