feat(ui): Reusable Button component enhancement by aamarin · Pull Request #1131 · Vets-Who-Code/vets-who-code-app

aamarin · 2026-05-28T01:02:41Z

Summary

Add ghost variant and secondary/danger colors to the shared Button component
Wrap with React.forwardRef and extend ButtonHTMLAttributes — refs and native HTML attributes pass through automatically
Replace defaultProps with destructure parameter defaults
Add barrel export (src/components/ui/index.ts) with @ui alias in tsconfig.json
Update 4 callsites: cta, contact-form, PlayerSetup, admin-dashboard
Remove dead btnPrimary/btnGhost CSS from cta.module.css
Add JSDoc for variant/color matrix and path→Anchor behavior
Add 21-case test suite covering all spec acceptance criteria

Test plan

npx tsc --noEmit — zero errors
pnpm vitest run __tests__/components/ui/button.test.tsx — all 21 cases pass
Verify <Button variant="ghost" color="secondary"> renders without background or border
Verify forwarded ref resolves to HTMLButtonElement
Verify import { Button } from "@ui" resolves correctly
Smoke-test CTA form, contact form, PlayerSetup, and admin dashboard in browser

Closes #896

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@ui

- Add ghost variant and secondary/danger colors to Button - Wrap with React.forwardRef and extend ButtonHTMLAttributes - Replace defaultProps with destructure parameter defaults - Add barrel export (src/components/ui/index.ts) with @ui alias in tsconfig - Update 4 callsites: cta, contact-form, PlayerSetup, admin-dashboard - Remove dead btnPrimary/btnGhost CSS from cta.module.css - Add JSDoc for variant/color matrix and path→Anchor behavior - Add 21-case test suite covering all spec acceptance criteria - Consolidate planning artifacts to docs/button/ Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

vercel · 2026-05-28T01:02:47Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
vets-who-code-app	Ready	Preview, Comment	May 28, 2026 9:51pm

Copilot

Pull request overview

This PR enhances the shared UI Button component to better support reuse across the app (new ghost variant, new semantic colors, ref/attribute pass-through), adds a @ui barrel export, updates several callsites to use the shared button, and introduces a dedicated Button test suite plus supporting design/spec docs.

Changes:

Extend Button API with ghost variant and secondary/danger colors; switch to forwardRef and ButtonHTMLAttributes + rest spread.
Add @ui barrel export + TS path alias and migrate 4 callsites to the shared Button; remove CTA dead CSS.
Add Button-focused docs and a 21-case vitest test suite.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tsconfig.json	Adds `@ui` alias for importing from the UI barrel.
src/containers/software-factory/cta/index.tsx	Migrates CTA buttons to the shared `Button` component.
src/containers/software-factory/cta/cta.module.css	Removes now-unused CTA button styles.
src/components/ui/index.ts	Adds UI barrel export for `Button`.
src/components/ui/button/index.tsx	Implements new variants/colors, `forwardRef`, and native-attribute pass-through.
src/components/game/PlayerSetup.tsx	Replaces raw submit button with shared `Button`.
src/components/forms/contact-form.tsx	Simplifies submit button styling by relying on shared `Button` defaults.
src/components/admin-dashboard-components.tsx	Replaces raw “Add User” button with shared `Button`.
docs/button/tasks.md	Adds task breakdown for the Button enhancement work.
docs/button/spec.md	Adds feature spec for Button enhancements and acceptance criteria.
docs/button/plan.md	Adds implementation plan and technical notes for the Button work.
docs/button/design.md	Adds design rationale and API notes for the Button enhancements.
docs/button/delivery.md	Adds delivery/verification plan for the Button enhancement feature.
tests/components/ui/button.test.tsx	Adds new test suite covering key Button behaviors and variant/color matrix rendering.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

aamarin · 2026-05-28T02:18:28Z

