[webkit-reviews] review granted: [Bug 214563] Web Inspector: Change DataGrid and Table styles to closer match macOS : [Attachment 405880] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Nikita Vasilyev
<nvasilyev at apple.com>'s request for review:
Bug 214563: Web Inspector: Change DataGrid and Table styles to closer match
macOS
https://bugs.webkit.org/show_bug.cgi?id=214563

Attachment 405880: Patch

https://bugs.webkit.org/attachment.cgi?id=405880&action=review




--- Comment #53 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 405880
  --> https://bugs.webkit.org/attachment.cgi?id=405880
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=405880&action=review

r=me, thanks for iterating.  A few last things:
 - (minor, can be followup) the border in the header and body before the graph
column in the Timelines Tab don't align horizontally
 - (minor, can be followup) the border in the header and body before the name
column in the Network Tab don't align horizontally
 - (minor, can be followup) the border in the header before and after each
column should expand to the full height when that column is `:active`
 - (needs discussion, can be followup) columns that have icons/disclosures
should have their header text indented so that all the text horizontally aligns

> Source/WebInspectorUI/ChangeLog:12
> +	   - Refactoring: add "sorted" CSS class on sorted header columns to
replace `th:matches(.sort-ascending, .sort-descending)`
> +	     that is used 10 times with `.sorted`.

I don't think we should do this.  It just adds another class that can already
be expressed otherwise in CSS, and adds yet another thing that needs to be kept
track of when manipulating the DOM of a `WI.DataGrid`/`WI.Table`.

At the very least, this should be in its own patch as it's not critical to
resolving this bug.

> Source/WebInspectorUI/UserInterface/Views/Variables.css:58
> +    --background-color-pressed: hsla(0, 0%, var(--foreground-lightness),
0.05);

I think we normally use "active", as that matches the CSS `:active` (and the
use cases above).