1493745 - Support toolbar on small viewports

Status: VERIFIED FIXED

(DevTools :: Responsive Design Mode, defect, P2)

Description

[:Harald Kirschner :digitarald]

Attachment: Screen Shot.PNG

… or: Responsive design mode needs some more responsive design.

STR:
- Enable RDM, switch to a nice mobile view
- Toolbox right side docked
- Make 3-panel inspector a good side

ER: All RDM toolbar options remain obvious and usable
AR: RDM toolbar mostly unusable.

Toolbars for Network was redesigned to flow to 2 lines on small viewports, which probably needs to be applied here as well.

Comment 1

[Martin Balfanz [:mbalfanz]]

I think this is mainly introduced through the UA input, which will not be enabled by default outside of Nightly.

Here's what I think we could do:

1. All dropdowns could resize (Device, DPR, Network)
2. Resizing elements should have a min-width (I tried 40px, that looks about right on normal UI scale)
3. UA could resize to smaller width as well, and at some point go on a second line (as this is optional, I think it makes most sense here than for any of the other controls)

Comment 4

[Patrick Brosset <:pbro>]

We should probably also fix bug 1493746 at the same time.

Comment 5

[Patrick Brosset <:pbro>]

I've taken a quick look at it and have an idea of how to fix this. So I'm going to take this bug for now. If there's a chance I can't finish it, I'll attach whatever WIP patch I have and will unassign myself.

Comment 6

[Gabriel Luong [:gl] (ΦωΦ)]

Hi Patrick, we spoke about this a month ago and I assume you're busy as we have discussed in our 1:1. So, I am gonna take this bug back assuming you don't have time to work on this.

Comment 7

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: 1493745.patch

I also move the device selector back to the center group as discussed in https://bugzilla.mozilla.org/show_bug.cgi?id=1488972#c5.

Comment 8

[Patrick Brosset <:pbro>]

Attachment: responsive-rdm-toolbar.patch

Thanks Gabriel for taking this from me, I indeed do not have time to pursue this any further right now. Sorry for not unassigning myself sooner.
Here's a work-in-progress patch showing what I was doing when I first looked at this bug. Maybe there are some ideas in here you want to salvage. Do whatever you want with it.
I'll download your patch and do the review today.

Comment 9

[Patrick Brosset <:pbro>]

Comment on attachment 9026518 [details] [diff] [review]
1493745.patch

Review of attachment 9026518 [details] [diff] [review]:
-----------------------------------------------------------------

This change looks fine, so R+ for this, but I don't think this is enough to address Harald's description of the problem (or Martin's suggestions in comment 1).
The difference I see now with/without the patch is that the device drop-down now doesn't disappear entirely anymore, which is good. But if you make the window small enough, things in the toolbar will still overflow past the viewport, especially if you have the UA field visible.
I think we should put the UA field in its own grid column, and drop it down to a second row if the width is getting too small. That would be a good first enhancement. Second, we should make all drop-down shrink as the window is getting smaller if possible. One thing that should never happen is the close/settings/screenshot icons should never disappear beyond the viewport. And as of now, they still do.

So, again, R+ for this code change since it looks ok, but please either leave-open the bug or file another one to address those other issues.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/18d939404b31
Support RDM toolbar on small viewports. r=pbro

Comment 11

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/18d939404b31

Comment 12

[Patrick Brosset <:pbro>]

Attachment: Bug 1493745 - Make the RDM toolbar responsive; r?gl!

Comment 13

[Patrick Brosset <:pbro>]

Attachment: rdm-toolbar.gif

I've tried my luck at making the situation a bit better. I've just uploaded a patch for review.
The idea here is that everything should squish until there isn't enough space, and then, the UA label should drop to a second line and things should be able to squish some more.
I've tested this with and without the UA label, and with left and centered toolbar.
Hopefully it works fine.

Comment 14

[Martin Balfanz [:mbalfanz]]

Thanks Patrick, that looks like a good start.

Looking at the gif, I'd like to suggest 2 adjustments:

1. when everything is in 1 line, I think the size of the UA input field should shrink rather than (or at the same rate as) the dropdowns for DPR or throttling.
2. once the UA input jumps on the second line, I think it would be great if it extends to the right edge of the toolbar, removing the empty space that's currently there.

Comment 15

[Patrick Brosset <:pbro>]

Attachment: rdm-toolbar-2.gif

Thanks Martin. Here's an updated GIF that shows how the toolbar now responds to window size changes.

Comment 16

[Patrick Brosset <:pbro>]

It's a bit hard to see in the GIF because of the reduced frame rate. I'm updating the patch now, maybe you want to apply that locally and compare to the current RDM toolbar.

Comment 17

[Martin Balfanz [:mbalfanz]]

Comment on attachment 9046727 [details]
rdm-toolbar-2.gif

Thanks Patrick!

Overall I'm very happy with the result. There are small details that we could look into:

- The dropdown arrows dissappear when the fields get smaller. I think they should be always visible
- When the viewport is aligned to center, and the UA input breaks on the second row, UA and "Responsive" are not aligned the same way. (see gif attached)
- The top row elements change their size when the UA input field is toggled on/off. I think that shouldn't happen. (see gif attached)

Comment 18

[Martin Balfanz [:mbalfanz]]

Attachment: Kapture 2019-02-26 at 17.27.14.gif

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/81de709aaadd
Make the RDM toolbar responsive; r=gl

Comment 20

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/81de709aaadd

Comment 21

[Rares Doghi, Desktop QA]

This issue is Verified as Fixed In all of our latest versions of Firefox, I will mark this issue accordingly.

Comment 22

[BugBot [:suhaib / :marco/ :calixte]]

Since it has been verified for nightly and release, is it verified for beta too?
For more information, please visit auto_nag documentation.

Comment 23

[Rares Doghi, Desktop QA]

Yes this issue is Verified as Fixed in 68.0b7.