Phase 2.2: Modal Components Migration by OpenStaxClaude · Pull Request #2825 · openstax/rex-web

OpenStaxClaude · 2026-03-24T20:59:17Z

Summary

Migrates the Modal component system from styled-components to plain/global CSS, following the established patterns from Phase 1.1-1.5.

Related Jira ticket: CORE-1696

Changes

New Files

Modal.css - Global CSS classes for all modal components
Modal.tsx - Plain React component wrappers using CSS classes
styles.legacy.tsx - Legacy styled-components exports for backward compatibility

Modified Files

index.tsx - Updated to import from ./Modal instead of ./styles
styles.tsx - Converted to redirect to ./styles.legacy for backward compatibility

Components Migrated

All 10 modal styled-components have been migrated to plain/global CSS:

ModalWrapper - Full-screen fixed overlay container with flexbox centering
CardWrapper - Z-index container
Card - Main modal card with shadow and typography
Header - Header section with background and border
Heading - Title with h4 typography styles
BodyHeading - Secondary heading with h3 typography
Body - Content area with padding
Mask - Semi-transparent backdrop overlay
Footer - Footer section with flexbox layout
CloseModalIcon - Close button with icon and hover states

Backward Compatibility

Maintains full backward compatibility for existing consumers:

ErrorModal - Imports Modal, Body, BodyHeading, Footer, modalPadding
ConfirmationModal - Imports and extends Footer styled-component
Footer ContactDialog - Extends Modal as styled-component

All existing imports from Modal/styles continue to work through the legacy exports.

Dependencies Handled

✅ Typography legacy exports (h3Style, h4Style, bodyCopyRegularStyle) - used for styling reference
✅ Theme system (colors, z-index) - bound as CSS variables
✅ Toolbar icon styles - inlined directly in CSS

Migration Strategy

Following the hybrid approach from Phase 1.1-1.5:

New implementations use plain/global CSS + React (no styled-components)
Legacy exports maintain styled-components wrappers for backward compatibility
Allows incremental migration without breaking existing consumers
CSS variables bound from theme for runtime values

Testing

Unit tests pass (existing snapshot tests)
TypeScript compilation succeeds
All existing Modal consumers remain functional through legacy exports
CI will verify screenshot tests and browser tests

Success Criteria

All Modal components migrated to plain/global CSS
Backward compatibility maintained for ErrorModal, ConfirmationModal, ContactDialog
No styled-components imports in new Modal.tsx
Legacy exports available in styles.legacy.tsx
TypeScript compilation succeeds
All tests pass (CI will verify)
Screenshot tests pass (CI will verify)

Follow-up Work

Future phases can migrate consuming components (ErrorModal, ConfirmationModal, ContactDialog) to use plain CSS Modal components directly, after which we can remove the legacy exports.

🤖 Generated with Claude Code

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

RoyEJohnson

Can you resolve the conflicts in this PR?

Migrates the Modal component system from styled-components to plain CSS modules, following the established patterns from Phase 1.1-1.5. Changes: - Created Modal.css with CSS classes for all modal components - Created Modal.tsx with plain React component wrappers - Created styles.legacy.tsx for backward compatibility - Updated index.tsx to use new CSS-based components - Updated styles.tsx to redirect to legacy exports Components migrated: - ModalWrapper - Full-screen fixed overlay container - CardWrapper - Z-index container - Card - Main modal card with shadow - Header - Header section - Heading - Title with h4 typography - BodyHeading - Secondary heading with h3 typography - Body - Content area - Mask - Semi-transparent backdrop - Footer - Footer section - CloseModalIcon - Close button with icon Maintains backward compatibility for existing consumers: - ErrorModal (imports Body, BodyHeading, Footer, modalPadding) - ConfirmationModal (imports and extends Footer) - Footer ContactDialog (extends Modal) All styled-component dependencies inlined or converted to CSS variables. Related: CORE-1696

