Phase 2.2: Modal Components Migration

PLAIN_CSS_MIGRATION_LEARNINGS.md

@@ -916,6 +916,85 @@ useEffect(() => {
 
 **For Future Phases**: After converting class components to functional components, review all state variables and ensure they're actually being used. Remove any that are set but never read.
 
+### Filter styled-components Internal Props Before Spreading to DOM
+
+**Issue (Phase 2.2 - Modal)**: When creating plain CSS/React components that are wrapped by styled-components in legacy exports (for backward compatibility), styled-components injects internal props like `theme` into the component. If these props are spread directly onto DOM elements using `{...props}`, React will produce warnings about invalid DOM attributes and render `theme="[object Object]"` in the HTML output.
+
+**Problem**:
+```typescript
+// ❌ Bad - theme prop leaks to DOM when wrapped by styled()
+export function ModalWrapper({ className, style, ...props }: React.HTMLAttributes<HTMLDivElement>) {
+  return (
+    <div
+      {...props}  // ⚠️ If styled() wraps this, theme will be spread to DOM!
+      className={classNames('modal-wrapper', className)}
+      style={{ ...style }}
+    />
+  );
+}
+
+// In styles.legacy.tsx
+export const ModalWrapper = styled(ModalComponents.ModalWrapper)``;
+// This injects a theme prop that will leak to the DOM
+```
+
+**Warning Signs**:
+- React warnings in console: "Warning: React does not recognize the `theme` prop on a DOM element"
+- Invalid HTML attributes in rendered output: `<div theme="[object Object]">`
+- Copilot/automated code review comments about styled-components prop injection
+
+**Resolution**: Widen the prop types to accept `theme?: unknown` and destructure it out before spreading to the DOM:
+
+```typescript
+// ✅ Good - filters out theme prop before spreading to DOM
+export function ModalWrapper(
+  { className, style, ...props }: React.HTMLAttributes<HTMLDivElement> & { theme?: unknown }
+) {
+  const { theme: _theme, ...domProps } = props as any;
+  return (
+    <div
+      {...domProps}  // ✅ theme filtered out, safe to spread
+      className={classNames('modal-wrapper', className)}
+      style={{ ...style }}
+    />
+  );
+}
+```
+
+**Pattern for forwardRef Components**:
+```typescript
+// ✅ Good - filters theme in forwardRef components too
+export const CloseModalIcon = React.forwardRef<
+  HTMLButtonElement,
+  React.ButtonHTMLAttributes<HTMLButtonElement> & { theme?: unknown }
+>(
+  function CloseModalIcon({ className, style, ...props }, ref) {
+    const { theme: _theme, ...domProps } = props as any;
+    return (
+      <button
+        ref={ref}
+        {...domProps}  // ✅ theme filtered out
+        type="button"
+        className={classNames('modal-close-icon', className)}
+      >
+        {/* ... */}
+      </button>
+    );
+  }
+);
+```
+
+**Why This Matters**:
+- Prevents React warnings about invalid DOM props
+- Prevents invalid HTML attributes in rendered output
+- Maintains clean separation between styled-components internals and DOM
+- Essential when using the hybrid migration approach with legacy styled-components wrappers
+
+**For Future Phases**: When creating plain CSS/React components that will be wrapped by styled-components in `.legacy.tsx` files, always:
+1. Add `& { theme?: unknown }` to the component's prop types
+2. Destructure `theme: _theme` out of props before spreading to DOM elements
+3. Apply this pattern to ALL components that spread props to DOM elements, including both regular components and `forwardRef` components
+
 ---
 
 ## Code Organization

src/app/components/Checkbox.css

@@ -55,7 +55,7 @@
   background-color: var(--checkbox-focus-bg, #f1f1f1); /* theme.color.neutral.pageBackground */
 
   /* Browser default focus outline */
-  outline: 0.2rem auto Highlight;
+  outline: 0.2rem auto highlight;
   /* stylelint-disable-next-line declaration-block-no-duplicate-properties */
   outline: 0.2rem auto -webkit-focus-ring-color; /* Fallback for webkit browsers */
 }

src/app/components/Modal/Modal.css

@@ -0,0 +1,166 @@
+/**
+ * Modal component styles
+ * Migrated from styled-components to plain CSS
+ */
+
+/* Modal wrapper - full screen fixed overlay */
+.modal-wrapper {
+  position: fixed;
+  top: 0;
+  left: 0;
+  width: 100%;
+  height: 100%;
+  display: flex;
+  justify-content: center;
+  align-items: center;
+  z-index: var(--modal-z-index, 10000);
+}
+
+/* Card wrapper - z-index container */
+.modal-card-wrapper {
+  z-index: 1;
+}
+
+/* Main modal card */
+.modal-card {
+  display: flex;
+  flex-direction: column;
+  margin: auto;
+  overflow: hidden;
+  width: 40rem;
+  background-color: white;
+  box-shadow: 0 0 2rem rgba(0, 0, 0, 0.05), 0 0 4rem rgba(0, 0, 0, 0.08);
+
+  /* bodyCopyRegularStyle - font-size: 1.6rem, line-height: 2.5rem, color from theme */
+  font-size: 1.6rem;
+  line-height: 2.5rem;
+  color: var(--text-color, #424242);
+}
+
+/* Link styling inside modal card - mirrors bodyCopyRegularStyle link behavior */
+.modal-card a {
+  color: var(--link-color, #027eb5);
+  cursor: pointer;
+  text-decoration: underline;
+}
+
+.modal-card a:hover {
+  color: var(--link-hover-color, #0064a0);
+}
+
+/* Modal header */
+.modal-header {
+  display: flex;
+  align-items: center;
+  margin-bottom: 1.5rem; /* modalPadding * 0.5 = 3.0 * 0.5 */
+  padding: 1.5rem 3rem; /* modalPadding * 0.5, modalPadding */
+  background: var(--header-bg, #f1f1f1);
+  border-bottom: solid 0.1rem var(--header-border, #fafafa);
+  justify-content: space-between;
+}
+
+/* Modal heading (h1 with h4 style) */
+.modal-heading {
+  /* h4Style from Typography/Headings.legacy.ts */
+  font-size: 1.8rem;
+  line-height: 2.5rem;
+  letter-spacing: -0.02rem;
+  padding: 1rem 0 1rem 0;
+  margin: 0;
+  color: var(--text-color, #424242);
+
+  /* Additional heading styles */
+  display: flex;
+  align-items: center;
+}
+
+@media screen and (max-width: 75em) {
+  .modal-heading {
+    /* h4MobileStyle */
+    font-size: 1.6rem;
+    line-height: 2rem;
+  }
+}
+
+/* Modal body heading (h3) */
+.modal-body-heading {
+  /* h3Style from Typography/Headings.legacy.ts */
+  font-size: 2.4rem;
+  line-height: 3rem;
+  letter-spacing: -0.02rem;
+  margin: 0;
+  color: var(--text-color, #424242);
+
+  /* Additional styles from BodyHeading */
+  font-weight: 400;
+  padding: 1.5rem 0; /* modalPadding * 0.5 */
+}
+
+@media screen and (max-width: 75em) {
+  .modal-body-heading {
+    /* h3MobileStyle */
+    font-size: 1.6rem;
+    line-height: 2rem;
+  }
+}
+
+/* Modal body */
+.modal-body {
+  display: flex;
+  flex-direction: column;
+  padding: 0 3rem; /* modalPadding */
+}
+
+/* Special case: when Card.Header + Body, remove top margin */
+.modal-card .modal-header + .modal-body {
+  margin-top: 0;
+}
+
+/* Modal mask - semi-transparent backdrop */
+.modal-mask {
+  position: fixed;
+  top: 0;
+  left: 0;
+  width: 100%;
+  height: 100%;
+  display: flex;
+  background-color: rgba(0, 0, 0, 0.3);
+}
+
+/* Modal footer */
+.modal-footer {
+  display: flex;
+  justify-content: space-between;
+  padding: 3rem; /* modalPadding */
+}
+
+/* Close modal icon button */
+.modal-close-icon {
+  /* Final legacy CloseModalIcon size: 2rem x 2rem */
+  height: 2rem;
+  width: 2rem;
+  padding: 0.4rem;
+
+  /* Base button styles */
+  cursor: pointer;
+  margin-right: 0;
+  padding-right: 0;
+  background: none;
+  border: none;
+  display: flex;
+  align-items: center;
+  justify-content: center;
+
+  /* toolbarIconColor.lighter */
+  color: var(--icon-color-lighter, #c5c5c5);
+}
+
+.modal-close-icon:hover {
+  /* toolbarIconColor.base */
+  color: var(--icon-color-base, #5e6062);
+}
+
+.modal-close-icon > svg {
+  height: 100%;
+  width: 100%;
+}

src/app/components/Modal/Modal.spec.tsx

@@ -3,6 +3,29 @@ import renderer from 'react-test-renderer';
 import Modal from '.';
 import TestContainer from '../../../test/TestContainer';
 
+/**
+ * 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.
+ *
+ * Explanation:
+ * - The legacy styled-components wrappers (styles.legacy.tsx) wrap plain components using styled()
+ * - These wrappers generate styled-components class names like "styleslegacy__Body-m93cxn-6 dmUpwe"
+ * - During snapshot serialization, Jest strips out these generated styled-components classes
+ * - This leaves the original "modal-body" class with a trailing space where the styled class was
+ *
+ * Example:
+ * - Runtime className: "modal-body styleslegacy__Body-m93cxn-6 dmUpwe"
+ * - After Jest stripping: "modal-body "
+ *
+ * This does NOT occur in actual runtime - browsers receive the full className with both the base
+ * class and styled-components classes. The trailing space only appears in test snapshots.
+ *
+ * This artifact will resolve itself once the legacy styled-components wrappers are removed in
+ * future migration phases.
+ */
+
 describe('Modal', () => {
   it('matches snapshot', () => {
     const component = renderer.create(<TestContainer>