[webkit-reviews] review granted: [Bug 201442] Web Inspector: Dark Mode: Overview Timeline View has jarring white box-shadows in dark mode : [Attachment 378221] [PATCH] Proposed Fix

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 201442: Web Inspector: Dark Mode: Overview Timeline View has jarring white
box-shadows in dark mode
https://bugs.webkit.org/show_bug.cgi?id=201442

Attachment 378221: [PATCH] Proposed Fix

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




--- Comment #13 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 378221
  --> https://bugs.webkit.org/attachment.cgi?id=378221
[PATCH] Proposed Fix

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

r-me, nice fix!

We spoke about this in person, but I think we should switch it such that the
first row is the one with the "odd" background color (e.g. it's different from
the background color of the surrounding content, like a navigation bar).  This
way, it's easier to identify where the table begins, because the first row has
a different background color.  I'm also fine with that being a followup.

> Source/WebInspectorUI/ChangeLog:27
> +	   (:root):

I think this is missing a `* UserInterface/Views/Main.css:`

> Source/WebInspectorUI/UserInterface/Views/DataGrid.js:1145
> +	   this._dataTableElement.classList.toggle("even-first-zebra-stripe",
!!(topHiddenRowCount % 2));

We should probably add a comment like "if there are an odd number of rows
hidden, meaning that the first visible row must be an even row".

> Source/WebInspectorUI/UserInterface/Views/OverviewTimelineView.css:97
> +	   --timeline-record-bar-selected-inactive-box-shadow-color: hsla(0,
0%, 35%);

This can just be `hsl()`.

> Source/WebInspectorUI/UserInterface/Views/OverviewTimelineView.css:101
> +	   --timeline-record-bar-selected-inactive-box-shadow-color: hsla(0,
0%, 38%);

Ditto (97).

> Source/WebInspectorUI/UserInterface/Views/Table.js:1130
> +	   this._listElement.classList.toggle("even-first-zebra-stripe",
!!(topHiddenRowCount % 2));

Ditto (DataGrid.js:1145).