Bug 179654 - Web Inspector: Cleanup navigation bar dividers and separators
Summary: Web Inspector: Cleanup navigation bar dividers and separators
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: Matt Baker
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-11-13 20:30 PST by Matt Baker
Modified: 2017-11-14 00:49 PST (History)
6 users (show)

See Also:


Attachments
Patch (5.18 KB, patch)
2017-11-13 20:44 PST, Matt Baker
no flags Details | Formatted Diff | Diff
[Image] before/after (219.43 KB, image/png)
2017-11-13 21:00 PST, Matt Baker
no flags Details
Patch (6.00 KB, patch)
2017-11-14 00:14 PST, Matt Baker
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Baker 2017-11-13 20:30:48 PST
Two small things:

- Hierarchical path arrow should have a simpler appearance, instead of a tapered arrow head.
- The number of dividers added ContentBrowser to its navigation bar is excessive. It makes sense to have dividers between the regular buttons and the left/right sidebar buttons. Other than that, let content views add their own dividers as needed.
Comment 1 Radar WebKit Bug Importer 2017-11-13 20:33:41 PST
<rdar://problem/35523734>
Comment 2 Matt Baker 2017-11-13 20:44:17 PST
Created attachment 326847 [details]
Patch
Comment 3 Matt Baker 2017-11-13 21:00:41 PST
Created attachment 326849 [details]
[Image] before/after
Comment 4 Devin Rousso 2017-11-13 21:19:32 PST
Comment on attachment 326847 [details]
Patch

r=me.

I think that changing the chevron is awesome, but I think we might want to adjust the margin to make it more even on both sides.  With this patch (and before it too actually), it looked like the divider was shifted more towards the left, and there was an uneven amount of space when compared to the right side.  I think we should adjust this.

HierarchicalPathComponent.css
    .hierarchical-path-component > .separator {
        --path-component-separator-margin-start: 4px; // or use `-webkit-margin-start: 4px;`
    }

Also, it's slightly weird to have a separator before/after the sidebar icons, but not elsewhere.  I'm not sure if Xcode has anything similar :(
Comment 5 Matt Baker 2017-11-13 23:34:58 PST
(In reply to Devin Rousso from comment #4)
> Comment on attachment 326847 [details]
> Patch
> 
> r=me.
> 
> I think that changing the chevron is awesome, but I think we might want to
> adjust the margin to make it more even on both sides.  With this patch (and
> before it too actually), it looked like the divider was shifted more towards
> the left, and there was an uneven amount of space when compared to the right
> side.  I think we should adjust this.

I'll take a look.

> 
> HierarchicalPathComponent.css
>     .hierarchical-path-component > .separator {
>         --path-component-separator-margin-start: 4px; // or use
> `-webkit-margin-start: 4px;`
>     }
> 
> Also, it's slightly weird to have a separator before/after the sidebar
> icons, but not elsewhere.  I'm not sure if Xcode has anything similar :(

In general Xcode doesn't use dividers between toolbar items. The only instance I found is in the Debug area, bottom right. A divider separates the filter buttons from the Variables/Console view buttons.

I like using them to separate sidebar buttons from normal buttons, and between groups of related buttons.
Comment 6 Matt Baker 2017-11-14 00:14:48 PST
Created attachment 326864 [details]
Patch
Comment 7 Matt Baker 2017-11-14 00:17:04 PST
(In reply to Matt Baker from comment #5)
> (In reply to Devin Rousso from comment #4)
> > Comment on attachment 326847 [details]
> > Patch
> > 
> > r=me.
> > 
> > I think that changing the chevron is awesome, but I think we might want to
> > adjust the margin to make it more even on both sides.  With this patch (and
> > before it too actually), it looked like the divider was shifted more towards
> > the left, and there was an uneven amount of space when compared to the right
> > side.  I think we should adjust this.
> 
> I'll take a look.
> 
> > 
> > HierarchicalPathComponent.css
> >     .hierarchical-path-component > .separator {
> >         --path-component-separator-margin-start: 4px; // or use
> > `-webkit-margin-start: 4px;`
> >     }
> >

Went with 3px, which was an improvement. 4px looked too wide (maybe due to the left column of pixels in the separator SVG being empty space).
Comment 8 WebKit Commit Bot 2017-11-14 00:49:17 PST
Comment on attachment 326864 [details]
Patch

Clearing flags on attachment: 326864

Committed r224807: <https://trac.webkit.org/changeset/224807>
Comment 9 WebKit Commit Bot 2017-11-14 00:49:19 PST
All reviewed patches have been landed.  Closing bug.