1. Fixed duplicate padding in .modal-body-heading CSS - Removed the overridden padding declaration (1.5rem 0 1rem 0) - Kept the actual applied padding (1.5rem 0) - Eliminates confusion about which padding value is used 2. Explained why styled() wrappers must be kept - Added comprehensive comment explaining that styled() wrappers are required - Consumers like ConfirmationModal extend components using styled(Footer) - Direct re-exports would break this functionality - Trailing whitespace is already handled by normalizeClassName() helper These changes address the last two unresolved Copilot review comments while maintaining full backward compatibility with existing code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Address Copilot review comments for Modal migration This commit addresses all three issues raised by Copilot's automated review: 1. Add link styling to modal card (.modal-card a) - Mirrors bodyCopyRegularStyle link behavior with underline and hover states - Uses link color constants from Typography (linkColor, linkHover) - Binds link colors as CSS variables in Card component 2. Fix CloseModalIcon size for pixel-perfect parity - Changed from 2.5rem to 2rem (height and width) - Matches the final size from legacy styled-components implementation 3. Ensure type="button" cannot be overridden in CloseModalIcon - Moved type="button" after {...props} spread - Prevents consumer-provided type from overriding (e.g., submit in forms) - Maintains previous behavior as a non-submitting close button All changes verified with successful build. 🤖 Generated with [Claude Code](https://claude.com/claude-code) snaps Filter styled-components theme prop from DOM spreading Addresses Copilot review comment about styled-components injecting a theme prop that should not be spread to DOM elements. Changes: - Added PropsWithPossibleTheme helper type to document the pattern - Filter out theme prop before spreading ...props to DOM elements - Applied consistently across all Modal components: - ModalWrapper, CardWrapper, Card, Header, Heading, BodyHeading, Body, Mask, Footer, CloseModalIcon - Use proper type assertion (not 'as any') to satisfy linting This prevents React warnings about invalid DOM attributes and ensures clean HTML output without theme="[object Object]" attributes. Also added comprehensive documentation to PLAIN_CSS_MIGRATION_LEARNINGS.md explaining: - The issue and why it occurs - Warning signs to watch for - The resolution pattern - Examples for both regular and forwardRef components 🤖 Generated with [Claude Code](https://claude.com/claude-code) Address Copilot review comments: improve code quality and documentation - Fix CSS comment references to point to correct Typography source files (Headings.legacy.ts instead of Typography.legacy.ts) - Update Modal.tsx header comment from 'CSS modules' to 'plain/global CSS' to accurately describe the implementation - Remove unnecessary type casting in theme prop filtering by destructuring theme directly from props parameter, maintaining strong typing throughout - All components now use direct destructuring instead of casting to Record<string, unknown> These changes improve code clarity, maintainability, and type safety while maintaining the same runtime behavior and backward compatibility. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Address Copilot review comments: Fix CSS fallbacks and normalize classNames - Updated --header-border fallback from #d5d5d5 to #fafafa (matches theme.color.neutral.darker) - Updated --icon-color-lighter fallback from #6f6f6f to #c5c5c5 (matches theme.color.primary.gray.lighter) - Updated --icon-color-base fallback from #424242 to #5e6062 (matches theme.color.primary.gray.base) These changes ensure the CSS fallback values match the actual theme values that are bound as CSS variables, preventing incorrect styling if CSS variables aren't provided. - Added normalizeClassName helper to remove whitespace-only className values - Applied normalization to all Modal components before passing to classNames() - Prevents trailing/empty classes like `class="modal-body "` from legacy styled-components wrappers - This will be self-resolving once legacy styled-components are removed in future phases All changes verified with successful build. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Remove normalizeClassName/finalizeClassName helpers and document trailing space test artifact The trailing space issue in test snapshots is not a runtime problem but a test artifact caused by Jest's snapshot serialization process stripping out styled-components class names. Changes: - Removed normalizeClassName() helper function from Modal.tsx - Removed finalizeClassName() helper function from Modal.tsx - Simplified all component className assignments to use classNames() directly - Added comprehensive documentation in Modal.spec.tsx explaining the test artifact - Updated styles.legacy.tsx to remove reference to the removed helper Explanation of the test artifact: - Legacy styled() wrappers generate class names like "modal-body styleslegacy__Body-m93cxn-6" - Jest's snapshot serialization strips the styled-components generated classes - This leaves "modal-body " with a trailing space in snapshots only - At runtime, browsers receive the full className with no trailing space issue - This artifact will resolve when legacy wrappers are removed in future phases Addresses review comment from RoyEJohnson in review #16 (PRR_kwDOCVMVFM7vdUKY). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Fix trailing space issue in Modal className attributes Added finalizeClassName() helper to trim the output of classNames() library, which can produce trailing spaces when some arguments are empty strings. This addresses Copilot's review comment about trailing spaces appearing in snapshots (e.g., "modal-body ", "modal-body-heading ", "modal-footer "). The fix: 1. Enhanced normalizeClassName() to be more explicit about trimming 2. Added finalizeClassName() helper that trims the final computed className 3. Applied finalizeClassName() to all Modal components to ensure clean output This ensures the DOM never receives trailing-space class attributes while maintaining backward compatibility with legacy styled-components wrappers. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Use useIntl instead of FormatMessage to guarantee string Fix Modal component accessibility and TypeScript issues - Add React.forwardRef to CloseModalIcon to accept refs - Explicitly accept and render children in Heading and BodyHeading components - Fix aria-label type by casting to string - Break long lines to meet max-len linting requirements This addresses the code review feedback to fix TypeScript compilation errors and jsx-a11y/heading-has-content warnings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) snaps Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-Authored-By: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Added dual stylelint configuration to support the hybrid migration approach where styled-components (.tsx) and plain CSS (.css) coexist during the transition period. Changes: - Created .stylelintrc.css.json for plain CSS files - Uses stylelint-config-standard without styled-components processor - Maintains same linting rules as .stylelintrc for consistency - Updated package.json scripts: - lint:css now runs both lint:css:tsx and lint:css:plain - lint:css:tsx: Lints .tsx files with styled-components (existing) - lint:css:plain: Lints .css files with new config This addresses Copilot's review comment about Modal.css not being covered by CI stylelint. Now both legacy styled-components and new plain CSS files will be linted during the build process. Eventually, when all styled-components are migrated to plain CSS, we can simplify to a single stylelint configuration for .css files only. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Fix remaining CSS lint errors across the codebase Addressed all remaining stylelint errors and warnings identified in review #21: 1. src/index.css: - Moved @import statements to top of file (lines 1-2) to fix no-invalid-position-at-import-rule errors 2. src/app/components/Checkbox.css: - Added stylelint-disable comment for intentional duplicate outline (webkit fallback) - Added stylelint-disable block for no-descending-specificity warnings (focus-within selectors intentionally override input state selectors for proper CSS cascade) 3. src/app/components/ScrollLock.css: - Added stylelint-disable comment for !important on overflow (must override scroll lock for printing) 4. src/app/content/components/ContentPane.css: - Added stylelint-disable comments for !important on padding-left (must override responsive padding when sidebar is closed) - Added stylelint-disable block for BEM modifier class names with double dashes 5. src/app/components/Typography/Links.css: - Added stylelint-disable block for BEM modifier class names with double dashes All lint disables include explanatory comments. These intentional overrides are necessary for proper functionality (focus states, print styles, responsive behavior, BEM naming convention). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Fix (most) css lint issues Fix stylelint errors in Modal.css Fixed all stylelint errors identified in lint.output: - Added empty lines before comments (comment-empty-line-before) - Converted hex colors to lowercase (#027EB5 → #027eb5, #0064A0 → #0064a0) - Removed trailing zeros from rem values (3.0rem → 3rem) All changes are formatting/style-only and don't affect functionality. 🤖 Generated with [Claude Code](https://claude.com/claude-code) accommodate BEM patterns Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-Authored-By: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

