Skip to content

fix: improve Button component accessibility and reusability - Add typ… #42 #44

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.

Sign up for GitHub

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

Open
ankitrraj wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: improve Button component accessibility and reusability - Add typ… #42 #44

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions src/components/Button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,32 @@
function Button(props: React.PropsWithChildren) {
import { twMerge } from 'tailwind-merge'

interface ButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> {
variant?: 'primary' | 'secondary'
}
Comment on lines +1 to +5

@coderabbitai coderabbitai Bot Dec 14, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix missing React type import (current code won’t typecheck/compile).
You reference React.ButtonHTMLAttributes but don’t import React (type). Import it, or stop using the React. namespace.

+import type * as React from 'react'
 import { twMerge } from 'tailwind-merge'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { twMerge } from 'tailwind-merge'
interface ButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> {
variant?: 'primary' | 'secondary'
}
import type * as React from 'react'
import { twMerge } from 'tailwind-merge'
interface ButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> {
variant?: 'primary' | 'secondary'
}
🤖 Prompt for AI Agents 
In src/components/Button.tsx around lines 1-5, the component uses
React.ButtonHTMLAttributes but React is not imported causing typecheck failures;
fix by either adding an import for React types (e.g., `import type React from
'react'` or `import React from 'react'` depending on tsconfig) or switch to the
direct type import `import type { ButtonHTMLAttributes } from 'react'` and
update the interface to extend ButtonHTMLAttributes<HTMLButtonElement>; ensure
imports use `import type` when only types are needed to avoid runtime imports.


export function Button({
children,
className,
variant = 'primary',
type = 'button',
...props
}: ButtonProps) {
Comment on lines +3 to +13

@coderabbitai coderabbitai Bot Dec 14, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

variant is part of the public API but currently does nothing.
Either wire it into the className (recommended) or remove it to avoid a misleading prop.

Example wiring:

 export function Button({
   children,
   className,
   variant = 'primary',
   type = 'button',
   ...props
 }: ButtonProps) {
+  const variantClasses =
+    variant === 'secondary'
+      ? 'from-slate-700 to-slate-900'
+      : 'from-[#331500] to-[#FF863B]'
   return (
     <button
       type={type}
       className={twMerge(
-        'relative py-1.5 px-2.5 sm:py-2 sm:px-3 rounded-lg font-medium text-xs sm:text-sm bg-gradient-to-b from-[#331500] to-[#FF863B] transition-transform hover:scale-105 active:scale-95 disabled:opacity-50 disabled:cursor-not-allowed disabled:hover:scale-100',
+        'relative py-1.5 px-2.5 sm:py-2 sm:px-3 rounded-lg font-medium text-xs sm:text-sm bg-gradient-to-b transition-transform hover:scale-105 active:scale-95 disabled:opacity-50 disabled:cursor-not-allowed disabled:hover:scale-100',
+        variantClasses,
         className
       )}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/components/Button.tsx around lines 3 to 13, the exported Button accepts a
variant prop but never uses it; either remove the prop or apply it to the
rendered element. Fix by mapping variant to CSS classes (e.g. determine a base
className and append variant-specific classes for 'primary' and 'secondary'),
merge with any incoming className, and pass the combined className to the button
element; keep default variant='primary' and preserve other props.

return (
<button
className="relative py-1.5 px-2.5 sm:py-2 sm:px-3 rounded-lg font-medium text-xs sm:text-sm bg-gradient-to-b from-[#331500] to-[#FF863B]"
type={type}
className={twMerge(
'relative py-1.5 px-2.5 sm:py-2 sm:px-3 rounded-lg font-medium text-xs sm:text-sm bg-gradient-to-b from-[#331500] to-[#FF863B] transition-transform hover:scale-105 active:scale-95 disabled:opacity-50 disabled:cursor-not-allowed disabled:hover:scale-100',
className
)}
style={{ boxShadow: '0 0 12px #FF863B' }}
{...props}
>
<div className="absolute inset-0 rounded-lg">
<div className="rounded-lg border border-white/20 absolute inset-0 [mask-image:linear-gradient(to_bottom,black,transparent)]"></div>
<div className="rounded-lg border absolute inset-0 border-white/40 [mask-image:linear-gradient(to_top,black,transparent)]"></div>
<div className="absolute inset-0 shadow-[0px_0px_10px_0px_rgba(255,134,59,0.7)_inset] rounded-lg"></div>
</div>
<span>{props.children}</span>
<span className="relative">{children}</span>
</button>
)
}
Expand Down