Skip to content

Phase 2.1: Button System Migration#2824

Open
OpenStaxClaude wants to merge 5 commits intomainfrom
phase-2.1-button-migration
Open

Phase 2.1: Button System Migration#2824
OpenStaxClaude wants to merge 5 commits intomainfrom
phase-2.1-button-migration

Conversation

@OpenStaxClaude
Copy link
Contributor

@OpenStaxClaude OpenStaxClaude commented Mar 23, 2026

Summary

Migrates the button component system from styled-components to plain CSS modules, following the established patterns from Phases 1.1-1.5. This migration maintains backward compatibility through legacy exports while providing new CSS-based implementations.

Resolves: CORE-1695

Components Migrated

All components in src/app/components/Button.tsx:

  1. Button - Main button component with:

    • Variants: primary, secondary, transparent, default
    • Sizes: large, medium (default), small
    • Disabled state support
    • Component prop for polymorphic rendering
  2. PlainButton - Unstyled button base (no margin, padding, border, background)

  3. ButtonLink - Button styled as a link (uses PlainButton + link styles)

  4. ButtonGroup - Container for button groups with grid layout

Changes

Created Files

  • src/app/components/Button.css - Plain CSS styles for all button components
  • src/app/components/Button.legacy.ts - Legacy styled-components exports for backward compatibility

Modified Files

  • src/app/components/Button.tsx - Refactored to use plain CSS classes and CSS variables
  • src/app/components/Button.spec.tsx - Enhanced with additional test coverage

Migration Approach

Hybrid Strategy

Following Phase 1.1-1.5 patterns:

  • New implementations use plain CSS + React (no styled-components)
  • Legacy exports in Button.legacy.ts for backward compatibility
  • Allows incremental migration without breaking ~40+ call sites across the codebase

Component Polymorphism

The current Button component supports a component prop for polymorphic rendering. This is preserved using React.cloneElement to maintain compatibility.

Theme Integration

CSS variables are bound for theme colors:

  • Primary variant: theme.color.primary.orange
  • Secondary variant: theme.color.secondary.lightGray
  • Default variant: theme.color.neutral
  • Disabled state: theme.color.disabled
  • Transparent variant: link colors

Migration Checklist

Following PLAIN_CSS_MIGRATION_LEARNINGS.md:

  • ✅ Use classnames package for className composition
  • ✅ Use normal function declarations (not React.FC)
  • ✅ Set CSS variables before ...style spread
  • ✅ Provide CSS variable fallbacks in CSS file
  • ✅ Create .legacy.ts for backward compatibility
  • ✅ Comprehensive test coverage for all variants
  • ✅ Update snapshots (will be done by CI)
  • ✅ No duplicate exports in index

Testing

Enhanced test coverage includes:

  • All button variants (primary, secondary, transparent, default)
  • All sizes (small, medium, large)
  • Disabled state
  • Custom className composition
  • Custom style prop merging
  • PlainButton base functionality
  • ButtonLink with decorated/undecorated states
  • ButtonGroup layout options (expand, vertical)
  • Component polymorphism

Success Criteria

  • All button components migrated to plain CSS
  • All existing tests pass (pending CI)
  • Visual appearance unchanged (screenshot tests)
  • No breaking changes for existing consumers
  • TypeScript compilation succeeds (pending CI)
  • Legacy exports functional for backward compatibility

Notes

  • CloseButton is not in Button.tsx - it's defined separately in multiple locations (Topbar, SidebarControl, ToastNotifications, etc.) and will be handled separately if needed
  • This PR is based on main branch - no conflicts with Phase 1.5 (which only touches GoToTopButton, a separate component)

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com

Migrates Button, PlainButton, ButtonLink, and ButtonGroup components
from styled-components to plain CSS modules while maintaining backward
compatibility.

Changes:
- Created Button.css with all button styles and variants
- Refactored Button.tsx to use plain CSS classes and CSS variables
- Created Button.legacy.ts for backward compatibility with styled-components
- Enhanced Button.spec.tsx with additional test coverage

Key features preserved:
- All button variants (primary, secondary, transparent, default)
- All sizes (small, medium, large)
- Disabled state
- Component polymorphism via component prop
- ButtonGroup layout options
- ButtonLink decorated/undecorated modes

