IMO, the strikethrough isn't a visually obvious enough indicator that "this property is NOT being used/applied"
Created attachment 406902 [details] Patch
Screenshot?
Comment on attachment 406902 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406902&action=review > Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css:114 > +body:not(.meta-key-pressed) .spreadsheet-style-declaration-editor > .property:matches(.invalid-name, .invalid-value, .other-vendor, .overridden):not(.disabled) > .content > :matches(.name, .value-container):not(.editing), > +body:not(.meta-key-pressed) .spreadsheet-style-declaration-editor > .property:matches(.invalid-name, .invalid-value, .other-vendor, .overridden):not(.disabled) > .content > .value-container > .value:not(.editing), > +body.meta-key-pressed .spreadsheet-style-declaration-editor > .property:matches(.invalid-name, .invalid-value, .other-vendor, .overridden):not(.disabled) > .content > :matches(.name, .value-container):not(:hover, .editing), > +body.meta-key-pressed .spreadsheet-style-declaration-editor > .property:matches(.invalid-name, .invalid-value, .other-vendor, .overridden):not(.disabled) > .content > .value-container > .value:not(:hover, .editing) { That's one long selector. I see it isn't particularly new — two rules below use very similar selector. The `body:not(.meta-key-pressed)` and `body.meta-key-pressed` gives me a pause in particular. I wonder how we can simplify it. > Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css:115 > + color: hsla(0, 0%, var(--foreground-lightness), 0.6); Any particular reason not to use `var(--text-color-secondary)`?
Comment on attachment 406902 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406902&action=review >> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css:114 >> +body.meta-key-pressed .spreadsheet-style-declaration-editor > .property:matches(.invalid-name, .invalid-value, .other-vendor, .overridden):not(.disabled) > .content > .value-container > .value:not(:hover, .editing) { > > That's one long selector. I see it isn't particularly new — two rules below use very similar selector. > The `body:not(.meta-key-pressed)` and `body.meta-key-pressed` gives me a pause in particular. I wonder how we can simplify it. `body:not(.meta-key-pressed)` includes `:hover` `body.meta-key-pressed` excludes `:hover` this is also "necessary" because of specificity conflicts with other rules (which possibly could be resolved using additional `:not`, but that's likely a larger change) >> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css:115 >> + color: hsla(0, 0%, var(--foreground-lightness), 0.6); > > Any particular reason not to use `var(--text-color-secondary)`? I wanted to match the color of the strikethrough.
Comment on attachment 406902 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406902&action=review >>> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css:114 >>> +body.meta-key-pressed .spreadsheet-style-declaration-editor > .property:matches(.invalid-name, .invalid-value, .other-vendor, .overridden):not(.disabled) > .content > .value-container > .value:not(:hover, .editing) { >> >> That's one long selector. I see it isn't particularly new — two rules below use very similar selector. >> The `body:not(.meta-key-pressed)` and `body.meta-key-pressed` gives me a pause in particular. I wonder how we can simplify it. > > `body:not(.meta-key-pressed)` includes `:hover` > `body.meta-key-pressed` excludes `:hover` > > this is also "necessary" because of specificity conflicts with other rules (which possibly could be resolved using additional `:not`, but that's likely a larger change) Okay, I think it's good for now. >>> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css:115 >>> + color: hsla(0, 0%, var(--foreground-lightness), 0.6); >> >> Any particular reason not to use `var(--text-color-secondary)`? > > I wanted to match the color of the strikethrough. I think that strikethrough should also be `var(--text-color-secondary)`.
Created attachment 406966 [details] [Image] Screenshot of issue
Created attachment 406967 [details] [Image] After Patch is applied
Comment on attachment 406967 [details] [Image] After Patch is applied Looking at this more closely, it appears that the `:` and `;` are also re-colored, which is probably not necessary. I'll tweak the selector to not include those.
Created attachment 407085 [details] Patch
Comment on attachment 407085 [details] Patch r=me, these huge selectors make me dizzy though.
Committed r266066: <https://trac.webkit.org/changeset/266066> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407085 [details].
<rdar://problem/67685695>