feat(button): add ghost variant - #1684
Conversation
|
@mhmdwaelanwr is attempting to deploy a commit to the Bergside Team on Vercel. A member of the Team first needs to authorize it. |
🦋 Changeset detectedLatest commit: ab975d0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughAdds a ChangesGhost button support
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/ui/src/components/Button/theme.ts (2)
63-64: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse a darker light-mode text color for yellow ghost buttons.
bg-transparent text-yellow-500places the button text on the surrounding light surface. The default yellow token is too light for normal button text. WCAG requires a contrast ratio of at least 4.5:1 for normal text. (tailwindcss.com)Use a darker token such as
text-yellow-700, then verify the hover and focus states.Proposed fix
yellow: - "bg-transparent text-yellow-500 hover:bg-yellow-100 focus:ring-yellow-300 dark:text-yellow-300 dark:hover:bg-yellow-900/50 dark:focus:ring-yellow-900", + "bg-transparent text-yellow-700 hover:bg-yellow-100 focus:ring-yellow-300 dark:text-yellow-300 dark:hover:bg-yellow-900/50 dark:focus:ring-yellow-900",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/components/Button/theme.ts` around lines 63 - 64, Update the yellow variant in the button theme to use a darker light-mode text token, such as text-yellow-700, instead of text-yellow-500. Preserve the existing transparent background and dark-mode styles, and verify that the yellow hover and focus states remain appropriate.
37-37: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the red focus-ring behavior consistent for ghost red buttons.
color.redandoutlineColor.redusedark:focus:ring-red-800, butghostColor.redstill usesdark:focus:ring-red-900. Use the same dark focus-ring value for all red variants, or add an explicit design distinction with tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/components/Button/theme.ts` at line 37, Update the red entry in ghostColor to use the same dark focus-ring value as color.red and outlineColor.red, ensuring all red button variants consistently use dark:focus:ring-red-800.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/ui/src/components/Button/theme.ts`:
- Around line 63-64: Update the yellow variant in the button theme to use a
darker light-mode text token, such as text-yellow-700, instead of
text-yellow-500. Preserve the existing transparent background and dark-mode
styles, and verify that the yellow hover and focus states remain appropriate.
- Line 37: Update the red entry in ghostColor to use the same dark focus-ring
value as color.red and outlineColor.red, ensuring all red button variants
consistently use dark:focus:ring-red-800.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 42f68895-502c-4c06-a2de-f3e65d0a9970
📒 Files selected for processing (1)
packages/ui/src/components/Button/theme.ts
1 participant
Summary
Adds a
ghostvariant to the Button component without treating ghost as a color.ghostboolean prop that can be combined with the existingcolorpropThis keeps semantic colors available for ghost buttons, so usages such as
<Button color="red" ghost>and<Button color="green" ghost>work without duplicating utility classes.Related issue
Closes #857
Breaking changes
None. The new
ghostprop is optional and existing Button behavior is unchanged.Validation
Targeted unit tests are included for the new behavior. The repository CI is expected to run the full typecheck, format, test, lint, and build checks for this pull request.
Summary by CodeRabbit
New Features
Documentation