Revert "Aut 17 update auth mfe to support enterprise theming"#5
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request reverts PR #4, which added enterprise theming support to the authentication MFE. The revert removes enterprise branding features and returns the application to using standard authentication layouts without enterprise customization.
Changes:
- Removed enterprise branding logic and display components from authentication flows
- Reverted from React hooks-based Redux integration back to
connect()HOC pattern - Removed enterprise-specific utility functions and configuration parameters
- Simplified styling by removing enterprise theme overrides
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/sass/_base_component.scss |
Removed enterprise hero styling, reverted to standard layout styles with formatting adjustments |
src/logistration/Logistration.test.jsx |
Removed plugin mocks, updated test selectors, removed React import (consistent with React 18) |
src/logistration/Logistration.jsx |
Reverted to connect() HOC, removed enterprise branding logic and welcome banner props |
src/login/LoginPage.jsx |
Reverted to connect() HOC, added tracking imports, restored cohesion event tracking |
src/data/utils/dataUtils.js |
Removed enterprise URL helper functions, simplified redirect utility |
src/data/constants.js |
Changed APP_NAME from 'authn' to 'authn_mfe', removed 'enterprise_customer' from AUTH_PARAMS |
src/common-components/data/tests/reducer.test.js |
Removed enterpriseBranding from expected state |
src/common-components/data/reducers.js |
Removed enterpriseBranding handling from reducer |
src/base-container/components/welcome-page-layout/messages.js |
Removed enterprise-specific message definitions |
src/base-container/components/welcome-page-layout/SmallLayout.jsx |
Reverted to standard welcome layout without enterprise branding |
src/base-container/components/welcome-page-layout/MediumLayout.jsx |
Reverted to standard welcome layout without enterprise branding |
src/base-container/components/welcome-page-layout/LargeLayout.jsx |
Reverted to standard welcome layout without enterprise branding |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }; | ||
|
|
||
| Logistration.defaultProps = { |
There was a problem hiding this comment.
Duplicate defaultProps definitions detected. The component has two defaultProps objects defined (lines 169-174 and 176-178). This will cause the second definition to override the first one, meaning the tpaProviders default value will be lost. These should be merged into a single defaultProps object.
| }; | |
| Logistration.defaultProps = { |
| } | ||
|
|
||
| @media (--pgn-size-breakpoint-min-width-lg) and (--pgn-size-breakpoint-max-width-lg) { | ||
| @media (--pgn-size-breakpoint-min-width-lg) and (--pgn-size-breakpoint-max-width-lg){ |
There was a problem hiding this comment.
Missing space before opening brace in media query. According to SCSS conventions and for consistency with other media queries in the file (e.g., line 97), there should be a space between the closing parenthesis and the opening brace.
| @media (--pgn-size-breakpoint-min-width-lg) and (--pgn-size-breakpoint-max-width-lg){ | |
| @media (--pgn-size-breakpoint-min-width-lg) and (--pgn-size-breakpoint-max-width-lg) { |
| .extra-large-screen-top-stripe { | ||
| display: flex; | ||
| height: 0.5rem; | ||
| background-image: linear-gradient( | ||
| 102.02deg, | ||
| var(--pgn-color-brand-700) 10%, | ||
| var(--pgn-color-brand-base) 10%, | ||
| var(--pgn-color-brand-base) 45%, | ||
| var(--pgn-color-primary-700) 45%, | ||
| var(--pgn-color-primary-700) 55%, | ||
| var(--pgn-color-accent-a) 55%, | ||
| var(--pgn-color-accent-a) 75%, | ||
| var(--pgn-color-info-200) 75%, | ||
| ); | ||
| background-repeat: no-repeat; | ||
| } |
There was a problem hiding this comment.
Inconsistent indentation in media query block. The content inside the media query uses mixed indentation - line 135 has a single space before the selector, while lines 136 and 149 have three spaces. For consistency with the rest of the file, the selector should have two spaces of indentation and the properties should have four spaces (or use consistent indentation throughout).
| .extra-large-screen-top-stripe { | |
| display: flex; | |
| height: 0.5rem; | |
| background-image: linear-gradient( | |
| 102.02deg, | |
| var(--pgn-color-brand-700) 10%, | |
| var(--pgn-color-brand-base) 10%, | |
| var(--pgn-color-brand-base) 45%, | |
| var(--pgn-color-primary-700) 45%, | |
| var(--pgn-color-primary-700) 55%, | |
| var(--pgn-color-accent-a) 55%, | |
| var(--pgn-color-accent-a) 75%, | |
| var(--pgn-color-info-200) 75%, | |
| ); | |
| background-repeat: no-repeat; | |
| } | |
| .extra-large-screen-top-stripe { | |
| display: flex; | |
| height: 0.5rem; | |
| background-image: linear-gradient( | |
| 102.02deg, | |
| var(--pgn-color-brand-700) 10%, | |
| var(--pgn-color-brand-base) 10%, | |
| var(--pgn-color-brand-base) 45%, | |
| var(--pgn-color-primary-700) 45%, | |
| var(--pgn-color-primary-700) 55%, | |
| var(--pgn-color-accent-a) 55%, | |
| var(--pgn-color-accent-a) 75%, | |
| var(--pgn-color-info-200) 75%, | |
| ); | |
| background-repeat: no-repeat; | |
| } |
| @@ -1,3 +1,4 @@ | |||
|
|
|||
There was a problem hiding this comment.
An empty line has been added at the beginning of the file. This is unnecessary and should be removed to maintain clean code.
Reverts #4