Phase 2.5: Notifications and Error Components Migration#2839
Merged
Conversation
OpenStaxClaude
added a commit
that referenced
this pull request
Apr 1, 2026
- Fix htmlMessage function calls: Remove third argument (className) from ErrorBoundary and ErrorModal, and pass className as a prop instead - Fix Times component props: Change focusable attribute from string 'false' to boolean false in Toast component Resolves review comments from PR #2839 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
OpenStaxClaude
added a commit
that referenced
this pull request
Apr 1, 2026
…mlMessage - ErrorBoundary.tsx: Create BodyErrorText wrapper component to replace styled component, pass it to htmlMessage instead of string 'div' - ErrorModal.tsx: Create BodyErrorText wrapper component to replace styled component, pass it to htmlMessage instead of string 'div' - Toast.tsx: Remove invalid focusable prop from Times SVG component (not part of React's HTMLAttributes<SVGElement> type) These changes maintain the same CSS styling (.body-error-text class) while properly typing the components for TypeScript. Addresses review feedback on PR #2839 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
OpenStaxClaude
added a commit
that referenced
this pull request
Apr 1, 2026
- Scope .body-error-text in ErrorBoundary.css under .error-wrapper parent - Scope .body-error-text in ErrorModal.css under .error-modal parent - Update link colors in ErrorBoundary.css to match design system constants: - Link color: #0074BC → #027EB5 (linkColor constant) - Hover color: #005A8E → #0064A0 (linkHover constant) This prevents CSS conflicts between ErrorBoundary and ErrorModal components and ensures consistent link styling across the application. Addresses Copilot review comments 1, 2, and 3 from PR #2839 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
RoyEJohnson
pushed a commit
that referenced
this pull request
Apr 1, 2026
This commit migrates ToastNotifications and error display components from styled-components to CSS Modules as part of Phase 2.5. Components migrated: - ToastNotifications (with slide-in/fade-out animations) - ErrorBoundary - ErrorModal - ErrorIdList - LoaderCentered Key changes: - Created .module.css files for all components - Preserved all animations (slide-in and fade-out for toasts) - Maintained responsive styling and variant support - Removed styled-components imports and usage - Used data attributes for animation states - Inline styles for dynamic z-index calculation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Fix CSS class name collisions and update link colors - Scope .body-error-text in ErrorBoundary.css under .error-wrapper parent - Scope .body-error-text in ErrorModal.css under .error-modal parent - Update link colors in ErrorBoundary.css to match design system constants: - Link color: #0074BC → #027EB5 (linkColor constant) - Hover color: #005A8E → #0064A0 (linkHover constant) This prevents CSS conflicts between ErrorBoundary and ErrorModal components and ensures consistent link styling across the application. Addresses Copilot review comments 1, 2, and 3 from PR #2839 🤖 Generated with [Claude Code](https://claude.com/claude-code) Fix TypeScript errors: Use React components instead of strings for htmlMessage - ErrorBoundary.tsx: Create BodyErrorText wrapper component to replace styled component, pass it to htmlMessage instead of string 'div' - ErrorModal.tsx: Create BodyErrorText wrapper component to replace styled component, pass it to htmlMessage instead of string 'div' - Toast.tsx: Remove invalid focusable prop from Times SVG component (not part of React's HTMLAttributes<SVGElement> type) These changes maintain the same CSS styling (.body-error-text class) while properly typing the components for TypeScript. Addresses review feedback on PR #2839 🤖 Generated with [Claude Code](https://claude.com/claude-code) Fix TypeScript errors in error and notification components - Fix htmlMessage function calls: Remove third argument (className) from ErrorBoundary and ErrorModal, and pass className as a prop instead - Fix Times component props: Change focusable attribute from string 'false' to boolean false in Toast component Resolves review comments from PR #2839 🤖 Generated with [Claude Code](https://claude.com/claude-code) Convert CSS Modules to plain CSS per team design decision Updated Phase 2.5 implementation to use plain CSS instead of CSS Modules, following the established migration pattern documented in PLAIN_CSS_MIGRATION_LEARNINGS.md. Components migrated: - ToastNotifications (with slide-in/fade-out animations) - ErrorBoundary - ErrorModal - ErrorIdList - LoaderCentered Key changes: - Renamed *.module.css to *.css (e.g., styles.module.css → ToastNotifications.css) - Updated imports to use plain CSS imports instead of CSS Module imports - Changed className references from styles.className to plain "class-name" strings - Used classnames package for conditional class composition - Converted camelCase/PascalCase CSS class names to kebab-case (e.g., bannerBodyWrapper → banner-body-wrapper) - Ensured all media queries are top-level in CSS files (not nested) - Preserved all animations (slide-in and fade-out for toasts) - Maintained responsive styling and variant support - Removed styled-components imports and usage This approach aligns with: - PLAIN_CSS_MIGRATION_LEARNINGS.md best practices - Existing codebase patterns from previous migration phases - Team's design decision to use plain CSS over CSS Modules 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
4fcf3c7 to
7adc25d
Compare
a719f91 to
d13ca00
Compare
d13ca00 to
f2bbb48
Compare
This comment was marked as resolved.
This comment was marked as resolved.
f5ea51b to
f96e8c9
Compare
This comment was marked as resolved.
This comment was marked as resolved.
85b6e68 to
a1086d2
Compare
OpenStaxClaude
added a commit
that referenced
this pull request
Apr 2, 2026
- Add test case to verify that props starting with $ (transient props)
are filtered out and not forwarded to the DOM
- This exercises the else-branch of the if(!key.startsWith('$')) condition
- Transient props are a styled-components convention for style-only props
that should not appear as HTML attributes
Addresses review comment from PR #2839 (Review #20)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
RoyEJohnson
pushed a commit
that referenced
this pull request
Apr 2, 2026
Non-standard props like isActive and practiceQuestionsEnabled are used by styled-components for conditional styling but should not be forwarded to the DOM as HTML attributes. This change filters them out before passing props to the native button element. Addresses Copilot review comment about isActive prop forwarding. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Update Button.tsx Add test for PlainButton transient props filtering - Add test case to verify that props starting with $ (transient props) are filtered out and not forwarded to the DOM - This exercises the else-branch of the if(!key.startsWith('$')) condition - Transient props are a styled-components convention for style-only props that should not appear as HTML attributes Addresses review comment from PR #2839 (Review #20) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-Authored-By: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
72b4efe to
9a84d7b
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 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.
This commit migrates ToastNotifications and error display components from styled-components to CSS Modules as part of Phase 2.5. Components migrated: - ToastNotifications (with slide-in/fade-out animations) - ErrorBoundary - ErrorModal - ErrorIdList - LoaderCentered Key changes: - Created .module.css files for all components - Preserved all animations (slide-in and fade-out for toasts) - Maintained responsive styling and variant support - Removed styled-components imports and usage - Used data attributes for animation states - Inline styles for dynamic z-index calculation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Fix CSS class name collisions and update link colors - Scope .body-error-text in ErrorBoundary.css under .error-wrapper parent - Scope .body-error-text in ErrorModal.css under .error-modal parent - Update link colors in ErrorBoundary.css to match design system constants: - Link color: #0074BC → #027EB5 (linkColor constant) - Hover color: #005A8E → #0064A0 (linkHover constant) This prevents CSS conflicts between ErrorBoundary and ErrorModal components and ensures consistent link styling across the application. Addresses Copilot review comments 1, 2, and 3 from PR #2839 🤖 Generated with [Claude Code](https://claude.com/claude-code) Fix TypeScript errors: Use React components instead of strings for htmlMessage - ErrorBoundary.tsx: Create BodyErrorText wrapper component to replace styled component, pass it to htmlMessage instead of string 'div' - ErrorModal.tsx: Create BodyErrorText wrapper component to replace styled component, pass it to htmlMessage instead of string 'div' - Toast.tsx: Remove invalid focusable prop from Times SVG component (not part of React's HTMLAttributes<SVGElement> type) These changes maintain the same CSS styling (.body-error-text class) while properly typing the components for TypeScript. Addresses review feedback on PR #2839 🤖 Generated with [Claude Code](https://claude.com/claude-code) Fix TypeScript errors in error and notification components - Fix htmlMessage function calls: Remove third argument (className) from ErrorBoundary and ErrorModal, and pass className as a prop instead - Fix Times component props: Change focusable attribute from string 'false' to boolean false in Toast component Resolves review comments from PR #2839 🤖 Generated with [Claude Code](https://claude.com/claude-code) Convert CSS Modules to plain CSS per team design decision Updated Phase 2.5 implementation to use plain CSS instead of CSS Modules, following the established migration pattern documented in PLAIN_CSS_MIGRATION_LEARNINGS.md. Components migrated: - ToastNotifications (with slide-in/fade-out animations) - ErrorBoundary - ErrorModal - ErrorIdList - LoaderCentered Key changes: - Renamed *.module.css to *.css (e.g., styles.module.css → ToastNotifications.css) - Updated imports to use plain CSS imports instead of CSS Module imports - Changed className references from styles.className to plain "class-name" strings - Used classnames package for conditional class composition - Converted camelCase/PascalCase CSS class names to kebab-case (e.g., bannerBodyWrapper → banner-body-wrapper) - Ensured all media queries are top-level in CSS files (not nested) - Preserved all animations (slide-in and fade-out for toasts) - Maintained responsive styling and variant support - Removed styled-components imports and usage This approach aligns with: - PLAIN_CSS_MIGRATION_LEARNINGS.md best practices - Existing codebase patterns from previous migration phases - Team's design decision to use plain CSS over CSS Modules 🤖 Generated with [Claude Code](https://claude.com/claude-code) Address Copilot review comments: Fix padding and simplify PlainButton 1. Fix ErrorModal.css padding calculation - Changed from 0.8rem to 1.5rem to match original styled-component - Original calculation: modalPadding (3.0) * 0.5 = 1.5rem 2. Simplify PlainButton component in Button.tsx - Removed unnecessary props reduce/copy logic that wasn't filtering anything - Test coverage confirmed no transient props are passed to PlainButton - Updated comment to clarify PlainButton accepts standard HTML button attributes - TypeScript types already enforce only valid button attributes are passed Resolves review comments from Copilot and Roy on PR #2839 🤖 Generated with [Claude Code](https://claude.com/claude-code) Update Button.tsx Fix test failures after styled-components migration - Update Toast.spec.tsx to check data-fading-out attribute instead of isFadingOut prop, matching the new plain CSS implementation - Update index.spec.tsx to query elements by data-testid and aria-label instead of styled-component types (BannerBodyWrapper, CloseButton) - Remove import of deleted styles.tsx file from index.spec.tsx - Delete unused styles.tsx file as specified in migration plan All tests now query the actual DOM elements using their attributes, which aligns with the plain CSS migration approach. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Update types.ts Fix test issues Move CSS imports to the global file Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-Authored-By: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- ErrorModal: Convert to function component using useSelector and useDispatch hooks - ErrorBoundary: Use hooks wrapper pattern to maintain Error Boundary class component while connecting to Redux with useSelector and useDispatch The ErrorBoundary requires a class component to use componentDidCatch lifecycle method (Error Boundaries can't be function components), so we use a wrapper pattern where: - ErrorBoundaryClass: Inner class component handling error catching - ErrorBoundary: Outer function component using hooks to connect to Redux This modernizes the Redux integration while maintaining all functionality. Addresses review comment from PR #2839 🤖 Generated with [Claude Code](https://claude.com/claude-code) Replace React.FC with ordinary function declarations Replaced all React.FC type annotations with standard function declarations in error components as requested in code review: - ErrorBoundary.tsx: Converted BodyErrorText, ErrorDisplay, and ErrorBoundary wrapper functions from React.FC to ordinary functions - ErrorModal.tsx: Converted BodyErrorText from React.FC to ordinary function All functions maintain their TypeScript type annotations through explicit parameter and return types. Addresses review comment in PR #2839 🤖 Generated with [Claude Code](https://claude.com/claude-code) Fix ErrorBoundary recordError prop type Change recordError prop type from 'typeof recordError' to '(error: Error) => void' to match the actual function signature passed from the wrapper component. The wrapper component creates a handleRecordError function that dispatches the action and returns void, not the action itself. Resolves TypeScript error TS2769 in ErrorBoundary.tsx:104 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-Authored-By: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Implements the ability to trigger the ErrorModal by adding ?modal=ERROR to the URL, similar to how modal=MH works for My Highlights. Changes: - Created src/app/errors/constants.ts with modalUrlName = 'ERROR' - Updated errors reducer to check for modal query parameter in locationChange - Added tests for the new modal=ERROR functionality - Updated existing tests to include query parameter This allows developers and QA to easily test the ErrorModal display by navigating to any page with ?modal=ERROR in the query string. Addresses review comment from PR #2839 🤖 Generated with [Claude Code](https://claude.com/claude-code) Fix TypeScript errors in reducer.spec.ts tests Remove query property from locationChange action calls in tests. The locationChange action creator automatically parses the query from location.search, so passing it as a separate property causes a TypeScript error since it's not part of the LocationChange type. The query is correctly parsed from the search string in the action creator's .map() function. Resolves Review #16 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Non-standard props like isActive and practiceQuestionsEnabled are used by styled-components for conditional styling but should not be forwarded to the DOM as HTML attributes. This change filters them out before passing props to the native button element. Addresses Copilot review comment about isActive prop forwarding. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Update Button.tsx Add test for PlainButton transient props filtering - Add test case to verify that props starting with $ (transient props) are filtered out and not forwarded to the DOM - This exercises the else-branch of the if(!key.startsWith('$')) condition - Transient props are a styled-components convention for style-only props that should not appear as HTML attributes Addresses review comment from PR #2839 (Review #20) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-Authored-By: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Dantemss
approved these changes
Apr 17, 2026
Member
Dantemss
left a comment
There was a problem hiding this comment.
Well... nothing jumps at me.
RoyEJohnson
approved these changes
Apr 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates notification and error display components from styled-components to plain CSS as part of Phase 2.5 of the REX modernization effort.
Related Jira Ticket: CORE-1699
Note: This PR was updated to use plain CSS instead of CSS Modules, following the team's design decision documented in
PLAIN_CSS_MIGRATION_LEARNINGS.md.Components Migrated
ToastNotifications (
src/app/notifications/components/ToastNotifications/)ErrorBoundary (
src/app/errors/components/ErrorBoundary.tsx)ErrorModal (
src/app/errors/components/ErrorModal.tsx)ErrorIdList (
src/app/errors/components/ErrorIdList.tsx)LoaderCentered (
src/app/errors/components/LoaderCentered.tsx)Technical Implementation
Plain CSS Approach (following PLAIN_CSS_MIGRATION_LEARNINGS.md)
.cssfiles for each component (e.g.,ToastNotifications.css,ErrorBoundary.css).banner-body-wrapper,.toast-close-button)classnamespackage for conditional class compositiondata-fading-in,data-fading-out) for animation state managementstyled-components/macroimportsstyles.tsxfile from ToastNotifications directoryCSS Class Naming Convention
ToastsContainer→.toasts-containerBannerBodyWrapper→.banner-body-wrapperBannerBody→.banner-bodywith.banner-body-errorand.banner-body-warningvariantsCloseButton→.toast-close-buttonCloseIcon→.toast-close-iconErrorId→.toast-error-idTesting Checklist
yarn test:unit- all unit tests passNotes
PLAIN_CSS_MIGRATION_LEARNINGS.md🤖 Generated with Claude Code