-
-
Notifications
You must be signed in to change notification settings - Fork 69
fix(button): remove wrapper div to restore flex layout behavior #241
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
fix(button): remove wrapper div to restore flex layout behavior #241
Conversation
|
@manumorante is attempting to deploy a commit to the OrcDev Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe Button component was updated to conditionally wrap its children and decorative pixelated border and shadow elements inside a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Button
participant ShadcnButton
User->>Button: Render Button with asChild prop
alt asChild is true
Button->>ShadcnButton: Wrap children and decorations inside <span> with gap and layout classes
else asChild is false
Button->>ShadcnButton: Render children and decorations directly without extra wrapper
end
ShadcnButton-->>User: Rendered Button with consistent pixelated decorations
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/ui/8bit/button.tsxOops! Something went wrong! :( ESLint: 9.30.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/eslint-config-next@15.3.4_eslint@9.30.0_jiti@2.4.2__typescript@5.8.3/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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
🔭 Outside diff range comments (3)
components/ui/8bit/button.tsx (1)
36-45:refis declared but never forwarded – breaks consumers needing a DOM ref
BitButtonPropsexposes arefprop, yet the implementation is a regular function component instead ofReact.forwardRef, so the ref is swallowed.-function Button({ children, asChild, ...props }: BitButtonProps) { +const Button = React.forwardRef<HTMLButtonElement, BitButtonProps>( + ({ children, asChild, ...props }, ref) => { const { variant, size, className, font } = props; … - <ShadcnButton + <ShadcnButton + ref={ref} … - </ShadcnButton> + </ShadcnButton> } ); + +Button.displayName = "BitButton";public/r/8bit-login-page.json (2)
28-34: MissingReactimport breaksforwardRefand leads to unused‐ref bugsThe component uses JSX and (after the next suggestion) will rely on
React.forwardRef, but the file doesn’t importreact. Add the import to guarantee the identifier is in scope and avoid bundler-specific surprises.-import { type VariantProps, cva } from "class-variance-authority"; +import * as React from "react"; +import { type VariantProps, cva } from "class-variance-authority";
52-96:refis declared in props but silently discarded – wrap the component withforwardRef
BitButtonPropsexposes aref, yet the current implementation never passes it down toShadcnButton. Any consumer expecting to obtain a DOM handle will getnull, breaking focus management and integrations (e.g. react-hook-form).Refactor:
-function Button({ children, asChild, ...props }: BitButtonProps) { - const { variant, size, className, font } = props; +const Button = React.forwardRef<HTMLButtonElement, BitButtonProps>(( + { children, asChild, variant, size, font, className, ...props }, + ref, +) => {- <ShadcnButton - {...props} + <ShadcnButton + ref={ref} + {...props} className={cn(This also removes duplicate
variant,size, andasChildprops from the spread, preventing potential React warnings.Note: remember to export the
forwardRefwrapped component:-export { Button }; +export { Button };
♻️ Duplicate comments (4)
public/r/8bit-login-form-2.json (1)
21-21: SameButtonobservations apply hereThe bundled copy of
components/ui/8bit/button.tsxhas the same missing ref propagation and pointer-event issue raised in8bit-button.json.public/r/8bit-combo-box.json (1)
15-15: Duplicate of theButtonfeedbackSee the comments on
public/r/8bit-button.jsonregardingforwardRefandpointer-events-none.public/r/8bit-login-form.json (1)
21-96: SameforwardRef/ duplicate-prop issue as noted in8bit-login-page.jsonThe
components/ui/8bit/button.tsxchunk inside this registry item is identical; please apply the import andforwardReffixes here too.public/r/8bit-login-form-with-image.json (1)
21-96: SameforwardRef/ duplicate-prop issue as noted in8bit-login-page.jsonReplicate the import and
forwardRefrefactor plus optional pointer-events refinement.
🧹 Nitpick comments (2)
components/ui/8bit/button.tsx (1)
50-53: Minor duplication betweenfontvariant and manual class toggle
buttonVariantsalready offers afontvariant, yetfont !== "normal" && "retro"is appended manually. Pick one approach to avoid drift; prefer the cva variant.public/r/8bit-login-page.json (1)
97-140: Decorative border layers should ignore pointer eventsThe absolute-positioned pixel borders can intercept hover / click events (especially when
asChildchanges the underlying element). Addpointer-events-noneto every decorative<div>to be safe:-<div className="absolute -top-1.5 w-1/2 left-1.5 h-1.5 bg-foreground dark:bg-ring" /> +<div className="absolute -top-1.5 w-1/2 left-1.5 h-1.5 bg-foreground dark:bg-ring pointer-events-none" />Apply to all eight border corners/edges.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
components/ui/8bit/button.tsx(1 hunks)public/r/8bit-button.json(1 hunks)public/r/8bit-calendar.json(1 hunks)public/r/8bit-chart-area-step.json(1 hunks)public/r/8bit-combo-box.json(1 hunks)public/r/8bit-login-form-2.json(1 hunks)public/r/8bit-login-form-with-image.json(1 hunks)public/r/8bit-login-form.json(1 hunks)public/r/8bit-login-page.json(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:170-189
Timestamp: 2025-05-26T15:29:11.988Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenuIndicator component uses Radix Indicator directly instead of the ShadcnNavigationMenuIndicator wrapper because it needs to apply custom styling (`dark:bg-ring`) to the inner div element, which the shadcn wrapper doesn't provide an interface for.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:132-155
Timestamp: 2025-05-26T15:26:33.301Z
Learning: When building themed variants of base UI components, it may be necessary to use Radix primitives directly instead of the base component wrappers if the base components don't expose sufficient props for granular styling control. This is a valid architectural pattern when the base component's API is too restrictive for the themed variant's requirements.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#175
File: components/ui/navigation-menu.tsx:124-138
Timestamp: 2025-05-23T12:15:58.088Z
Learning: Components from shadcn UI, such as those in components/ui/navigation-menu.tsx, should be kept as close to the original implementation as possible without modifications.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:44-68
Timestamp: 2025-05-26T15:27:10.766Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenu component uses Radix Root directly instead of the ShadcnNavigationMenu wrapper because it needs to replace the contained NavigationMenuViewport with a custom implementation that includes shadow-border styling.
public/r/8bit-chart-area-step.json (2)
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#175
File: components/ui/navigation-menu.tsx:124-138
Timestamp: 2025-05-23T12:15:58.088Z
Learning: Components from shadcn UI, such as those in components/ui/navigation-menu.tsx, should be kept as close to the original implementation as possible without modifications.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:44-68
Timestamp: 2025-05-26T15:27:10.766Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenu component uses Radix Root directly instead of the ShadcnNavigationMenu wrapper because it needs to replace the contained NavigationMenuViewport with a custom implementation that includes shadow-border styling.
components/ui/8bit/button.tsx (6)
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:44-68
Timestamp: 2025-05-26T15:27:10.766Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenu component uses Radix Root directly instead of the ShadcnNavigationMenu wrapper because it needs to replace the contained NavigationMenuViewport with a custom implementation that includes shadow-border styling.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:170-189
Timestamp: 2025-05-26T15:29:11.988Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenuIndicator component uses Radix Indicator directly instead of the ShadcnNavigationMenuIndicator wrapper because it needs to apply custom styling (`dark:bg-ring`) to the inner div element, which the shadcn wrapper doesn't provide an interface for.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#175
File: components/ui/navigation-menu.tsx:124-138
Timestamp: 2025-05-23T12:15:58.088Z
Learning: Components from shadcn UI, such as those in components/ui/navigation-menu.tsx, should be kept as close to the original implementation as possible without modifications.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:132-155
Timestamp: 2025-05-26T15:26:33.301Z
Learning: When building themed variants of base UI components, it may be necessary to use Radix primitives directly instead of the base component wrappers if the base components don't expose sufficient props for granular styling control. This is a valid architectural pattern when the base component's API is too restrictive for the themed variant's requirements.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: app/docs/components/navigation-menu/demo.tsx:67-67
Timestamp: 2025-05-26T15:16:41.575Z
Learning: In the 8bitcn-ui project, `outline-hidden` is a valid custom CSS utility class used extensively throughout UI components. It should not be flagged as an error or suggested to be changed to `outline-none`.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: app/docs/components/navigation-menu/demo.tsx:67-67
Timestamp: 2025-05-26T15:16:41.575Z
Learning: In the 8bitcn-ui project, CSS class names like `bg-linear-to-b` may be custom utilities specific to their 8-bit design system and should not be automatically suggested to be changed to standard Tailwind classes without verification.
public/r/8bit-login-form.json (6)
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:170-189
Timestamp: 2025-05-26T15:29:11.988Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenuIndicator component uses Radix Indicator directly instead of the ShadcnNavigationMenuIndicator wrapper because it needs to apply custom styling (`dark:bg-ring`) to the inner div element, which the shadcn wrapper doesn't provide an interface for.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:44-68
Timestamp: 2025-05-26T15:27:10.766Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenu component uses Radix Root directly instead of the ShadcnNavigationMenu wrapper because it needs to replace the contained NavigationMenuViewport with a custom implementation that includes shadow-border styling.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#175
File: components/ui/navigation-menu.tsx:124-138
Timestamp: 2025-05-23T12:15:58.088Z
Learning: Components from shadcn UI, such as those in components/ui/navigation-menu.tsx, should be kept as close to the original implementation as possible without modifications.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: app/docs/components/navigation-menu/demo.tsx:67-67
Timestamp: 2025-05-26T15:16:41.575Z
Learning: In the 8bitcn-ui project, CSS class names like `bg-linear-to-b` may be custom utilities specific to their 8-bit design system and should not be automatically suggested to be changed to standard Tailwind classes without verification.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: app/docs/components/navigation-menu/demo.tsx:67-67
Timestamp: 2025-05-26T15:16:41.575Z
Learning: In the 8bitcn-ui project, `outline-hidden` is a valid custom CSS utility class used extensively throughout UI components. It should not be flagged as an error or suggested to be changed to `outline-none`.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:132-155
Timestamp: 2025-05-26T15:26:33.301Z
Learning: When building themed variants of base UI components, it may be necessary to use Radix primitives directly instead of the base component wrappers if the base components don't expose sufficient props for granular styling control. This is a valid architectural pattern when the base component's API is too restrictive for the themed variant's requirements.
public/r/8bit-login-page.json (6)
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:44-68
Timestamp: 2025-05-26T15:27:10.766Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenu component uses Radix Root directly instead of the ShadcnNavigationMenu wrapper because it needs to replace the contained NavigationMenuViewport with a custom implementation that includes shadow-border styling.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:170-189
Timestamp: 2025-05-26T15:29:11.988Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenuIndicator component uses Radix Indicator directly instead of the ShadcnNavigationMenuIndicator wrapper because it needs to apply custom styling (`dark:bg-ring`) to the inner div element, which the shadcn wrapper doesn't provide an interface for.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#175
File: components/ui/navigation-menu.tsx:124-138
Timestamp: 2025-05-23T12:15:58.088Z
Learning: Components from shadcn UI, such as those in components/ui/navigation-menu.tsx, should be kept as close to the original implementation as possible without modifications.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: app/docs/components/navigation-menu/demo.tsx:67-67
Timestamp: 2025-05-26T15:16:41.575Z
Learning: In the 8bitcn-ui project, CSS class names like `bg-linear-to-b` may be custom utilities specific to their 8-bit design system and should not be automatically suggested to be changed to standard Tailwind classes without verification.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: app/docs/components/navigation-menu/demo.tsx:67-67
Timestamp: 2025-05-26T15:16:41.575Z
Learning: In the 8bitcn-ui project, `outline-hidden` is a valid custom CSS utility class used extensively throughout UI components. It should not be flagged as an error or suggested to be changed to `outline-none`.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:132-155
Timestamp: 2025-05-26T15:26:33.301Z
Learning: When building themed variants of base UI components, it may be necessary to use Radix primitives directly instead of the base component wrappers if the base components don't expose sufficient props for granular styling control. This is a valid architectural pattern when the base component's API is too restrictive for the themed variant's requirements.
public/r/8bit-calendar.json (4)
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: app/docs/components/navigation-menu/demo.tsx:67-67
Timestamp: 2025-05-26T15:16:41.575Z
Learning: In the 8bitcn-ui project, CSS class names like `bg-linear-to-b` may be custom utilities specific to their 8-bit design system and should not be automatically suggested to be changed to standard Tailwind classes without verification.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#175
File: components/ui/navigation-menu.tsx:124-138
Timestamp: 2025-05-23T12:15:58.088Z
Learning: Components from shadcn UI, such as those in components/ui/navigation-menu.tsx, should be kept as close to the original implementation as possible without modifications.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:44-68
Timestamp: 2025-05-26T15:27:10.766Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenu component uses Radix Root directly instead of the ShadcnNavigationMenu wrapper because it needs to replace the contained NavigationMenuViewport with a custom implementation that includes shadow-border styling.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:170-189
Timestamp: 2025-05-26T15:29:11.988Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenuIndicator component uses Radix Indicator directly instead of the ShadcnNavigationMenuIndicator wrapper because it needs to apply custom styling (`dark:bg-ring`) to the inner div element, which the shadcn wrapper doesn't provide an interface for.
public/r/8bit-login-form-with-image.json (6)
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:170-189
Timestamp: 2025-05-26T15:29:11.988Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenuIndicator component uses Radix Indicator directly instead of the ShadcnNavigationMenuIndicator wrapper because it needs to apply custom styling (`dark:bg-ring`) to the inner div element, which the shadcn wrapper doesn't provide an interface for.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:44-68
Timestamp: 2025-05-26T15:27:10.766Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenu component uses Radix Root directly instead of the ShadcnNavigationMenu wrapper because it needs to replace the contained NavigationMenuViewport with a custom implementation that includes shadow-border styling.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#175
File: components/ui/navigation-menu.tsx:124-138
Timestamp: 2025-05-23T12:15:58.088Z
Learning: Components from shadcn UI, such as those in components/ui/navigation-menu.tsx, should be kept as close to the original implementation as possible without modifications.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: app/docs/components/navigation-menu/demo.tsx:67-67
Timestamp: 2025-05-26T15:16:41.575Z
Learning: In the 8bitcn-ui project, CSS class names like `bg-linear-to-b` may be custom utilities specific to their 8-bit design system and should not be automatically suggested to be changed to standard Tailwind classes without verification.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: app/docs/components/navigation-menu/demo.tsx:67-67
Timestamp: 2025-05-26T15:16:41.575Z
Learning: In the 8bitcn-ui project, `outline-hidden` is a valid custom CSS utility class used extensively throughout UI components. It should not be flagged as an error or suggested to be changed to `outline-none`.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:132-155
Timestamp: 2025-05-26T15:26:33.301Z
Learning: When building themed variants of base UI components, it may be necessary to use Radix primitives directly instead of the base component wrappers if the base components don't expose sufficient props for granular styling control. This is a valid architectural pattern when the base component's API is too restrictive for the themed variant's requirements.
public/r/8bit-login-form-2.json (6)
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:170-189
Timestamp: 2025-05-26T15:29:11.988Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenuIndicator component uses Radix Indicator directly instead of the ShadcnNavigationMenuIndicator wrapper because it needs to apply custom styling (`dark:bg-ring`) to the inner div element, which the shadcn wrapper doesn't provide an interface for.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:44-68
Timestamp: 2025-05-26T15:27:10.766Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenu component uses Radix Root directly instead of the ShadcnNavigationMenu wrapper because it needs to replace the contained NavigationMenuViewport with a custom implementation that includes shadow-border styling.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#175
File: components/ui/navigation-menu.tsx:124-138
Timestamp: 2025-05-23T12:15:58.088Z
Learning: Components from shadcn UI, such as those in components/ui/navigation-menu.tsx, should be kept as close to the original implementation as possible without modifications.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: app/docs/components/navigation-menu/demo.tsx:67-67
Timestamp: 2025-05-26T15:16:41.575Z
Learning: In the 8bitcn-ui project, CSS class names like `bg-linear-to-b` may be custom utilities specific to their 8-bit design system and should not be automatically suggested to be changed to standard Tailwind classes without verification.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: app/docs/components/navigation-menu/demo.tsx:67-67
Timestamp: 2025-05-26T15:16:41.575Z
Learning: In the 8bitcn-ui project, `outline-hidden` is a valid custom CSS utility class used extensively throughout UI components. It should not be flagged as an error or suggested to be changed to `outline-none`.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:132-155
Timestamp: 2025-05-26T15:26:33.301Z
Learning: When building themed variants of base UI components, it may be necessary to use Radix primitives directly instead of the base component wrappers if the base components don't expose sufficient props for granular styling control. This is a valid architectural pattern when the base component's API is too restrictive for the themed variant's requirements.
public/r/8bit-combo-box.json (6)
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:44-68
Timestamp: 2025-05-26T15:27:10.766Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenu component uses Radix Root directly instead of the ShadcnNavigationMenu wrapper because it needs to replace the contained NavigationMenuViewport with a custom implementation that includes shadow-border styling.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:170-189
Timestamp: 2025-05-26T15:29:11.988Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenuIndicator component uses Radix Indicator directly instead of the ShadcnNavigationMenuIndicator wrapper because it needs to apply custom styling (`dark:bg-ring`) to the inner div element, which the shadcn wrapper doesn't provide an interface for.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#175
File: components/ui/navigation-menu.tsx:124-138
Timestamp: 2025-05-23T12:15:58.088Z
Learning: Components from shadcn UI, such as those in components/ui/navigation-menu.tsx, should be kept as close to the original implementation as possible without modifications.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: app/docs/components/navigation-menu/demo.tsx:67-67
Timestamp: 2025-05-26T15:16:41.575Z
Learning: In the 8bitcn-ui project, `outline-hidden` is a valid custom CSS utility class used extensively throughout UI components. It should not be flagged as an error or suggested to be changed to `outline-none`.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:132-155
Timestamp: 2025-05-26T15:26:33.301Z
Learning: When building themed variants of base UI components, it may be necessary to use Radix primitives directly instead of the base component wrappers if the base components don't expose sufficient props for granular styling control. This is a valid architectural pattern when the base component's API is too restrictive for the themed variant's requirements.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: app/docs/components/navigation-menu/demo.tsx:67-67
Timestamp: 2025-05-26T15:16:41.575Z
Learning: In the 8bitcn-ui project, CSS class names like `bg-linear-to-b` may be custom utilities specific to their 8-bit design system and should not be automatically suggested to be changed to standard Tailwind classes without verification.
public/r/8bit-button.json (6)
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:170-189
Timestamp: 2025-05-26T15:29:11.988Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenuIndicator component uses Radix Indicator directly instead of the ShadcnNavigationMenuIndicator wrapper because it needs to apply custom styling (`dark:bg-ring`) to the inner div element, which the shadcn wrapper doesn't provide an interface for.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:44-68
Timestamp: 2025-05-26T15:27:10.766Z
Learning: In components/ui/8bit/navigation-menu.tsx, the NavigationMenu component uses Radix Root directly instead of the ShadcnNavigationMenu wrapper because it needs to replace the contained NavigationMenuViewport with a custom implementation that includes shadow-border styling.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#175
File: components/ui/navigation-menu.tsx:124-138
Timestamp: 2025-05-23T12:15:58.088Z
Learning: Components from shadcn UI, such as those in components/ui/navigation-menu.tsx, should be kept as close to the original implementation as possible without modifications.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: app/docs/components/navigation-menu/demo.tsx:67-67
Timestamp: 2025-05-26T15:16:41.575Z
Learning: In the 8bitcn-ui project, `outline-hidden` is a valid custom CSS utility class used extensively throughout UI components. It should not be flagged as an error or suggested to be changed to `outline-none`.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: app/docs/components/navigation-menu/demo.tsx:67-67
Timestamp: 2025-05-26T15:16:41.575Z
Learning: In the 8bitcn-ui project, CSS class names like `bg-linear-to-b` may be custom utilities specific to their 8-bit design system and should not be automatically suggested to be changed to standard Tailwind classes without verification.
Learnt from: BIT-zhaoyang
PR: TheOrcDev/8bitcn-ui#203
File: components/ui/8bit/navigation-menu.tsx:132-155
Timestamp: 2025-05-26T15:26:33.301Z
Learning: When building themed variants of base UI components, it may be necessary to use Radix primitives directly instead of the base component wrappers if the base components don't expose sufficient props for granular styling control. This is a valid architectural pattern when the base component's API is too restrictive for the themed variant's requirements.
🔇 Additional comments (2)
public/r/8bit-chart-area-step.json (1)
11-17: Margin increase looks safe but double-check legend / tooltip clippingThe chart’s left/right margin changed from 12 → 20 px. Visually this is fine, but ensure the added padding doesn’t push small viewports below the breakpoint where
ResponsiveContainerstarts clipping the legend or tooltip.public/r/8bit-calendar.json (1)
13-13: Check hit-target size after removingp-2from day buttons
day_buttonnow relies onh-10without any horizontal padding. On touch devices this might drop below the 24 × 24 px WCAG minimum tap target once the calendar is scaled down.Consider re-introducing a minimal
px-1or usingmin-w-[2.5rem]to keep the control accessible.
public/r/8bit-button.json
Outdated
| { | ||
| "path": "components/ui/8bit/button.tsx", | ||
| "content": "import { type VariantProps, cva } from \"class-variance-authority\";\n\nimport { cn } from \"@/lib/utils\";\n\nimport { Button as ShadcnButton } from \"@/components/ui/button\";\n\nimport \"./styles/retro.css\";\n\nexport const buttonVariants = cva(\"\", {\n variants: {\n font: {\n normal: \"\",\n retro: \"retro\",\n },\n variant: {\n default: \"bg-foreground\",\n destructive: \"bg-foreground\",\n outline: \"bg-foreground\",\n secondary: \"bg-secondary text-secondary-foreground hover:bg-secondary/80\",\n ghost: \"hover:bg-accent hover:text-accent-foreground\",\n link: \"text-primary underline-offset-4 hover:underline\",\n },\n size: {\n default: \"h-9 px-4 py-2 has-[>svg]:px-3\",\n sm: \"h-8 rounded-md gap-1.5 px-3 has-[>svg]:px-2.5\",\n lg: \"h-10 rounded-md px-6 has-[>svg]:px-4\",\n icon: \"size-9\",\n },\n },\n defaultVariants: {\n variant: \"default\",\n size: \"default\",\n },\n});\n\nexport interface BitButtonProps\n extends React.ButtonHTMLAttributes<HTMLButtonElement>,\n VariantProps<typeof buttonVariants> {\n asChild?: boolean;\n ref?: React.Ref<HTMLButtonElement>;\n}\n\nfunction Button({ children, asChild, ...props }: BitButtonProps) {\n const { variant, size, className, font } = props;\n\n return (\n <ShadcnButton\n {...props}\n className={cn(\n \"rounded-none active:translate-y-1 transition-transform relative inline-flex items-center justify-center\",\n font !== \"normal\" && \"retro\",\n className\n )}\n size={size}\n variant={variant}\n asChild={asChild}\n >\n <div>\n {children}\n\n {variant !== \"ghost\" && variant !== \"link\" && size !== \"icon\" && (\n <>\n {/* Pixelated border */}\n <div className=\"absolute -top-1.5 w-1/2 left-1.5 h-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute -top-1.5 w-1/2 right-1.5 h-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute -bottom-1.5 w-1/2 left-1.5 h-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute -bottom-1.5 w-1/2 right-1.5 h-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute top-0 left-0 size-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute top-0 right-0 size-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute bottom-0 left-0 size-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute bottom-0 right-0 size-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute top-1.5 -left-1.5 h-2/3 w-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute top-1.5 -right-1.5 h-2/3 w-1.5 bg-foreground dark:bg-ring\" />\n {variant !== \"outline\" && (\n <>\n {/* Top shadow */}\n <div className=\"absolute top-0 left-0 w-full h-1.5 bg-foreground/20\" />\n <div className=\"absolute top-1.5 left-0 w-3 h-1.5 bg-foreground/20\" />\n\n {/* Bottom shadow */}\n <div className=\"absolute bottom-0 left-0 w-full h-1.5 bg-foreground/20\" />\n <div className=\"absolute bottom-1.5 right-0 w-3 h-1.5 bg-foreground/20\" />\n </>\n )}\n </>\n )}\n\n {size === \"icon\" && (\n <>\n <div className=\"absolute top-0 left-0 w-full h-[5px] md:h-1.5 bg-foreground dark:bg-ring pointer-events-none\" />\n <div className=\"absolute bottom-0 w-full h-[5px] md:h-1.5 bg-foreground dark:bg-ring pointer-events-none\" />\n <div className=\"absolute top-1 -left-1 w-[5px] md:w-1.5 h-1/2 bg-foreground dark:bg-ring pointer-events-none\" />\n <div className=\"absolute bottom-1 -left-1 w-[5px] md:w-1.5 h-1/2 bg-foreground dark:bg-ring pointer-events-none\" />\n <div className=\"absolute top-1 -right-1 w-[5px] md:w-1.5 h-1/2 bg-foreground dark:bg-ring pointer-events-none\" />\n <div className=\"absolute bottom-1 -right-1 w-[5px] md:w-1.5 h-1/2 bg-foreground dark:bg-ring pointer-events-none\" />\n </>\n )}\n </div>\n </ShadcnButton>\n );\n}\n\nexport { Button };\n", | ||
| "content": "import { type VariantProps, cva } from \"class-variance-authority\";\n\nimport { cn } from \"@/lib/utils\";\n\nimport { Button as ShadcnButton } from \"@/components/ui/button\";\n\nimport \"./styles/retro.css\";\n\nexport const buttonVariants = cva(\"\", {\n variants: {\n font: {\n normal: \"\",\n retro: \"retro\",\n },\n variant: {\n default: \"bg-foreground\",\n destructive: \"bg-foreground\",\n outline: \"bg-foreground\",\n secondary: \"bg-secondary text-secondary-foreground hover:bg-secondary/80\",\n ghost: \"hover:bg-accent hover:text-accent-foreground\",\n link: \"text-primary underline-offset-4 hover:underline\",\n },\n size: {\n default: \"h-9 px-4 py-2 has-[>svg]:px-3\",\n sm: \"h-8 rounded-md gap-1.5 px-3 has-[>svg]:px-2.5\",\n lg: \"h-10 rounded-md px-6 has-[>svg]:px-4\",\n icon: \"size-9\",\n },\n },\n defaultVariants: {\n variant: \"default\",\n size: \"default\",\n },\n});\n\nexport interface BitButtonProps\n extends React.ButtonHTMLAttributes<HTMLButtonElement>,\n VariantProps<typeof buttonVariants> {\n asChild?: boolean;\n ref?: React.Ref<HTMLButtonElement>;\n}\n\nfunction Button({ children, asChild, ...props }: BitButtonProps) {\n const { variant, size, className, font } = props;\n\n return (\n <ShadcnButton\n {...props}\n className={cn(\n \"rounded-none active:translate-y-1 transition-transform relative inline-flex items-center justify-center\",\n font !== \"normal\" && \"retro\",\n className\n )}\n size={size}\n variant={variant}\n asChild={asChild}\n >\n {children}\n\n {variant !== \"ghost\" && variant !== \"link\" && size !== \"icon\" && (\n <>\n {/* Pixelated border */}\n <div className=\"absolute -top-1.5 w-1/2 left-1.5 h-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute -top-1.5 w-1/2 right-1.5 h-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute -bottom-1.5 w-1/2 left-1.5 h-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute -bottom-1.5 w-1/2 right-1.5 h-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute top-0 left-0 size-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute top-0 right-0 size-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute bottom-0 left-0 size-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute bottom-0 right-0 size-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute top-1.5 -left-1.5 h-2/3 w-1.5 bg-foreground dark:bg-ring\" />\n <div className=\"absolute top-1.5 -right-1.5 h-2/3 w-1.5 bg-foreground dark:bg-ring\" />\n {variant !== \"outline\" && (\n <>\n {/* Top shadow */}\n <div className=\"absolute top-0 left-0 w-full h-1.5 bg-foreground/20\" />\n <div className=\"absolute top-1.5 left-0 w-3 h-1.5 bg-foreground/20\" />\n\n {/* Bottom shadow */}\n <div className=\"absolute bottom-0 left-0 w-full h-1.5 bg-foreground/20\" />\n <div className=\"absolute bottom-1.5 right-0 w-3 h-1.5 bg-foreground/20\" />\n </>\n )}\n </>\n )}\n\n {size === \"icon\" && (\n <>\n <div className=\"absolute top-0 left-0 w-full h-[5px] md:h-1.5 bg-foreground dark:bg-ring pointer-events-none\" />\n <div className=\"absolute bottom-0 w-full h-[5px] md:h-1.5 bg-foreground dark:bg-ring pointer-events-none\" />\n <div className=\"absolute top-1 -left-1 w-[5px] md:w-1.5 h-1/2 bg-foreground dark:bg-ring pointer-events-none\" />\n <div className=\"absolute bottom-1 -left-1 w-[5px] md:w-1.5 h-1/2 bg-foreground dark:bg-ring pointer-events-none\" />\n <div className=\"absolute top-1 -right-1 w-[5px] md:w-1.5 h-1/2 bg-foreground dark:bg-ring pointer-events-none\" />\n <div className=\"absolute bottom-1 -right-1 w-[5px] md:w-1.5 h-1/2 bg-foreground dark:bg-ring pointer-events-none\" />\n </>\n )}\n </ShadcnButton>\n );\n}\n\nexport { Button };\n", |
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.
Forward ref to keep the component drop-in compatible
BitButtonProps exposes ref, yet the implementation does not pass it down, so callers can’t imperatively focus / measure the button.
Wrap the component in React.forwardRef and relay the ref to ShadcnButton.
-import function Button({ children, asChild, ...props }: BitButtonProps) {
- const { variant, size, className, font } = props;
+const Button = React.forwardRef<HTMLButtonElement, BitButtonProps>(
+ ({ children, asChild, ...props }, ref) => {
+ const { variant, size, className, font } = props;
...
- <ShadcnButton
+ <ShadcnButton
+ ref={ref}
...
-}
+ }
+);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In public/r/8bit-button.json at line 13, the BitButtonProps interface includes a
ref property, but the Button component does not forward this ref to the
underlying ShadcnButton, preventing callers from accessing the button's DOM
node. To fix this, wrap the Button function component with React.forwardRef and
pass the received ref down to the ShadcnButton component as its ref prop,
ensuring the component remains drop-in compatible and supports imperative
actions like focus or measurement.
🛠️ Refactor suggestion
Decorative border layers should not steal pointer events
The eight pixel-border <div>s lack pointer-events-none, so clicks originating near the button edges are intercepted by those absolutely-positioned elements instead of the button itself.
You already add the flag for the icon-size variant; apply it consistently:
-<div className="absolute -top-1.5 w-1/2 left-1.5 h-1.5 bg-foreground dark:bg-ring" />
+<div className="pointer-events-none absolute -top-1.5 w-1/2 left-1.5 h-1.5 bg-foreground dark:bg-ring" />Repeat for the other seven border pieces (and both lateral bars).
End-users will regain the expected click area.
🤖 Prompt for AI Agents
In public/r/8bit-button.json at line 13, the decorative pixel-border div
elements lack the pointer-events-none class, causing them to intercept clicks
near the button edges. To fix this, add the pointer-events-none class to all
eight pixel-border divs and the two lateral bar divs to ensure clicks pass
through to the button itself, matching the existing behavior for the icon size
variant.
TheOrcDev
left a 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.
Thanks for this one! Nice catch! ⚔️
I found this bug in the buttons that prevents the icons from aligning correctly with the text.
This is what it would look like now:
Summary by CodeRabbit