1319619 - Move dimension controls above the viewport

Status: RESOLVED FIXED

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

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Several people[1][2] have requested the dimension UI be moved (either by default or with an option) above the viewport instead of below.

At the moment, the bottom of the viewport might be off screen if it's tall, so that make the dimensions hard to reach.

Additionally, once we have multiple viewports, it could be preferable to have dimensions on the top so there are at a consistent vertical position for all viewports.

[1]: https://hacks.mozilla.org/2016/11/new-responsive-design-mode-rdm-lands-in-firefox-dev-tools/comment-page-1/#comment-20351
[2]: https://twitter.com/rybrad/status/801112188521881600

Comment 1

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

Attachment: 1319619.patch

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8813495 [details] [diff] [review]
1319619.patch

I don't think we know yet what we want here.  I am just trying to capture feedback so far.  We still need more discussion with UX (Helen) about this first.

Even if we agree it should move, we may want a different UI appearance that meshes with other toolbars, so I don't think the implementation is known yet.

Thanks for jumping on it, though!  I'll bring it up for triage at the next RDM meeting.

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: RDM dimensions at the top

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8813495 [details] [diff] [review]
1319619.patch

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

We discussed this during our triage today.  In the long term, this concern might go away, since we'll likely be changing up the look of things a bit to accommodate multiple viewports.  Overall though, it seems like there are several valid reasons to want the dimensions up top, including:

* Have to scroll to reach dimensions with a tall viewport
* Hard to compare to other properties like DPR since they're on opposite sides

So, I think we're open to short term fix that moves the controls above the viewport somewhere.  I think the main question is where.

Helen, where do you think would make sense above the viewport?  :gl's current patch moves them just above the viewport (see attachment 8815486 [details]).  Would you prefer it goes into one of the toolbars instead?  (I can see arguments for having it near the device and also for near the DPR...)

Comment 5

[Alfred Kayser]

There are now three places where the controls of RDM live: the toolbar on top, the toolbar in the viewport and beneath the viewport. One should place them all together in the toolbar.

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Alfred Kayser from comment #5)
> There are now three places where the controls of RDM live: the toolbar on
> top, the toolbar in the viewport and beneath the viewport. One should place
> them all together in the toolbar.

Yes, there are many toolbars.  We expect to improve this in future.  The current design is a bit too rigid with some things inside the viewport's width, which can get very small.

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I think this is a good short term fix to alleviate some valid UX issues, so let's move forward here.

We'll keep iterating on the toolbars in the future, but in the mean time, it seems good to take this simple fix.

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8813495 [details] [diff] [review]
1319619.patch

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

Thanks for working on it!  I'd like to try to uplift to 52 as well, if we can.

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

:gl, is a try run needed, or is this ready to land?

Comment 10

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f05c5565a62ba69a0b20484229867009fd1c2767

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/886a579b22cf
Move RDM dimension controls above the viewport. r=jryans

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/886a579b22cf

Comment 13

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8813495 [details] [diff] [review]
1319619.patch

Approval Request Comment
[Feature/Bug causing the regression]: New RDM was first enabled in 52, many users mentioned they'd prefer the controls up top during release post
[User impact if declined]: Controls will be at the bottom, which can be hard to find for large viewports (scrolling needed)
[Is this code covered by automated tests?]: Yes, there are tests, though it's a visual placement change only
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No, only moving elements
[Why is the change risky/not risky?]: N/A
[String changes made/needed]: None

Comment 14

[Julien Cristau [:jcristau]]

Comment on attachment 8813495 [details] [diff] [review]
1319619.patch

devtools RDM tweak, aurora52+

Comment 15

[Julien Cristau [:jcristau]]

Comment on attachment 8813495 [details] [diff] [review]
1319619.patch

devtools RDM tweak, aurora52+

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2e770075ae14