Bug 212398 – Web Inspector: match the undocked tab bar style when docked bottom/side

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

see screenshots attached to <https://webkit.org/b/204627>

[Devin Rousso]

Created attachment 400297 [details]
Patch

[Devin Rousso]

Created attachment 400298 [details]
[Image] after Patch is applied (docked bottom - light)

[Devin Rousso]

Created attachment 400299 [details]
[Image] after Patch is applied (docked side - light)

[Devin Rousso]

Created attachment 400300 [details]
[Image] after Patch is applied (docked bottom - dark)

[Devin Rousso]

Created attachment 400301 [details]
[Image] after Patch is applied (docked side - dark)

[Maciej Stachowiak]

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.

[Devin Rousso]

Created attachment 435202 [details]
Patch

[Devin Rousso]

Created attachment 435203 [details]
[Image] after Patch is applied (docked bottom - light)

[Devin Rousso]

Created attachment 435204 [details]
[Image] after Patch is applied (docked side - light)

[Devin Rousso]

Created attachment 435205 [details]
[Image] after Patch is applied (docked bottom - dark)

[Devin Rousso]

Created attachment 435206 [details]
[Image] after Patch is applied (docked side - dark)

[Joseph Pecoraro]

Looks nice!

[Patrick Angle]

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...

[Blaze Burg]

(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?

[EWS]

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].

[Radar WebKit Bug Importer]

<rdar://problem/82064001>