RESOLVED FIXED 215681
Web Inspector: Elements: Styles: don't show swatches for properties that aren't used/apply
https://bugs.webkit.org/show_bug.cgi?id=215681
Summary Web Inspector: Elements: Styles: don't show swatches for properties that aren...
Devin Rousso
Reported 2020-08-19 20:03:41 PDT
a common pattern in CSS is to have two of the same property next to each other, the earlier intended as a fallback for the latter ``` color: black; color: var(--color); ``` showing a swatch in front of the former property isn't all that useful as it's not used/applied and just adds to the confusion of what is used/applied (especially after bug 215680)
Attachments
Patch (1.51 KB, patch)
2020-08-19 20:05 PDT, Devin Rousso
no flags
Devin Rousso
Comment 1 2020-08-19 20:05:28 PDT
Nikita Vasilyev
Comment 2 2020-08-19 21:05:09 PDT
I'm a little bit on the fence about this. Leaning towards that the benefits of this patch outweigh the downsides. On one hand, the strikethrough doesn't go over the swatches and it looks bad. From the visual design perspective, this patch is an improvement. On the other hand, I find it useful to see what colors are overridden.
Devin Rousso
Comment 3 2020-08-20 12:20:35 PDT
(In reply to Nikita Vasilyev from comment #2) > On one hand, the strikethrough doesn't go over the swatches and it looks bad. From the visual design perspective, this patch is an improvement. Agreed. > On the other hand, I find it useful to see what colors are overridden. Really? I've never really found this to be something I cared for. Not to mention, in my experience the thing that's overridden is often a raw color (i.e. not a variable), meaning that I can just read it to see what color is used.
Blaze Burg
Comment 4 2020-08-24 10:00:34 PDT
Comment on attachment 406903 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406903&action=review r=me with a question. > Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:466 > + if (this._property.enabled && !this._property.overridden && this._property.valid && !this._property.hasOtherVendorNameOrKeyword()) Do we support any hasOtherVendorNameOrKeyword properties that do not have an unprefixed counterpart? This seems fine, unless it means some obscure --webkit- properties no longer get swatches.
Radar WebKit Bug Importer
Comment 5 2020-08-24 10:11:13 PDT
Devin Rousso
Comment 6 2020-08-24 10:22:58 PDT
Comment on attachment 406903 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406903&action=review >> Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:466 >> + if (this._property.enabled && !this._property.overridden && this._property.valid && !this._property.hasOtherVendorNameOrKeyword()) > > Do we support any hasOtherVendorNameOrKeyword properties that do not have an unprefixed counterpart? This seems fine, unless it means some obscure --webkit- properties no longer get swatches. IIRC `hasOtherVendorNameOrKeyword` checks to see whether the name/value starts with `-moz-`/`-ms-`/`-o-`, so it shouldn't ever affect anything starting with `-webkit`.
EWS
Comment 7 2020-08-24 10:24:56 PDT
Committed r266070: <https://trac.webkit.org/changeset/266070> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406903 [details].
Note You need to log in before you can comment on or make changes to this bug.