1303732 - Inspector's width is too small when dragging the splitter to the right

Status: VERIFIED FIXED

(DevTools :: Shared Components, defect, P1)

Description

[Petruta Horea [:phorea], Desktop QA]

[Note]:
- This only reproduces with 2016-09-17 Nightly, the patch causing it was backed-out for test failures. 

[Affected versions]:
- Nightly 51.0a1 2016-09-17

[Affected platforms]:
- Win 10 64-bit
- Mac OS X 10.11

[Steps to reproduce]:
1. Open the Inspector
2. Drag the splitter to the right

[Expected result]:
- The Inspector's right side has the same minimum width as before bug 1260552 landed

[Actual result]:
- The width of the Inspector's right side becomes very small so that only a part of "Rules" is visible

[Regression range]:
- Regressed by bug 1260552 which is currently backed-out from Nightly

Comment 1

[Julian Descottes [:jdescottes]]

bug 1260552 did not land in Firefox 51, updating affected versions.

Comment 2

[Julian Descottes [:jdescottes]]

I think we swapped the min-width for the markup view and the sidebar panel:
- min-width for markup-view was 50px and is now 275px
- min-width for side panel was 300px and is now 50px

Helen: We could simply restore the previous values, but I remember discussions on #devtools regarding our minimum dimensions and letting users resize panels as they want (even if it means making some panels unusable when resized to very small dimensions). What do you think we should do here? Restore the previous values, or settle on smaller min-width (50px for both?)

Comment 3

[Helen V. Holmes (:helenvholmes) (:✨)]

(In reply to Julian Descottes [:jdescottes] from comment #2)
> I think we swapped the min-width for the markup view and the sidebar panel:
> - min-width for markup-view was 50px and is now 275px
> - min-width for side panel was 300px and is now 50px
> 
> Helen: We could simply restore the previous values, but I remember
> discussions on #devtools regarding our minimum dimensions and letting users
> resize panels as they want (even if it means making some panels unusable
> when resized to very small dimensions). What do you think we should do here?
> Restore the previous values, or settle on smaller min-width (50px for both?)

I'd be fine with a smaller min-widths for both, although I think that at that small a size it would be a nice enhancement to snap close the sidebar panel. (I assume if you're making it that small you just want it out of the way.)

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1303732 - Set min-height/width to 50px for markupview and sidebar panel;

Review commit: https://reviewboard.mozilla.org/r/81738/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81738/

Comment 5

[Julian Descottes [:jdescottes]]

(In reply to Helen V. Holmes (:helenvholmes) (:✨) from comment #3)
> (In reply to Julian Descottes [:jdescottes] from comment #2)
> > I think we swapped the min-width for the markup view and the sidebar panel:
> > - min-width for markup-view was 50px and is now 275px
> > - min-width for side panel was 300px and is now 50px
> > 
> > Helen: We could simply restore the previous values, but I remember
> > discussions on #devtools regarding our minimum dimensions and letting users
> > resize panels as they want (even if it means making some panels unusable
> > when resized to very small dimensions). What do you think we should do here?
> > Restore the previous values, or settle on smaller min-width (50px for both?)
> 
> I'd be fine with a smaller min-widths for both, although I think that at
> that small a size it would be a nice enhancement to snap close the sidebar
> panel. (I assume if you're making it that small you just want it out of the
> way.)

Thanks for the feedback! Let's just restore the previous values for now and discuss the enhancement in Bug 1306064.
The patch attached here also addresses Bug 1305051, since it's just about an additional min-height.

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8795794 [details]
Bug 1303732 - Set  min-height/width to 50px for markupview and sidebar panel;

https://reviewboard.mozilla.org/r/81738/#review80664

It would be great if it's possible to make the sidebar small (like 50px), but I understand taht this will be done in Bug 1306064 so, R+

Thanks for the patch!

Honza

Comment 7

[Julian Descottes [:jdescottes]]

Comment on attachment 8795794 [details]
Bug 1303732 - Set  min-height/width to 50px for markupview and sidebar panel;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81738/diff/1-2/

Comment 8

[Julian Descottes [:jdescottes]]

Comment on attachment 8795794 [details]
Bug 1303732 - Set  min-height/width to 50px for markupview and sidebar panel;

Re-flagging for review. I'm now using 50px everywhere as we discussed.

Using 275px as a min dimension for the markup view was creating a different regression: the markup view was too big in vertical layout (its min height used to be 35vh).

As we all agree that smaller min dimensions are better in the end, let's make the change here. 

Bug 1306064 will be dedicated to implement the improvement suggested by Helen, (collapse the panel if it reaches its minimum dimension).

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8795794 [details]
Bug 1303732 - Set  min-height/width to 50px for markupview and sidebar panel;

https://reviewboard.mozilla.org/r/81738/#review80702

Great!

Honza

Comment 10

[Julian Descottes [:jdescottes]]

Thanks for the review, Honza! 
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2f0d0a49d04a

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/846e44451176
Set  min-height/width to 50px for markupview and sidebar panel;r=Honza

Comment 13

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/846e44451176

Comment 14

[Julian Descottes [:jdescottes]]

After discussing with Helen and Honza (comment #3 and comment #6) we agree to go for smaller minimum widths here.
This means: 
- the minimum width for the right panel remained the same here (50px)
- the minimum width for the left panel was decreased from 275px to 50px
- the minimum height (when in portrait mode) has been declared to 50px (no value before)

Comment 15

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

Thank you for the clarification! I will mark it as verified as the width match the values.

Cheers!