@font-face

1. Fixed quote consistency in Modal/index.tsx - Changed formatMessage parameter from double quotes to single quotes - Maintains consistency with rest of file 2. Removed unused --link-hover CSS variable - Modal.css only references --link-hover-color - No Typography components use --link-hover - Keeps JS/CSS contract minimal and clear 3. Added explanatory comment for @import positioning in index.css - Clarifies that external fonts complement local @font-face declarations - IBM Plex Mono (external) is for code/monospace usage - Neue Helvetica (local @font-face) is for body text - No conflicts or double-loading 🤖 Generated with [Claude Code](https://claude.com/claude-code) Address final Copilot review comments - Fix Checkbox.css: Change 'Highlight' to 'highlight' for stylelint compliance CSS keywords are case-insensitive, using lowercase for consistency - Fix Modal.tsx: Add --link-hover variable alongside --link-hover-color Ensures compatibility with both Modal.css and Typography link components that use different variable names for hover colors 🤖 Generated with [Claude Code](https://claude.com/claude-code) Lint for Button.css (added from main after 2.1 merged) Update index.css Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-Authored-By: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

OpenStaxClaude · 2026-04-09T19:36:10Z

Rebase Complete (Addressing Review #33)

I've successfully rebased the phase-2.2-modal-migration branch onto the latest origin/main.

Rebase Summary

✅ No conflicts encountered - The rebase completed cleanly
✅ Branch is now up-to-date with all recent content updates from main
✅ 72 new commits from main incorporated (primarily content updates #2895-#2906)
✅ Force-pushed to phase-2.2-modal-migration with --force-with-lease

Current State

The PR branch now includes:

All Modal component migration changes (plain CSS implementation)
All recent content updates from main branch
All previous review feedback addressed

Unresolved Inline Comment Status

The one remaining "unresolved" inline comment about Highlight keyword casing in Checkbox.css is already fixed - the comment is marked as OUTDATED because the code has been updated to use lowercase highlight (line 58).

The branch is ready for CI checks and final review.

Dantemss · 2026-04-13T15:29:49Z

+export function ModalWrapper(
+  { className, style, ...props }: React.HTMLAttributes<HTMLDivElement> & { theme?: unknown }
+) {
+  const { theme: _theme, ...domProps } = props as any;


This is just the plan but you gotta watch out for Claude randomly casting stuff as any.

Lint calls them out.

jivey

No blockers but left some comments to consider cutting down on snapshot noise and improve the DX

jivey · 2026-04-14T14:46:29Z

+      {...props}
+      className={classNames('modal-wrapper', className)}
+      style={{
+        '--modal-z-index': theme.zIndex.errorPopup,


These might be nicer as root variables that you reference in the modal css file

jivey · 2026-04-14T14:48:26Z

+ * NOTE ON TRAILING SPACES IN SNAPSHOTS:
+ *
+ * The snapshot tests may show trailing spaces in className attributes (e.g., "modal-body ").
+ * This is a test artifact caused by Jest's snapshot serialization process.


Can't we avoid this by using || null with the classnames var?

TomWoodward temporarily deployed to rex-web-phase-2-2-modal-wvhwrc March 24, 2026 20:59 Inactive

This comment was marked as resolved.

Sign in to view

RoyEJohnson assigned OpenStaxClaude Mar 24, 2026

This comment was marked as resolved.

Sign in to view

TomWoodward temporarily deployed to rex-web-phase-2-2-modal-if7lvu March 24, 2026 22:36 Inactive

RoyEJohnson force-pushed the phase-2.2-modal-migration branch from 9e350cc to d2fd171 Compare March 26, 2026 12:41

TomWoodward temporarily deployed to rex-web-phase-2-2-modal-if7lvu March 26, 2026 12:42 Inactive

RoyEJohnson requested a review from Copilot March 26, 2026 14:33

Copilot started reviewing on behalf of RoyEJohnson March 26, 2026 14:33 View session

This comment was marked as resolved.

Sign in to view

TomWoodward temporarily deployed to rex-web-phase-2-2-modal-if7lvu March 26, 2026 14:56 Inactive

RoyEJohnson force-pushed the phase-2.2-modal-migration branch from 879d0d1 to c4d9e5e Compare March 26, 2026 15:01

TomWoodward temporarily deployed to rex-web-phase-2-2-modal-if7lvu March 26, 2026 15:01 Inactive

RoyEJohnson force-pushed the phase-2.2-modal-migration branch from c4d9e5e to 7e1e43c Compare March 26, 2026 15:13

TomWoodward temporarily deployed to rex-web-phase-2-2-modal-if7lvu March 26, 2026 15:14 Inactive

RoyEJohnson requested a review from Copilot March 26, 2026 15:26

Copilot started reviewing on behalf of RoyEJohnson March 26, 2026 15:27 View session

This comment was marked as resolved.

Sign in to view

TomWoodward temporarily deployed to rex-web-phase-2-2-modal-if7lvu March 26, 2026 16:03 Inactive

This comment was marked as resolved.

Sign in to view

RoyEJohnson requested a review from Copilot March 26, 2026 16:04

Copilot started reviewing on behalf of RoyEJohnson March 26, 2026 16:05 View session

This comment was marked as resolved.

Sign in to view

RoyEJohnson force-pushed the phase-2.2-modal-migration branch from cfa1abe to 631325f Compare March 26, 2026 16:52

TomWoodward temporarily deployed to rex-web-phase-2-2-modal-if7lvu March 26, 2026 16:53 Inactive

This comment was marked as resolved.

Sign in to view

RoyEJohnson force-pushed the phase-2.2-modal-migration branch from 061e284 to 2a71a53 Compare March 27, 2026 14:01

TomWoodward temporarily deployed to rex-web-phase-2-2-modal-if7lvu March 27, 2026 14:02 Inactive

This comment was marked as resolved.

Sign in to view

RoyEJohnson requested changes Apr 9, 2026

View reviewed changes

OpenStaxClaude and others added 4 commits April 9, 2026 19:35

staxly Bot added 6 commits April 10, 2026 10:44

Merge branch 'main' into phase-2.2-modal-migration

12d31bd

Merge branch 'main' into phase-2.2-modal-migration

61d1126

Merge branch 'main' into phase-2.2-modal-migration

61bbdec

Merge branch 'main' into phase-2.2-modal-migration

ba7c4a3

Merge branch 'main' into phase-2.2-modal-migration

be6570f

Merge branch 'main' into phase-2.2-modal-migration

47f0dab

Dantemss approved these changes Apr 13, 2026

View reviewed changes

staxly Bot added 2 commits April 13, 2026 15:41

Merge branch 'main' into phase-2.2-modal-migration

14de174

Merge branch 'main' into phase-2.2-modal-migration

2ed1f27

jivey approved these changes Apr 14, 2026

View reviewed changes

staxly Bot added 2 commits April 14, 2026 15:47

Merge branch 'main' into phase-2.2-modal-migration

12a619b

Merge branch 'main' into phase-2.2-modal-migration

ad5ca63

RoyEJohnson approved these changes Apr 14, 2026

View reviewed changes

Conversation

OpenStaxClaude commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

New Files

Modified Files

Components Migrated

Backward Compatibility

Dependencies Handled

Migration Strategy

Testing

Success Criteria

Follow-up Work

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

Uh oh!

RoyEJohnson left a comment

Choose a reason for hiding this comment

Uh oh!

OpenStaxClaude commented Apr 9, 2026

Rebase Complete (Addressing Review #33)

Rebase Summary

Current State

Unresolved Inline Comment Status

Uh oh!

Uh oh!

Dantemss Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

RoyEJohnson Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

jivey left a comment

Choose a reason for hiding this comment

Uh oh!

jivey Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

jivey Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

OpenStaxClaude commented Mar 24, 2026 •

edited

Loading