Closed Bug 1307408 Opened 9 years ago Closed 8 years ago

Various layout problems when the inspector sidebar is very small

Categories

(DevTools :: Inspector, defect, P2)

Product:
DevTools
Component:
Inspector
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED INVALID

People

(Reporter: pbro, Assigned: jdescottes)

References

Details

Attachments

(2 files)

Bug 1307408 - part1: fix layout issues for small widths in inspector sidebar;
58 bytes, text/x-review-board-request
Honza
: review+
Details
bug1307408-min-width-reason.png
238.98 KB, image/png
Details
This might need to be a meta bug if it's easier to fix these issues separately. STR: - Open about:home - Open the inspector panel - Grab the splitter and make the sidebar really small so you start seeing problems (at its minimum, 50px, the sidebar is so small that you don't see anything anymore, so make it a bit bigger) - Go through the various sidebar panels and notice these layout problems: 1) computed-view In the box-model widget, the various labels "margin", "padding", ... overlap the values Also the various boxes (the purple padding box for instance) overflow out of the padding and margin boxes while they should stay inside The Browser Styles checkbox also becomes incorrectly vertically aligned and overflows its container. 2) Animations The "All Animations" label also overflows its container. When there are animations displayed in the panel, only the list of DOM nodes is visible, not the animations themselves, and there's no horizontal scrollbar to get to them. There should be a min-width that causes a scrollbar to appear so the whole content of the panel is visible. 3) Fonts The "Show all fonts used" label overflows its container.
Not a regression, but this originally comes from bug 1262443 and one of its follow-ups since we now can make the sidebar much smaller than we used to.
Blocks: top-inspector-bugs
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Comment on attachment 8797582 [details] Bug 1307408 - part1: fix layout issues for small widths in inspector sidebar; Just a first improvement here, making sure no scrollbars are displayed when the inspector has a small width. Patrick: can you give it a try and let me know if you'd be ok to land this first and then address improvements in a follow up (such as adding horizontal scrollbars). Note this also should address Bug 1307404.
Attachment #8797582 - Flags: feedback?(pbrosset)
added Helen in cc, as we discussed this on IRC today. Here is a try push with builds for windows/osx/linux if anyone wants to try. https://treeherder.mozilla.org/#/jobs?repo=try&revision=c77c31b7f97b10f6002e69e737b1deb5a8d7536b
Comment on attachment 8797582 [details] Bug 1307408 - part1: fix layout issues for small widths in inspector sidebar; I've tested this quickly, it looks really good. Thanks for these changes. Yes, I do think we should land this first and then tackle other individual problems later. Just one weird thing I noticed: if you drag the splitter to the left quickly, then the markup-view may scroll within its frame. I hope it's visible in the GIF: https://dl.dropboxusercontent.com/u/714210/inspector-slides-left.gif
Attachment #8797582 - Flags: feedback?(pbrosset) → feedback+
See Also: → 1307404
(In reply to Patrick Brosset <:pbro> from comment #5) > Comment on attachment 8797582 [details] > Bug 1307408 - part1: fix layout issues for small widths in inspector sidebar; > > I've tested this quickly, it looks really good. Thanks for these changes. > Yes, I do think we should land this first and then tackle other individual > problems later. > > Just one weird thing I noticed: if you drag the splitter to the left > quickly, then the markup-view may scroll within its frame. I hope it's > visible in the GIF: > https://dl.dropboxusercontent.com/u/714210/inspector-slides-left.gif Thanks for checking. I investigated the issue your GIF illustrates. It comes from the breadcrumbs widget. When they overflow, the "chevron" buttons are added, and we scrollIntoView to make sure the selected breadcrumb item is visible. Which in this case will make the whole container scroll. Not sure how to fix that just yet, but I would like to go forward and land the first batch of improvements.
Keywords: leave-open
Inspector bug triage (filter on CLIMBING SHOES).
Priority: -- → P2
Comment on attachment 8797582 [details] Bug 1307408 - part1: fix layout issues for small widths in inspector sidebar; https://reviewboard.mozilla.org/r/83258/#review83144 Looks good to me and all the issues mentioned in comment #0 are fixed for me. R+, assuming my comment gets resolved. Honza ::: devtools/client/themes/inspector.css:39 (Diff revision 2) > -.devtools-main-content { > +#inspector-main-content { > /* Subtract 1 pixel from the panel height. It's puzzling why this > is needed, but if not presented the entire Inspector panel > content jumps 1 pixel up when the Toolbox is opened. */ > height: calc(100% - 1px); > + min-width: 125px; Is this needed? I don't see any difference when disabling this (using Browser Toolbox). Honza
Attachment #8797582 - Flags: review?(odvarko) → review+
Comment on attachment 8797582 [details] Bug 1307408 - part1: fix layout issues for small widths in inspector sidebar; https://reviewboard.mozilla.org/r/83258/#review83144 Thanks for the review, see my reply below. > Is this needed? I don't see any difference when disabling this (using Browser Toolbox). > > Honza This allows to mitigate the glitch described by Patrick and discussed in the previous comments. If you reduce the splitter size quickly the scrollIntoView call will force the container to scroll and will show blank space in the panel, which really feels buggy. With this min-width, even if the container scrolls, it will not look empty. 125px is kind of an arbitrary value though, it only has to be over 100px, which is the effective min-height of the #inspector-toolbar element (which is the element that would force the overflow in the absence of a greater min-width set on the container). So overall I'd be in favor of keeping this min-width.
See my previous comment for the min-width: 125px, and above a screenshot of what happens without it (top) and with it (bottom) if you quickly resize the splitter. I can of course add a comment above the value to explain what I just said here.
Flags: needinfo?(odvarko)
(In reply to Julian Descottes [:jdescottes] from comment #11) > Comment on attachment 8797582 [details] > This allows to mitigate the glitch described by Patrick and discussed in the > previous comments. If you reduce the splitter size quickly the > scrollIntoView call will force the container to scroll and will show blank > space in the panel, which really feels buggy. With this min-width, even if > the container scrolls, it will not look empty. Ah, I see it now. > 125px is kind of an arbitrary value though, it only has to be over 100px, > which is the effective min-height of the #inspector-toolbar element (which > is the element that would force the overflow in the absence of a greater > min-width set on the container). > > So overall I'd be in favor of keeping this min-width. Agree > I can of course add a comment above the value to explain what I just said here. Yes, please! Thanks! Honza
Flags: needinfo?(odvarko)
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a38df6f51b67 part1: fix layout issues for small widths in inspector sidebar;r=Honza
Enough fixes done here that it doesn't need to block the inspector-top-bugs bug anymore. Thanks!
No longer blocks: top-inspector-bugs
Patrick I propose to close this as invalid. There's no clear small width issue in the inspector at the moment and the bug is not actionable.
Flags: needinfo?(pbrosset)
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: needinfo?(pbrosset)
Resolution: --- → INVALID
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: