| Summary: | REGRESSION(r201177): Web Inspector: ContentBrowser navigation bar should fit on a single line | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Matt Baker <mattbaker> | ||||||||||||
| Component: | Web Inspector | Assignee: | Nikita Vasilyev <nvasilyev> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | bburg, commit-queue, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||
| Hardware: | All | ||||||||||||||
| OS: | All | ||||||||||||||
| Attachments: |
|
||||||||||||||
*** Bug 158885 has been marked as a duplicate of this bug. *** Regressed in http://trac.webkit.org/changeset/201177. This is similar to bug 157950. Created attachment 281669 [details]
WIP
This fixes the bug.
I'm looking if there are any other areas affected
and if there's a way to solve this without setting
a fixed height.
Comment on attachment 281669 [details]
WIP
Curious, why is the height -1?
(In reply to comment #6) > Comment on attachment 281669 [details] > WIP > > Curious, why is the height -1? Good question. This is how it was before r201177. It slightly changes vertical alignment of items. Created attachment 281766 [details]
[Animated GIF] An attempt to fix the bug
I tried fixing this without setting a fixed item height.
I expected it to be easy, but so far I had no luck.
1. Adding `flex-wrap: nowrap` makes all the items to fit on a single line.
Unfortunately, it also changes height of navigation bar items in the sidebar.
2. Adding `align-items: center` fixes the height issues.
Unfortunately, it makes dividers invisible.
Created attachment 281769 [details]
[Animated GIF] --navigation-bar-height vs --navigation-bar-height - 1px
Created attachment 281770 [details]
Patch
Comment on attachment 281770 [details]
Patch
I don't understand how the analysis in this bug leads to this fix. Do we know why the bar started wrapping? Why do items in the navigation bar need to be shorter by 1 pixel? Is there a bug in the manual layout that's caused by setting height: auto instead of an explicit size? I don't think fiddling with the flex-* properties will lead to a fix if the bug is in our manual layout. Similarly, adjusting by 1px seems like a brittle fix if we don't know why it's necessary.
Comment on attachment 281770 [details]
Patch
r=me, seems fine.
(In reply to comment #7) > (In reply to comment #6) > > Comment on attachment 281669 [details] > > WIP > > > > Curious, why is the height -1? > > Good question. This is how it was before r201177. > It slightly changes vertical alignment of items. Better answer: .navigation-bar is 29px tall, but it has a 1px border at the bottom. Since we use `box-sizing: border-box`, navigation bar items should be 28px tall, one pixel less than the navigation bar. Comment on attachment 281770 [details] Patch Clearing flags on attachment: 281770 Committed r202294: <http://trac.webkit.org/changeset/202294> All reviewed patches have been landed. Closing bug. (In reply to comment #11) > Comment on attachment 281770 [details] > Patch > > I don't understand how the analysis in this bug leads to this fix. Do we > know why the bar started wrapping? Yes, as Matt mentioned, it regressed in r201177. Specifically, this change: http://trac.webkit.org/changeset/201177/trunk/Source/WebInspectorUI/UserInterface/Views/NavigationBar.css > Why do items in the navigation bar need > to be shorter by 1 pixel? Is there a bug in the manual layout that's caused > by setting height: auto instead of an explicit size? I don't think fiddling > with the flex-* properties will lead to a fix if the bug is in our manual > layout. I think this is just how flexbox works, even though I find it not very intuitive. > Similarly, adjusting by 1px seems like a brittle fix if we don't > know why it's necessary. See my comment above. Comment on attachment 281770 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281770&action=review > Source/WebInspectorUI/UserInterface/Views/ContentBrowser.css:37 > + height: calc(var(--navigation-bar-height) - 1px); I should have just used `height: 100%`. |
Created attachment 281182 [details] [Image] wrapping * SUMMARY ContentBrowser navigation bar should fit on a single line. * STEPS TO REPRODUCE 1. Inspector > Timelines 2. Select Network timeline, navigation bar path grows 3. Reduce inspector window width to the minimum => "Show Details Sidebar" button wraps, controls become misaligned (see screenshot)