Bug 135705

Summary: Web Inspector: Update glyphs to be more like Xcode 6
Product: WebKit Reporter: Timothy Hatcher <timothy>
Component: Web InspectorAssignee: Timothy Hatcher <timothy>
Status: RESOLVED FIXED    
Severity: Normal CC: graouts, joepeck, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
joepeck: review+, timothy: commit-queue-
New Screenshot 1
none
New Screenshot 2
none
New Screenshot 3
none
New Screenshot 4 none

Description Timothy Hatcher 2014-08-07 10:25:54 PDT
Xcode 6 changes the glpyhs in the UI to match the new look better. We should pick up these changes in Web Inspector.

<rdar://problem/17846770>
Comment 1 Timothy Hatcher 2014-08-07 10:42:37 PDT
Created attachment 236196 [details]
Patch
Comment 2 Timothy Hatcher 2014-08-07 10:48:37 PDT
Created attachment 236197 [details]
New Screenshot 1
Comment 3 Timothy Hatcher 2014-08-07 10:48:48 PDT
Created attachment 236198 [details]
New Screenshot 2
Comment 4 Timothy Hatcher 2014-08-07 10:49:00 PDT
Created attachment 236199 [details]
New Screenshot 3
Comment 5 Timothy Hatcher 2014-08-07 10:49:14 PDT
Created attachment 236200 [details]
New Screenshot 4
Comment 6 Joseph Pecoraro 2014-08-07 11:23:29 PDT
Comment on attachment 236196 [details]
Patch

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

r=me

The screenshots look awesome!

Also, I wonder if we should avoid including the Images/Legacy directory on 10.10 entirely.

In Source/WebInspectorUI/Scripts/copy-user-interface-resources.pl we could remove Images/Legacy directory after copying Images if we are not a legacy mac:

    ditto(File::Spec->catdir($uiRoot, 'Images'), File::Spec->catdir($targetResourcePath, 'Images'));

Tools/Scripts/webkitdirs.pm has some small functions we can copy to check the OS and version, e.g. isDarwin() and osXVersion().

That said, this could be done in a follow-up.

> Source/WebInspectorUI/ChangeLog:218
> +2014-08-07  Timothy Hatcher  <timothy@apple.com>
> +
> +        Need a short description (OOPS!).
> +        Need the bug URL (OOPS!).
> +
> +        Reviewed by NOBODY (OOPS!).

Nit: Double ChangeLogs everywhere.

> Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.css:42
>  body.mac-platform.legacy .style-declaration-section + .style-declaration-section {
> -    border-top: 1px solid rgb(210, 210, 210);
> +    border-top: 1px solid rgb(179, 179, 179);
>  }

You're changing the border-top in the legacy as well? You changed this color in a few different files, so I suspect it was intended.

> Source/WebInspectorUI/UserInterface/Views/DataGrid.js:1110
> +        generateColoredImagesForCSS(WebInspector.Platform.isLegacyMacOS ? "Images/Legacy/SortIndicatorDownArrow.svg" : "Images/SortIndicatorDownArrow.svg", specifications, 9, 8, "data-grid-sort-indicator-down-");
> +        generateColoredImagesForCSS(WebInspector.Platform.isLegacyMacOS ? "Images/Legacy/SortIndicatorUpArrow.svg" : "Images/SortIndicatorUpArrow.svg", specifications, 9, 8, "data-grid-sort-indicator-up-");

For cases like this, I wonder if it would be easier to just have a ImageUtilities helper. Something like that:

    function platformImage(img) {
        return WebInspector.Platform.isLegacyMacOS ? "Images/Legacy/" + img : "Images/" + img;
    }

    ..., platformImage("SortIndicatorDownArrow.svg"), ...

Of course where size changes we have to be explicit.

> Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.css:236
>  .navigation-sidebar-panel-content-tree-outline .item.selected {
> -    color: white;
> -
> -    background-color: rgb(185, 184, 185);
> +    background-color: rgb(212, 212, 212);
>  }

Does legacy need color:white?
Comment 7 Timothy Hatcher 2014-08-07 13:32:38 PDT
Committed r172241. https://trac.webkit.org/r172241
Comment 8 Timothy Hatcher 2014-08-07 14:19:26 PDT
Follow up fix: https://trac.webkit.org/r172263