Closed
Conversation
TerranceKhumalo-absa
requested review from
fateeand,
korel-san and
lukasmatta
as code owners
TerranceKhumalo-absa
force-pushed
the
fix/512-colors-page-crashes
branch
from
e967ade to
363bf10
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds first-class theming (light/dark + semantic tokens) to the CPS UI Kit and Composition app, and fixes the production-only Colors page crash caused by CSS-minified rgb(r g b / a) color syntax.
Changes:
- Introduces dark theme tokens (
_colors-dark.scss) and broad adoption of semantic CSS variables across components and Composition styles. - Adds
CpsThemeService(signals-based) + a CompositionThemeToggleComponentto switch theme options at runtime. - Fixes
colors-utils.tsparsing to support both legacy and modernrgb/rgbaformats and updates API docs/type mappings.
Reviewed changes
Copilot reviewed 39 out of 41 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/cps-ui-kit/styles/styles.scss | Imports dark theme tokens and adds global theme-dependent base styles + transition hook. |
| projects/cps-ui-kit/styles/_colors.scss | Adds semantic design tokens and theme/radius overrides via CSS variables. |
| projects/cps-ui-kit/styles/_colors-dark.scss | New dark-theme palette and semantic token overrides keyed off data-theme='dark'. |
| projects/cps-ui-kit/src/public-api.ts | Reorders exports and exposes the new CpsThemeService. |
| projects/cps-ui-kit/src/lib/utils/colors-utils.ts | Fixes rgb/rgba parsing to prevent production crash on minified CSS formats. |
| projects/cps-ui-kit/src/lib/services/cps-theme/cps-theme.service.ts | New root service managing theme attributes, persistence, and system preference watching. |
| projects/cps-ui-kit/src/lib/services/cps-theme/cps-theme.service.spec.ts | Adds unit tests for the new theme service. |
| projects/cps-ui-kit/src/lib/components/cps-progress-linear/cps-progress-linear.component.ts | Updates default progress color to a semantic theme variable. |
| projects/cps-ui-kit/src/lib/components/cps-loader/cps-loader.component.ts | Updates loader defaults toward theme tokens. |
| projects/cps-ui-kit/src/lib/components/cps-loader/cps-loader.component.scss | Updates loader circle colors to semantic accent tokens. |
| projects/cps-ui-kit/src/lib/components/cps-input/cps-input.component.ts | Import reordering (no functional change). |
| projects/cps-ui-kit/src/lib/components/cps-input/cps-input.component.scss | Replaces hard-coded/light-theme styles with semantic theme variables and focus ring styling. |
| projects/cps-ui-kit/src/lib/components/cps-input/cps-input.component.html | Adds error label styling, updates disabled icon color, and tweaks progress opacity. |
| projects/cps-ui-kit/src/lib/components/cps-icon/cps-icon.component.ts | Adds new icons (sun, moon, kafka) to the icon name whitelist. |
| projects/cps-ui-kit/src/lib/components/cps-chip/cps-chip.component.scss | Updates chip styling to semantic surfaces/borders/text. |
| projects/cps-ui-kit/src/lib/components/cps-chip/cps-chip.component.html | Updates close icon default color token. |
| projects/cps-ui-kit/src/lib/components/cps-autocomplete/cps-autocomplete.component.scss | Migrates autocomplete styling to semantic tokens, adds focus ring, and updates popover styling. |
| projects/cps-ui-kit/src/lib/components/cps-autocomplete/cps-autocomplete.component.html | Improves keyboard/accessibility (tabindex + aria-label) and updates disabled icon color. |
| projects/cps-ui-kit/assets/icons.svg | Adds sun, moon, and kafka SVG symbols. |
| projects/composition/src/variables.scss | Switches Composition variables to semantic theme tokens. |
| projects/composition/src/styles.scss | Updates table/code block styling to semantic surfaces and adds row hover highlight. |
| projects/composition/src/app/pages/icons-page/icons-page/icons-page.component.scss | Updates icon list text color to semantic token. |
| projects/composition/src/app/pages/icons-page/icons-page/icons-page.component.html | Updates icon color usage to semantic theme variable. |
| projects/composition/src/app/components/theme-toggle/theme-toggle.component.ts | New UI for selecting theme/radius/base and toggling dark mode via CpsThemeService. |
| projects/composition/src/app/components/theme-toggle/theme-toggle.component.scss | Styling for the new theme toggle controls and popover menu. |
| projects/composition/src/app/components/theme-toggle/theme-toggle.component.html | Adds appearance popover + dark mode toggle button with new icons. |
| projects/composition/src/app/components/navigation-sidebar/navigation-sidebar.component.scss | Applies semantic tokens to sidebar surfaces/text/hover states. |
| projects/composition/src/app/components/component-docs-viewer/component-docs-viewer.component.html | Updates tab background token to a semantic surface. |
| projects/composition/src/app/app.module.ts | Registers ThemeToggleComponent in the app module. |
| projects/composition/src/app/app.component.spec.ts | Updates AppComponent test imports to include ThemeToggleComponent. |
| projects/composition/src/app/app.component.scss | Updates toolbar and layout styling to semantic surfaces/borders/text. |
| projects/composition/src/app/app.component.html | Adds logo alt text and inserts the new theme toggle in the toolbar. |
| projects/composition/src/app/api-data/types_map.json | Maps new theme-related types to a dedicated “theme” docs section. |
| projects/composition/src/app/api-data/cron-validation.service.json | Normalizes doc line endings/formatting. |
| projects/composition/src/app/api-data/cps-tree-table.json | Normalizes doc line endings/formatting. |
| projects/composition/src/app/api-data/cps-theme.json | Adds new API documentation entry for CpsThemeService and theme types. |
| projects/composition/src/app/api-data/cps-table.json | Normalizes doc line endings/formatting. |
| projects/composition/src/app/api-data/cps-progress-linear.json | Updates documented default color to semantic variable. |
| projects/composition/src/app/api-data/cps-notification.json | Normalizes doc line endings/formatting. |
| projects/composition/src/app/api-data/cps-loader.json | Updates documented default label color. |
| package-lock.json | Updates lockfile metadata (adds/removes peer flags across entries). |
projects/cps-ui-kit/src/lib/components/cps-loader/cps-loader.component.ts
Outdated
Show resolved
Hide resolved
projects/cps-ui-kit/src/lib/components/cps-chip/cps-chip.component.html
Outdated
Show resolved
Hide resolved
projects/cps-ui-kit/src/lib/services/cps-theme/cps-theme.service.spec.ts
Show resolved
Hide resolved
projects/cps-ui-kit/src/lib/components/cps-input/cps-input.component.html
Outdated
Show resolved
Hide resolved
projects/cps-ui-kit/src/lib/components/cps-loader/cps-loader.component.ts
Show resolved
Hide resolved
projects/composition/src/app/components/theme-toggle/theme-toggle.component.ts
Outdated
Show resolved
Hide resolved
Coverage report for library
Test suite run success429 tests passing in 22 suites. Report generated by 🧪jest coverage report action from 9657371 |
…stency across components
…stency and accessibility
… navigation sidebar styles for better theming consistency
…ormance and clarity
…default values, and improve descriptions for clarity
- Added a new "kafka" icon to the icons.svg file. - Updated the iconNames array in cps-icon.component.ts to include "kafka". - Improved regex matching for RGB color parsing in colors-utils.ts to support modern syntax.
TerranceKhumalo-absa
force-pushed
the
fix/512-colors-page-crashes
branch
from
9be6232 to
9657371
Compare
Playwright test results
Details
|
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.
Dark Mode Support
This pull request introduces dark mode theming support for the CPS Shared UI library, including a theme toggle component, CSS variable-based styling, and updated API documentation.
Theming & UI Enhancements
ThemeToggleComponentto the top toolbar, allowing users to switch between light and dark modes.app.component.scssandnavigation-sidebar.component.scssto use CSS variables (e.g.,--cps-surface-body,--cps-text-primary) for backgrounds, borders, and text, ensuring theme consistency across both modes.component-docs-viewer.component.htmlto use the new theme variable.alttext to the logo image for accessibility.Theming API Documentation
cps-theme.jsonAPI doc describingCpsThemeServiceand new types:CpsTheme,CpsColorTheme,CpsBaseTheme, andCpsRadiusTheme.types_map.jsonto map the new theme types to the "theme" section.Component Updates
cps-loaderandcps-progress-linearto use theme-aware CSS variables.cps-notification.json,cps-table.json,cps-tree-table.json,cron-validation.service.json).Bug Fix
The original dark mode release (v20.5.0) introduced a production-only crash on the Colors page:
Root cause:
The
isDark()function used a regex that only matched the legacy comma-separated CSS color format (rgba(73, 185, 255, 0.16)). In production builds, the CSS minifier converts these to the modern space-separated syntax (rgb(73 185 255 / .16)), causing the regex to returnnull. This was not an issue before dark mode because the light theme's--cps-color-*variables are all hex values. The dark theme (_colors-dark.scss) introducedrgba()values forinfo-bg,success-bg,warn-bg,error-bg, and highlight colors.Fix: Updated the regex in
isDark()from:/^rgba?\((\d+),\s*(\d+),\s*(\d+)(?:,\s*(\d+(?:\.\d+)?))?\)$/to:
/^rgba?\(\s*(\d+)[,\s]+(\d+)[,\s]+(\d+)/This matches both legacy and modern CSS color syntax. A null guard was also added to prevent crashes on any other unrecognized format.
Affected file:
projects/cps-ui-kit/src/lib/utils/colors-utils.tsRelease notes: