Bug 212398 - Web Inspector: match the undocked tab bar style when docked bottom/side
Summary: Web Inspector: match the undocked tab bar style when docked bottom/side
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on: 204627
Blocks:
  Show dependency tree / graph
 
Reported: 2020-05-26 19:29 PDT by Devin Rousso
Modified: 2021-08-17 23:51 PDT (History)
8 users (show)

See Also:

Attachments
Patch (30.68 KB, patch)
2020-05-26 19:52 PDT, Devin Rousso 		no flags Details | Formatted Diff | Diff
[Image] after Patch is applied (docked bottom - light) (3.92 MB, image/png)
2020-05-26 19:53 PDT, Devin Rousso 		no flags Details
[Image] after Patch is applied (docked side - light) (3.40 MB, image/png)
2020-05-26 19:53 PDT, Devin Rousso 		no flags Details
[Image] after Patch is applied (docked bottom - dark) (3.97 MB, image/png)
2020-05-26 20:00 PDT, Devin Rousso 		no flags Details
[Image] after Patch is applied (docked side - dark) (3.50 MB, image/png)
2020-05-26 20:01 PDT, Devin Rousso 		no flags Details
Patch (47.30 KB, patch)
2021-08-09 13:12 PDT, Devin Rousso 		no flags Details | Formatted Diff | Diff
[Image] after Patch is applied (docked bottom - light) (3.40 MB, image/png)
2021-08-09 13:12 PDT, Devin Rousso 		no flags Details
[Image] after Patch is applied (docked side - light) (3.02 MB, image/png)
2021-08-09 13:13 PDT, Devin Rousso 		no flags Details
[Image] after Patch is applied (docked bottom - dark) (3.43 MB, image/png)
2021-08-09 13:13 PDT, Devin Rousso 		no flags Details
[Image] after Patch is applied (docked side - dark) (2.48 MB, image/png)
2021-08-09 13:13 PDT, Devin Rousso 		no flags Details
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2020-05-26 19:29:35 PDT 
see screenshots attached to <https://webkit.org/b/204627>
Comment 1 Devin Rousso 2020-05-26 19:52:21 PDT 
Created attachment 400297 [details]
Patch
Comment 2 Devin Rousso 2020-05-26 19:53:21 PDT 
Created attachment 400298 [details]
[Image] after Patch is applied (docked bottom - light)
Comment 3 Devin Rousso 2020-05-26 19:53:50 PDT 
Created attachment 400299 [details]
[Image] after Patch is applied (docked side - light)
Comment 4 Devin Rousso 2020-05-26 20:00:49 PDT 
Created attachment 400300 [details]
[Image] after Patch is applied (docked bottom - dark)
Comment 5 Devin Rousso 2020-05-26 20:01:04 PDT 
Created attachment 400301 [details]
[Image] after Patch is applied (docked side - dark)
Comment 6 Maciej Stachowiak 2020-05-26 20:59:11 PDT 
Comment on attachment 400297 [details]
Patch

I don’t think this is an improvement. While it avoids the problem of all white, it reintroduces the problem when tabs stack in the side docked configuration (and makes bottom-docked look weird). I don’t want to get into a review fight here but I don’t think this is shippable.
Comment 7 Devin Rousso 2021-08-09 13:12:17 PDT 
Created attachment 435202 [details]
Patch
Comment 8 Devin Rousso 2021-08-09 13:12:39 PDT 
Created attachment 435203 [details]
[Image] after Patch is applied (docked bottom - light)
Comment 9 Devin Rousso 2021-08-09 13:13:00 PDT 
Created attachment 435204 [details]
[Image] after Patch is applied (docked side - light)
Comment 10 Devin Rousso 2021-08-09 13:13:18 PDT 
Created attachment 435205 [details]
[Image] after Patch is applied (docked bottom - dark)
Comment 11 Devin Rousso 2021-08-09 13:13:36 PDT 
Created attachment 435206 [details]
[Image] after Patch is applied (docked side - dark)
Comment 12 Joseph Pecoraro 2021-08-09 13:23:20 PDT 
Looks nice!
Comment 13 Patrick Angle 2021-08-09 15:59:01 PDT 
Comment on attachment 435202 [details]
Patch

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

Looks great! Glad to see a single design for this between modes instead of two different designs with different spacing.

> Source/WebCore/inspector/InspectorFrontendHost.cpp:396
> +#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 120000

Is there a specific reason we don't use the platform macros for these version numbers (which I think is what the style bot is grumpy about) e.g. `__MAC_12_0`, `__MAC_11_0`, `__MAC_10_15`, and `__MAC_10_14`? Similar use already in `/source/thirdparty/angle/src/libangle/renderer/metal/mtl_utils.mm`. I do see many other places we have hardcoded these values throughout WebKit though...
Comment 14 BJ Burg 2021-08-10 08:31:06 PDT 
(In reply to Patrick Angle from comment #13)
> Comment on attachment 435202 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=435202&action=review
> 
> Looks great! Glad to see a single design for this between modes instead of
> two different designs with different spacing.
> 
> > Source/WebCore/inspector/InspectorFrontendHost.cpp:396
> > +#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 120000
> 
> Is there a specific reason we don't use the platform macros for these
> version numbers (which I think is what the style bot is grumpy about) e.g.
> `__MAC_12_0`, `__MAC_11_0`, `__MAC_10_15`, and `__MAC_10_14`? Similar use
> already in
> `/source/thirdparty/angle/src/libangle/renderer/metal/mtl_utils.mm`. I do
> see many other places we have hardcoded these values throughout WebKit
> though...

Those are macros in 3rd party code. I believe WebKit is still using hardcoded version numbers.. but I could be wrong?
Comment 15 EWS 2021-08-17 23:50:23 PDT 
Committed r281182 (240627@main): <https://commits.webkit.org/240627@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 435202 [details].
Comment 16 Radar WebKit Bug Importer 2021-08-17 23:51:32 PDT 
<rdar://problem/82064001>