+interface ButtonProps
+    extends Omit<React.ButtonHTMLAttributes<HTMLButtonElement>, "id" | "onClick"> {
    children: React.ReactNode;
-    type?: "button" | "submit" | "reset";
-    variant?: "contained" | "outlined" | "texted";
-    color?: "primary" | "light";
+    variant?: "contained" | "outlined" | "texted" | "ghost";
+    color?: "primary" | "light" | "secondary" | "danger";
    size?: "xs" | "sm" | "md" | "lg";
    shape?: "tw-rounded" | "square" | "ellipse";
    fullwidth?: boolean;
    active?: boolean;
-    disabled?: boolean;
    iconButton?: boolean;
-    onClick?: () => void;
-    className?: string;
    path?: string;
    label?: string;
    hover?: "default" | "light" | false;
+    /** string | number to accept data-layer ids (HTML id is string-only) */
+    id?: string | number;
+    /** () => void to stay compatible with Anchor's onClick signature */
+    onClick?: () => void;
 }

-const Button = ({
-    children,
-    type,
-    variant,
-    color,
-    size,
-    shape,
-    fullwidth,
-    active,
-    disabled,
-    iconButton,
-    label,
-    className,
-    path,
-    onClick,
-    hover,
-}: ButtonProps) => {
+const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
+    (
+        {
+            children,
+            // eslint-disable-next-line @typescript-eslint/no-unused-vars
+            id: _id,
+            type = "button",


Fixed. id is now destructured, coerced to string via String(id), and explicitly passed to both <button> and <Anchor> as id={domId}. The string | number override is kept since ButtonType.id is typed as IDType = string | number and callsites spread it directly.

aamarin · 2026-05-28T02:18:30Z

+            ...rest
+        },
+        ref
+    ) => {


Fixed. ...rest is now spread onto <Anchor> as {...(rest as React.HTMLAttributes<HTMLAnchorElement>)} so aria-* and data-* attrs pass through in link mode.

aamarin · 2026-05-28T02:18:38Z

+    /** () => void to stay compatible with Anchor's onClick signature */
+    onClick?: () => void;


Fixed. Removed onClick from the Omit and changed the type to React.MouseEventHandler<HTMLButtonElement>. Consumers can now use (e) => e.preventDefault() style handlers. At the <Anchor> call site a cast to any is used because Anchor's onClick resolves to an intersection type (MouseEventHandler<HTMLAnchorElement> & (() => void)) that TypeScript cannot satisfy without it — widening Anchor itself is a separate change.

aamarin · 2026-05-28T02:18:40Z

    const fontUpgrade = variant !== "texted" ? "[font-family:var(--font-headline)]" : "";

    const classnames = clsx(
-        variant !== "texted" && baseClass,
-        variant !== "texted" && baseNotFullWidthClass,
-        variant === "contained" && [containedPrimaryBtn, containedLightBtn],
-        variant === "outlined" && [outlinedPrimaryBtn, outlinedLightBtn],
+        variant !== "texted" && baseLayoutClass,
+        variant !== "texted" && variant !== "ghost" && baseBorderClass,
+        variant !== "texted" && variant !== "ghost" && baseNotFullWidthClass,
+        variant === "ghost" && !fullwidth && "tw-inline-flex",
+        variant === "contained" && [containedPrimaryBtn, containedLightBtn, containedSecondaryBtn, containedDangerBtn],
+        variant === "outlined" && [outlinedPrimaryBtn, outlinedLightBtn, outlinedSecondaryBtn, outlinedDangerBtn],
+        variant === "ghost" && ghostBtn,


Fixed. Added disabled && 'tw-opacity-50 tw-cursor-not-allowed' to the clsx composition, restoring the visual disabled state that was previously handled by the now-deleted .btnPrimary:disabled CSS rule.

- Forward id to DOM (coerce string|number to string via String(id)) - Spread ...rest onto <Anchor> for aria-*/data-* pass-through in link mode - Widen onClick to React.MouseEventHandler<HTMLButtonElement> - Add tw-opacity-50 tw-cursor-not-allowed for disabled visual state - Fix test assertion to verify disabled opacity class Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

aamarin · 2026-05-28T12:57:08Z

+import Button from "@ui/button";
+
+describe("Button", () => {
+    // T009 — renders children, default type, disabled state


Remove speckit specific nomenclature such as T009. Keep general comments and add tsdoc at the model-level as well

Fixed. Removed T009–T012 speckit identifiers from all section comments (kept the descriptions). Added module-level TSDoc at the top of the test file.

aamarin · 2026-05-28T12:59:04Z

@@ -1,45 +1,64 @@
 /* eslint-disable react/button-has-type */
+import React from "react";


Add module-level tsdoc

Fixed. Added a @module TSDoc block at the top of button/index.tsx (above the eslint-disable directive) covering variants, colors, and rendering modes.

aamarin · 2026-05-28T13:00:37Z

    color: #f8f9fa;
 }

-.btnPrimary,


This is being taken into consideration in cslx now, or did we lose this styling completely?

The disabled styling is preserved. disabled && 'tw-opacity-50 tw-cursor-not-allowed' is composed in the Button's clsx call (line 220 of src/components/ui/button/index.tsx). The old CSS rule that was at this location in the module is fully replaced by those Tailwind utilities — nothing was lost.

aamarin · 2026-05-28T13:01:55Z

@@ -1,3 +1,4 @@
+import { Button } from "@ui";


Check for other modules and cta examples. Do we normally use named exports for components in React?

Checked. The project convention is import Button from "@ui/button" (default import via path alias) — used by ~25 existing callsites. Named barrel imports (import { Button } from "@ui") were introduced only on the 3 files migrated in this PR (cta/index.tsx, admin-dashboard-components.tsx, PlayerSetup.tsx). All 3 are now updated to import Button from "@ui/button" to match the existing convention.

- Remove speckit T009-T012 ids from test comments; add module TSDoc to test file - Add module-level TSDoc to src/components/ui/button/index.tsx - Normalize Button imports to path-based default import across 3 files migrated in this PR (cta, admin-dashboard, PlayerSetup) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

aamarin · 2026-05-29T18:42:58Z

+    });
+
+    it("passes data-testid through to the button element", () => {
+        render(<Button data-testid="my-btn">Click</Button>);


Why use test ids in production code? Common in successful open source codebases

aamarin · 2026-05-29T18:49:48Z

    shape?: "tw-rounded" | "square" | "ellipse";
    fullwidth?: boolean;
    active?: boolean;
-    disabled?: boolean;


Why remove the disable feature?

aamarin · 2026-05-29T18:58:30Z

+interface ButtonProps
+    extends Omit<React.ButtonHTMLAttributes<HTMLButtonElement>, "id" | "onClick"> {
    children: React.ReactNode;
-    type?: "button" | "submit" | "reset";


I saw type used in button components

aamarin and others added 5 commits May 26, 2026 19:13

docs: Add Button component enhancement design spec (#896)

2f2350d

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs: Add speckit spec/plan/tasks for Button enhancement (#896)

47b0a02

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs: Apply speckit.clarify resolutions to spec and tasks (#896)

47f2d1d

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs: Add delivery plan for Button component enhancement (#896)

e6c6e35

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

aamarin requested a review from Copilot May 28, 2026 01:09

Copilot started reviewing on behalf of aamarin May 28, 2026 01:11 View session

Copilot AI reviewed May 28, 2026

View reviewed changes

vercel Bot deployed to Preview May 28, 2026 01:47 View deployment

aamarin marked this pull request as draft May 28, 2026 12:56

aamarin self-assigned this May 28, 2026

aamarin requested a review from jeromehardaway May 28, 2026 12:56

aamarin commented May 28, 2026

View reviewed changes

vercel Bot deployed to Preview May 28, 2026 21:51 View deployment

aamarin commented May 29, 2026

View reviewed changes

jeromehardaway approved these changes Jun 8, 2026

View reviewed changes

jeromehardaway marked this pull request as ready for review June 8, 2026 23:45

		/** () => void to stay compatible with Anchor's onClick signature */
		onClick?: () => void;

		@@ -1,45 +1,64 @@
		/* eslint-disable react/button-has-type */
		import React from "react";

Uh oh!

Uh oh!

Conversation

aamarin commented May 28, 2026

Summary

Test plan

Uh oh!

vercel Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vercel Bot commented May 28, 2026 •

edited

Loading