Follows migration patterns from PLAIN_CSS_MIGRATION_LEARNINGS.md:
- Uses classnames package for className composition
- Normal function declarations (not React.FC)
- CSS variables set before ...style spread
- CSS variable fallbacks in CSS file
- Legacy exports in separate .legacy.ts file

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@TomWoodward TomWoodward temporarily deployed to rex-web-phase-2-1-butto-gwxbab March 23, 2026 15:35 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-phase-2-1-butto-4bpl95 March 23, 2026 16:16 Inactive
RoyEJohnson

This comment was marked as duplicate.

RoyEJohnson

This comment was marked as resolved.

OpenStaxClaude added a commit that referenced this pull request Mar 23, 2026
- Add React.forwardRef to Button, PlainButton, and ButtonLink components to support ref forwarding
- Remove invalid 'default' prop from Button usage in ShowAnswerButton and SkipAndSubmitButtons
- Fixes TypeScript errors where ref was not allowed on Button components

This addresses review feedback from PR #2824 where the linter identified that ref was not an allowed prop.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

This comment was marked as resolved.

@TomWoodward TomWoodward temporarily deployed to rex-web-phase-2-1-butto-fco9wj March 23, 2026 16:33 Inactive
@TomWoodward TomWoodward had a problem deploying to rex-web-phase-2-1-butto-m3fvjo March 23, 2026 16:53 Failure
RoyEJohnson

This comment was marked as resolved.

@OpenStaxClaude

This comment was marked as resolved.

@RoyEJohnson RoyEJohnson force-pushed the phase-2.1-button-migration branch from fb1c063 to d110c18 Compare March 23, 2026 17:24
@TomWoodward TomWoodward temporarily deployed to rex-web-phase-2-1-butto-rpu5ih March 23, 2026 17:24 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-phase-2-1-butto-m3fvjo March 23, 2026 17:24 Inactive
RoyEJohnson

This comment was marked as resolved.

@OpenStaxClaude

This comment was marked as resolved.

@TomWoodward TomWoodward had a problem deploying to rex-web-phase-2-1-butto-rpu5ih March 23, 2026 17:29 Failure
RoyEJohnson

This comment was marked as resolved.

@TomWoodward TomWoodward had a problem deploying to rex-web-phase-2-1-butto-rpu5ih March 23, 2026 17:40 Failure
@OpenStaxClaude

This comment was marked as resolved.

@TomWoodward TomWoodward had a problem deploying to rex-web-phase-2-1-butto-rpu5ih March 23, 2026 17:56 Failure
RoyEJohnson

This comment was marked as resolved.

@OpenStaxClaude

This comment was marked as resolved.

@TomWoodward TomWoodward had a problem deploying to rex-web-phase-2-1-butto-rpu5ih March 23, 2026 17:59 Failure
@TomWoodward TomWoodward temporarily deployed to rex-web-phase-2-1-butto-rpu5ih March 23, 2026 18:05 Inactive
@RoyEJohnson RoyEJohnson force-pushed the phase-2.1-button-migration branch from 02d3487 to 8591bce Compare March 23, 2026 18:25
@TomWoodward TomWoodward temporarily deployed to rex-web-phase-2-1-butto-rpu5ih March 23, 2026 18:25 Inactive
RoyEJohnson

This comment was marked as resolved.

@TomWoodward TomWoodward temporarily deployed to rex-web-phase-2-1-butto-rpu5ih March 23, 2026 19:21 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-phase-2-1-butto-rpu5ih March 23, 2026 19:41 Inactive
RoyEJohnson

This comment was marked as resolved.

@OpenStaxClaude

This comment was marked as resolved.

@TomWoodward TomWoodward temporarily deployed to rex-web-phase-2-1-butto-rpu5ih March 23, 2026 19:54 Inactive
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@RoyEJohnson RoyEJohnson force-pushed the phase-2.1-button-migration branch from 38340c9 to c3e77cc Compare March 23, 2026 19:59
@TomWoodward TomWoodward temporarily deployed to rex-web-phase-2-1-butto-rpu5ih March 23, 2026 19:59 Inactive
RoyEJohnson

