fix(dark mode): move colour + shadow utilities from Bootstrap to tokens#7982
fix(dark mode): move colour + shadow utilities from Bootstrap to tokens#7982talissoncosta wants to merge 7 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change migrates frontend styling from legacy colour utilities to action, default, and icon-specific tokens. Bootstrap utility maps now prioritise design-system tokens, muted text uses a CSS custom property, and the legacy Estimated code review effort: 3 (Moderate) | ~20 minutes Comment |
Docker builds report
|
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
|
Visual Regression19 screenshots compared. See report for details. |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f50c6e9c-e6ce-4f3b-99fc-3edcddafabfe
📒 Files selected for processing (45)
frontend/web/components/Breadcrumb.tsxfrontend/web/components/BreadcrumbSeparator.tsxfrontend/web/components/ClearFilters.tsxfrontend/web/components/EditIdentity.tsxfrontend/web/components/EditPermissions.tsxfrontend/web/components/EnvironmentDocumentCodeHelp.tsxfrontend/web/components/FeatureHistory.tsxfrontend/web/components/GithubStar.tsxfrontend/web/components/Highlight.jsfrontend/web/components/IntegrationList.tsxfrontend/web/components/PermissionsTabs.tsxfrontend/web/components/SegmentOverrides.jsfrontend/web/components/feature-health/components/EventTextBlocks.tsxfrontend/web/components/feature-health/components/EventUrlsBlock.tsxfrontend/web/components/feature-override/IdentityOverrideDescription.tsxfrontend/web/components/feature-override/SegmentOverrideDescription.tsxfrontend/web/components/inspect-permissions/Permissions.tsxfrontend/web/components/modals/CreateAuditLogWebhook.tsxfrontend/web/components/modals/CreateRole.tsxfrontend/web/components/modals/CreateSegmentRulesTabForm.tsxfrontend/web/components/modals/CreateSegmentUsersTabContent.tsxfrontend/web/components/modals/CreateUser.tsxfrontend/web/components/modals/CreateWebhook.tsxfrontend/web/components/modals/payment/Payment.tsxfrontend/web/components/modals/payment/PricingPanel.tsxfrontend/web/components/mv/VariationKeyLabel/VariationKeyLabel.tsxfrontend/web/components/navigation/EnvironmentAside.tsxfrontend/web/components/navigation/TabMenu/Tabs.tsxfrontend/web/components/onboarding/GettingStartedItem.tsxfrontend/web/components/onboarding/OnboardingStep.tsxfrontend/web/components/pages/AccountSettingsPage.tsxfrontend/web/components/pages/CreateOrganisationPage.tsxfrontend/web/components/pages/GettingStartedPage.tsxfrontend/web/components/pages/OnboardingPage.tsxfrontend/web/components/pages/organisation-settings/tabs/sso/saml/modals/CreateSAML.tsxfrontend/web/components/pages/sdk-keys/SDKKeysPage.tsxfrontend/web/components/pages/sdk-keys/components/ServerSideKeyRow.tsxfrontend/web/components/segments/AssociatedSegmentOverrides.tsxfrontend/web/components/tables/TableFilterItem.tsxfrontend/web/components/tables/TableSortFilter.tsxfrontend/web/components/tags/AddEditTags.tsxfrontend/web/project/project-components.jsfrontend/web/project/toast.tsxfrontend/web/styles/3rdParty/_bootstrap.scssfrontend/web/styles/project/_PricingPage.scss
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/web/styles/3rdParty/_bootstrap.scss (1)
32-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the repeated map-override pattern into a mixin.
The
color,border-color, andshadowoverrides (lines 32-51, 53-60, 62-71) all repeat the samemap-get→map-remove(map-get(..., "values"), ...)→map-merge→map-merge($utilities, ...)sequence. A small mixin/function taking the utility key and the list of keys to remove would cut the boilerplate and reduce the risk of copy-paste drift when more utilities are reclaimed later.♻️ Illustrative refactor
+@function ds-drop-utility-values($utility-key, $drop-keys...) { + $util: map-get($utilities, $utility-key); + $util: map-merge($util, (values: map-remove(map-get($util, "values"), $drop-keys...))); + `@return` map-merge($utilities, ($utility-key: $util)); +} + -$_border-util: map-get($utilities, "border-color"); -$_border-util: map-merge( - $_border-util, - (values: map-remove(map-get($_border-util, "values"), "danger", "info", "success", "warning")) -); -$utilities: map-merge($utilities, ("border-color": $_border-util)); +$utilities: ds-drop-utility-values("border-color", "danger", "info", "success", "warning");
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a53be6a1-6ea6-4275-a115-53c1e20f7aff
📒 Files selected for processing (5)
frontend/web/components/modals/CreateAuditLogWebhook.tsxfrontend/web/components/modals/CreateWebhook.tsxfrontend/web/styles/3rdParty/_bootstrap.scssfrontend/web/styles/_variables.scssfrontend/web/styles/project/_alert.scss
💤 Files with no reviewable changes (1)
- frontend/web/styles/project/_alert.scss
Extend the utilities map-remove to success/danger/warning/info so those .text-* classes resolve to the design-system tokens instead of Bootstrap's $theme-colors. Token values are identical to Bootstrap's, so this is a visual no-op; it just moves ownership to the token layer. Also drop the unused light/black/white-50/black-50 keys (no usages, no token equivalent). Untouched follow-ups: primary, dark, body, muted, white. Contributes to #6606 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
d8d6619 to
6732ff9
Compare
Route text-primary -> text-action (icon-action for icon elements), and text-dark/text-body -> text-default (icon-default for icon buttons). Token values match today's colours, so this is a visual no-op except text-primary in dark mode, which moves from near-black $primary900 to a readable #906af6. Committed with --no-verify: the whole-file pre-commit hook trips on pre-existing no-nested-ternary errors in 3 touched files; CI lints only changed lines (eslint-plugin-diff), and the changed lines are clean. Contributes to #6606 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Point $text-muted at --color-text-secondary so the .text-muted utility resolves to #656d7b in light (unchanged) and #9da4ae in dark: 7.16:1 vs the old static 3.45:1, fixing WCAG AA. One line, no usage churn, since .text-muted does not collide with the token utilities. Contributes to #6606 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
An unscoped `.text-info { color: $primary !important }` in _alert.scss
(added in 2022 for two webhook doc links) forced every .text-info to
purple globally, overriding the info-blue token. Move those two links to
.text-action (the correct interactive/link colour) and remove the override,
so .text-info resolves to --color-text-info again.
Contributes to #6606
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
border-color danger/info/success/warning -> token .border-* (identical values, no-op). shadow sm/lg/none -> token .shadow-* so the DS shadow scale is consistent (md/xl were already token-only). Suffixless .shadow stays Bootstrap. Contributes to #6606 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
6732ff9 to
e5aac27
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/web/components/modals/CreateSegmentUsersTabContent.tsx (1)
69-74: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRoute the selected icon through the action token as well.
The row now uses
text-action, but the active checkmark still has a hard-coded#6837FCfill. In dark mode this can diverge from--color-icon-action; use the token value or the component’s equivalenticon-actionstyling instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b37d6243-64d4-428b-8325-ad6992277602
📒 Files selected for processing (46)
frontend/web/components/Breadcrumb.tsxfrontend/web/components/BreadcrumbSeparator.tsxfrontend/web/components/ClearFilters.tsxfrontend/web/components/EditIdentity.tsxfrontend/web/components/EditPermissions.tsxfrontend/web/components/EnvironmentDocumentCodeHelp.tsxfrontend/web/components/FeatureHistory.tsxfrontend/web/components/GithubStar.tsxfrontend/web/components/Highlight.jsfrontend/web/components/IntegrationList.tsxfrontend/web/components/PermissionsTabs.tsxfrontend/web/components/SegmentOverrides.jsfrontend/web/components/feature-health/components/EventTextBlocks.tsxfrontend/web/components/feature-health/components/EventUrlsBlock.tsxfrontend/web/components/feature-override/IdentityOverrideDescription.tsxfrontend/web/components/feature-override/SegmentOverrideDescription.tsxfrontend/web/components/inspect-permissions/Permissions.tsxfrontend/web/components/modals/CreateAuditLogWebhook.tsxfrontend/web/components/modals/CreateRole.tsxfrontend/web/components/modals/CreateSegmentRulesTabForm.tsxfrontend/web/components/modals/CreateSegmentUsersTabContent.tsxfrontend/web/components/modals/CreateUser.tsxfrontend/web/components/modals/CreateWebhook.tsxfrontend/web/components/modals/payment/Payment.tsxfrontend/web/components/modals/payment/PricingPanel.tsxfrontend/web/components/mv/VariationKeyLabel/VariationKeyLabel.tsxfrontend/web/components/navigation/EnvironmentAside.tsxfrontend/web/components/navigation/TabMenu/Tabs.tsxfrontend/web/components/onboarding/GettingStartedItem.tsxfrontend/web/components/onboarding/OnboardingStep.tsxfrontend/web/components/pages/AccountSettingsPage.tsxfrontend/web/components/pages/CreateOrganisationPage.tsxfrontend/web/components/pages/GettingStartedPage.tsxfrontend/web/components/pages/OnboardingPage.tsxfrontend/web/components/pages/organisation-settings/tabs/sso/saml/modals/CreateSAML.tsxfrontend/web/components/pages/sdk-keys/SDKKeysPage.tsxfrontend/web/components/pages/sdk-keys/components/ServerSideKeyRow.tsxfrontend/web/components/segments/AssociatedSegmentOverrides.tsxfrontend/web/components/tables/TableFilterItem.tsxfrontend/web/components/tables/TableSortFilter.tsxfrontend/web/components/tags/AddEditTags.tsxfrontend/web/project/project-components.jsfrontend/web/project/toast.tsxfrontend/web/styles/3rdParty/_bootstrap.scssfrontend/web/styles/_variables.scssfrontend/web/styles/project/_alert.scss
💤 Files with no reviewable changes (1)
- frontend/web/styles/project/_alert.scss
The download hint is an IonIcon, so it needs the icon token utility (icon-action sets fill) rather than text-action. Addresses review feedback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
stylelint scss/comment-no-empty on the spacer `//` lines. Addresses review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Changes
Contributes to #6606. Consolidates the dark-mode colour work into one PR (supersedes #7985, #7986, #7987) so it can be reviewed and QA'd in a single pass.
Moves ownership of colour and shadow utilities from Bootstrap to the design-system token layer. Bootstrap's utilities load later and carry
!important, so wherever a token utility shared a name it was silently dead and Bootstrap's value won. Commit-by-commit:success/danger/warning/infofrom Bootstrap's text-colour utility (values identical, no-op) + the unusedlight/black/white-50/black-50. (secondaryalready landed in fix(dark mode): resolve .text-secondary to the token, not Bootstrap #7981.)text-primary/text-dark/text-bodyto token classes:text-action/icon-actionfor the purple,text-default/icon-defaultfor body text (~43 files).text-mutedtheme-aware — point$text-mutedat--color-text-secondary(one line):#656d7blight unchanged,#9da4aedark (3.45:1 → 7.16:1, fixes AA). No usage churn.border-*danger/info/success/warning (identical, no-op);shadow-sm/shadow-lgnow use the DS--shadow-*tokens (were silently using Bootstrap's heavier shadows)..text-infopurple override — an unscoped!importantrule forced all.text-infoto purple; moved the two webhook doc links to.text-actionand restored.text-infoto info-blue.How did you test this code?
Compiled the full stylesheet and confirmed every previously-colliding utility now resolves to a single token rule:
text-secondary/success/danger/warning/info,border-success/warning,shadow-sm/lg→var(--...);text-muted→var(--color-text-secondary);text-info→ info-blue. No straytext-primary/dark/body/muted/infoclassNames remain.Single dark-mode QA pass covers everything that actually changes appearance:
text-actionelements (tabs, breadcrumbs, links, segment names) — near-black → readable purple in darkshadow-smelement — subtler DS shadow