Summary: | Web Inspector: system accent color follow-ups | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Matt Baker <mattbaker> | ||||||||||||
Component: | Web Inspector | Assignee: | Matt Baker <mattbaker> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | commit-queue, hi, inspector-bugzilla-changes, nvasilyev, webkit-bug-importer | ||||||||||||
Priority: | P3 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | All | ||||||||||||||
Attachments: |
|
Description
Matt Baker
2019-02-28 15:46:25 PST
Created attachment 363270 [details]
Patch
Created attachment 363271 [details]
[Image] New accent color uses
Comment on attachment 363270 [details]
Patch
Reviewable, but investigating a ButtonToolbarItem issue.
Created attachment 363284 [details]
Patch
Created attachment 363285 [details]
[Image] Inspect Element button
Comparison of light/dark, new/old.
(In reply to Matt Baker from comment #6) > Created attachment 363285 [details] > [Image] Inspect Element button > > Comparison of light/dark, new/old. The CSS shown to the left refers to the "Patch" column only. Comment on attachment 363284 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363284&action=review r=me > Source/WebInspectorUI/UserInterface/Views/ButtonToolbarItem.css:73 > + filter: brightness(1.35); Nice! > Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.css:45 > + background-color: var(--selected-text-background-color) !important; This is almost too light now. It makes it harder to notice for small record bars. Can you darken it a bit? Comment on attachment 363284 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363284&action=review Looks all good to me! >> Source/WebInspectorUI/UserInterface/Views/ButtonToolbarItem.css:73 >> + filter: brightness(1.35); > > Nice! I honestly didn't notice that the color was different — it just looked right! >> Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.css:45 >> + background-color: var(--selected-text-background-color) !important; > > This is almost too light now. It makes it harder to notice for small record bars. Can you darken it a bit? Too light on what configuration? It doesn't seem too light to me. I tried both Dark and Light modes with various accent colors. Created attachment 363776 [details]
[Image] Selected timeline segment with patch applied
Looks good to me.
(In reply to Nikita Vasilyev from comment #10) > Created attachment 363776 [details] > [Image] Selected timeline segment with patch applied > > Looks good to me. I personally think that that doesn't match the "pattern" of the other records. The difference between the border and the content of the selected record is greater than that of the non-selected records. Basically, I think we should make the light-blue slightly darker (or make the border slightly lighter) so that the difference is less pronounced. (In reply to Devin Rousso from comment #11) > (In reply to Nikita Vasilyev from comment #10) > > Created attachment 363776 [details] > > [Image] Selected timeline segment with patch applied > > > > Looks good to me. > I personally think that that doesn't match the "pattern" of the other > records. The difference between the border and the content of the selected > record is greater than that of the non-selected records. > > Basically, I think we should make the light-blue slightly darker (or make > the border slightly lighter) so that the difference is less pronounced. I see what you mean now. If anything, the difference should be more pronounced. Try selecting red bars with red accent colors! Comment on attachment 363284 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363284&action=review >>> Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.css:45 >>> + background-color: var(--selected-text-background-color) !important; >> >> This is almost too light now. It makes it harder to notice for small record bars. Can you darken it a bit? > > Too light on what configuration? > > It doesn't seem too light to me. I tried both Dark and Light modes with various accent colors. I see what you mean, but I think the current approach is fine for now - it's simple and uses unmodified system colors. Comment on attachment 363284 [details] Patch Clearing flags on attachment: 363284 Committed r242578: <https://trac.webkit.org/changeset/242578> All reviewed patches have been landed. Closing bug. |