This comment was marked as resolved.

@OpenStaxClaude

This comment was marked as resolved.

@OpenStaxClaude
Copy link
Contributor Author

Summary of All Review Feedback - Phase 2.1 Button Migration

All review comments (Reviews 1-11) have been addressed. Here's a complete summary:

✅ Reviews 1-3: TypeScript Ref Errors

Fixed: Added React.forwardRef support to Button, PlainButton, and ButtonLink components. Removed invalid default prop usage. Fixed type shadowing issues with HTMLButtonElement imports.

✅ Reviews 4-9: PrettyFormatPluginError in Snapshots

Fixed: Applied multiple fixes following the established Phase 1 patterns:

  1. Moved Button.css import to global imports in index.tsx (prevents jest-styled-components from parsing CSS)
  2. Wrapped Button, PlainButton, ButtonLink, and ButtonGroup with styled() in files that use them in component selectors:
    • EditCard.tsx
    • Card.tsx (notifications)
    • NextSectionMessage.tsx
    • QuestionNavigation/index.tsx
    • IntroScreen.tsx
    • ChapterFilter.tsx
    • AllOrNone.tsx

✅ Review 10: decoratedLinkStyle Coverage

Verified: The decorated prop and .button-link-decorated styles are actively used in AllOrNone.tsx and properly tested in Button.spec.tsx. No changes needed.

⚠️ Review 11: Typography.legacy.ts Coverage Mystery

Investigated: The uncovered disabled branch in decoratedLinkStyle is a pre-existing issue from Phase 1.1 (Typography migration), not introduced by this PR. None of the 3 production usages (PrevNextBar, Attribution, Topbar) pass a disabled prop. See comment above for detailed analysis and recommendations.

Files Changed in This PR

  • New: Button.css, Button.legacy.ts (if not already created)
  • Modified: Button.tsx, Button.spec.tsx, DotMenu.tsx, NextButton.tsx, ShowAnswerButton.tsx, SkipAndSubmitButtons.tsx, index.tsx (global CSS imports), and 6 files with styled() wrappers
  • Updated: 30+ snapshot files

Status

All Phase 2.1 requirements completed. All review feedback through Review 10 resolved. Review 11 requires decision on whether to address pre-existing Typography.legacy.ts coverage issue.

🤖 Generated with Claude Code

RoyEJohnson

This comment was marked as resolved.

Removed the unused disabled prop conditional from decoratedLinkStyle in Typography.legacy.ts.
This conditional was never used in production code - all 3 usages (PrevNextBar.tsx, Attribution.tsx,
and Topbar/styled.tsx) do not pass a disabled prop.

This change:
- Improves code coverage by removing untested dead code
- Simplifies the decoratedLinkStyle implementation
- Has no impact on existing functionality since the branch was never executed

Addresses review feedback from PR #2824 Review 12.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

This comment was marked as resolved.

This comment was marked as resolved.

RoyEJohnson

This comment was marked as resolved.

…riable ordering

1. Add disabled state CSS styling for ButtonLink:
   - Added cursor: not-allowed, opacity: 0.6, pointer-events: none
   - Disabled decorated underline animation (width: 0, transparent background)
   - Supports both [disabled] and [aria-disabled='true'] attributes
   - Required because AllOrNone.tsx uses ButtonLink with disabled prop

2. Fix CSS variable ordering in ButtonLink style prop:
   - Changed order from CSS vars first, then ...style
   - To: ...style first, then CSS vars
   - This ensures CSS variables take precedence over user-provided styles
   - Prevents custom styles from accidentally overriding theme colors

Addresses Review 13-14 feedback and Copilot inline comments.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

This comment was marked as resolved.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 51 out of 51 changed files in this pull request and generated no new comments.


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

@RoyEJohnson RoyEJohnson marked this pull request as ready for review March 23, 2026 21:34
@RoyEJohnson RoyEJohnson requested a review from a team as a code owner March 23, 2026 21:34
@RoyEJohnson RoyEJohnson self-requested a review March 23, 2026 21:34
@TomWoodward TomWoodward requested a deployment to rex-web-phase-2-1-butto-rpu5ih March 23, 2026 21:41 Pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants