| Summary: | Web Inspector: match the undocked tab bar style when docked bottom/side | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Devin Rousso <hi> |
| Component: | Web Inspector | Assignee: | Devin Rousso <hi> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | bburg, ews-watchlist, hi, inspector-bugzilla-changes, joepeck, pangle, timothy, webkit-bug-importer |
| Priority: | P2 | Keywords: | InRadar |
| Version: | WebKit Nightly Build | ||
| Hardware: | All | ||
| OS: | All | ||
| Bug Depends on: | 204627 | ||
| Bug Blocks: | |||
| Attachments: | |||
|
Description
Devin Rousso
2020-05-26 19:29:35 PDT
Created attachment 400297 [details]
Patch
Created attachment 400298 [details]
[Image] after Patch is applied (docked bottom - light)
Created attachment 400299 [details]
[Image] after Patch is applied (docked side - light)
Created attachment 400300 [details]
[Image] after Patch is applied (docked bottom - dark)
Created attachment 400301 [details]
[Image] after Patch is applied (docked side - dark)
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.
Created attachment 435202 [details]
Patch
Created attachment 435203 [details]
[Image] after Patch is applied (docked bottom - light)
Created attachment 435204 [details]
[Image] after Patch is applied (docked side - light)
Created attachment 435205 [details]
[Image] after Patch is applied (docked bottom - dark)
Created attachment 435206 [details]
[Image] after Patch is applied (docked side - dark)
Looks nice! 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... (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? 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]. |