| Summary: | Web Inspector: Elements: Styles: don't show swatches for properties that aren't used/apply | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Devin Rousso <hi> | ||||
| Component: | Web Inspector | Assignee: | Devin Rousso <hi> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | bburg, hi, inspector-bugzilla-changes, nvasilyev, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Bug Depends on: | 215680 | ||||||
| Bug Blocks: | |||||||
| Attachments: |
|
||||||
Created attachment 406903 [details]
Patch
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. (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. 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. 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`. Committed r266070: <https://trac.webkit.org/changeset/266070> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406903 [details]. |
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)