From 8425afc1eb0f760485ec92518f65665df2afb0fe Mon Sep 17 00:00:00 2001 From: Jack Franklin Date: Tue, 25 May 2021 13:30:00 +0000 Subject: [PATCH] [DarkMode] filter.css Also discovered that the stylelint rule needs to allow `rgb(var(...))`, so fixed that as a drive-by. This will change in time but was the easiest path to getting the first migration stage done. Bug: chromium:1152736 Change-Id: I70816edee5217dc3f52804877b04dd479a6edb15 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2909588 Auto-Submit: Jack Franklin Commit-Queue: Tim van der Lippe Reviewed-by: Tim van der Lippe --- front_end/ui/legacy/FilterBar.ts | 2 +- front_end/ui/legacy/filter.css | 28 +++++++++---------- .../stylelint_rules/lib/use_theme_colors.js | 7 ++++- .../tests/use_theme_colors_test.js | 5 ++++ 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/front_end/ui/legacy/FilterBar.ts b/front_end/ui/legacy/FilterBar.ts index 35858359c6..39b91c7cfe 100644 --- a/front_end/ui/legacy/FilterBar.ts +++ b/front_end/ui/legacy/FilterBar.ts @@ -77,7 +77,7 @@ export class FilterBar extends HBox { constructor(name: string, visibleByDefault?: boolean) { super(); - this.registerRequiredCSS('ui/legacy/filter.css', {enableLegacyPatching: true}); + this.registerRequiredCSS('ui/legacy/filter.css', {enableLegacyPatching: false}); this._enabled = true; this.element.classList.add('filter-bar'); diff --git a/front_end/ui/legacy/filter.css b/front_end/ui/legacy/filter.css index 081c8ea2f2..269a6c8041 100644 --- a/front_end/ui/legacy/filter.css +++ b/front_end/ui/legacy/filter.css @@ -56,13 +56,14 @@ } .filter-bitset-filter span { + --override-background-color-base: 0 0 0; + display: inline-block; flex: none; margin: auto 2px; padding: 3px; background: transparent; - text-shadow: rgb(255 255 255 / 50%) 0 1px 0; /* stylelint-disable-line plugin/use_theme_colors */ - /* See: crbug.com/1152736 for color variable migration. */ + text-shadow: var(--color-background-opacity-50) 0 1px 0; border-radius: 6px; overflow: hidden; } @@ -72,8 +73,7 @@ } .filter-bitset-filter-divider { - background-color: #ccc; /* stylelint-disable-line plugin/use_theme_colors */ - /* See: crbug.com/1152736 for color variable migration. */ + background-color: var(--color-details-hairline); height: 16px; width: 1px; margin: auto 2px; @@ -83,25 +83,25 @@ .filter-bitset-filter span.selected, .filter-bitset-filter span:hover, .filter-bitset-filter span:active { - color: #fff; /* stylelint-disable-line plugin/use_theme_colors */ - /* See: crbug.com/1152736 for color variable migration. */ - text-shadow: rgb(0 0 0 / 40%) 0 1px 0; /* stylelint-disable-line plugin/use_theme_colors */ - /* See: crbug.com/1152736 for color variable migration. */ + color: var(--color-background); + text-shadow: rgb(var(--override-background-color-base) / 40%) 0 1px 0; } .filter-bitset-filter span:hover { - background: rgb(0 0 0 / 20%); /* stylelint-disable-line plugin/use_theme_colors */ - /* See: crbug.com/1152736 for color variable migration. */ + background: rgb(var(--override-background-color-base) / 20%); } .filter-bitset-filter span.selected { - background: rgb(0 0 0 / 30%); /* stylelint-disable-line plugin/use_theme_colors */ - /* See: crbug.com/1152736 for color variable migration. */ + background: rgb(var(--override-background-color-base) / 30%); } .filter-bitset-filter span:active { - background: rgb(0 0 0 / 50%); /* stylelint-disable-line plugin/use_theme_colors */ - /* See: crbug.com/1152736 for color variable migration. */ + background: rgb(var(--override-background-color-base) / 50%); +} + +.-theme-with-dark-background .filter-bitset-filter span, +:host-context(.-theme-with-dark-background) .filter-bitset-filter span { + --override-background-color-base: 255 255 255; } .filter-checkbox-filter { diff --git a/scripts/stylelint_rules/lib/use_theme_colors.js b/scripts/stylelint_rules/lib/use_theme_colors.js index 7c6f8fb1e6..d7c17e41fa 100644 --- a/scripts/stylelint_rules/lib/use_theme_colors.js +++ b/scripts/stylelint_rules/lib/use_theme_colors.js @@ -101,7 +101,12 @@ module.exports = stylelint.createPlugin(RULE_NAME, function(primary, secondary, function checkColorValueIsValidOrError({declarationToErrorOn, cssValueToCheck, alreadyFixed}) { for (const indicator of COLOR_INDICATOR_REGEXES) { - if (indicator.test(cssValueToCheck)) { + /** + * In rare situations in the codebase we allow + * rgb(var(--some-base-color) / 20%) so we don't want to error if we + * match that. + */ + if (indicator.test(cssValueToCheck) && !cssValueToCheck.startsWith('rgb(var')) { reportError(declarationToErrorOn, !alreadyFixed); } } diff --git a/scripts/stylelint_rules/tests/use_theme_colors_test.js b/scripts/stylelint_rules/tests/use_theme_colors_test.js index e32ac145f3..12496dabc9 100644 --- a/scripts/stylelint_rules/tests/use_theme_colors_test.js +++ b/scripts/stylelint_rules/tests/use_theme_colors_test.js @@ -158,6 +158,11 @@ describe('use_theme_colors', () => { assert.lengthOf(warnings, 0); }); + it('allows variables within rgb', async () => { + const warnings = await lint('p { background: rgb(var(--override-base-color) / 20%); }'); + assert.lengthOf(warnings, 0); + }); + it('allows any color to be used when in a :host-context dark theme block', async () => { const code = `:host-context(.-theme-with-dark-background) p { color: #fff;