Closed Bug 1493745 Opened 2 years ago Closed 2 years ago

Support toolbar on small viewports

Categories

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

defect

Tracking

(firefox67 verified, firefox68 verified, firefox69 verified)

VERIFIED FIXED
Firefox 67
Tracking Status
firefox67 --- verified
firefox68 --- verified
firefox69 --- verified

People

(Reporter: Harald, Assigned: gl)

References

(Blocks 1 open bug)

Details

(Whiteboard: [dt-q:P1])

Attachments

(6 files, 1 obsolete file)

Attached image 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.
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)
Duplicate of this bug: 1493874
Whiteboard: [dt-q]
Priority: -- → P2
Duplicate of this bug: 1497485
See Also: → 1497490
Whiteboard: [dt-q] → [dt-q:P1]
We should probably also fix bug 1493746 at the same time.
See Also: → 1493746
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.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Depends on: 1488972
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.
Assignee: pbrosset → gl
Attached patch 1493745.patchSplinter Review
I also move the device selector back to the center group as discussed in https://bugzilla.mozilla.org/show_bug.cgi?id=1488972#c5.
Attachment #9026518 - Flags: review?(pbrosset)
Attached patch responsive-rdm-toolbar.patch (obsolete) — Splinter Review
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 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.
Attachment #9026518 - Flags: review?(pbrosset) → review+
Keywords: leave-open
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/18d939404b31
Support RDM toolbar on small viewports. r=pbro
No longer depends on: 1488972
Blocks: rdm-ux
Attachment #9026631 - Attachment is obsolete: true
Attached image 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.

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.
Attached image rdm-toolbar-2.gif

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

Attachment #9046727 - Flags: feedback?(mbalfanz)

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 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)
Attachment #9046727 - Flags: feedback?(mbalfanz) → feedback+
Keywords: leave-open
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/81de709aaadd
Make the RDM toolbar responsive; r=gl
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
QA Whiteboard: [qa-67b-p2]
See Also: → 1545257

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

Status: RESOLVED → VERIFIED

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

Yes this issue is Verified as Fixed in 68.0b7.

You need to log in before you can comment on or make changes to this bug.