Closed Bug 1243691 Opened 8 years ago Closed 8 years ago

Tool itself probably shouldn't allow scrolling vertically at all, the viewport should scroll instead

Categories

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

defect

Tracking

(firefox47 verified)

VERIFIED FIXED
Firefox 47
Iteration:
47.1 - Feb 8
Tracking Status
firefox47 --- verified

People

(Reporter: pbro, Assigned: pbro)

References

Details

(Whiteboard: [multiviewport] [mvp-rdm])

Attachments

(2 files)

As a user, I'm only interested in scrolling the page I'm inspecting/debugging in the RDM tool.
If I make my browser window smaller and as a result scrollbars appear in the RDM tool, then it becomes harder for me to only scroll my test page.

I believe that whatever the dimensions of the browser window, the RDM tool should always manage to center itself nicely.

As long as we only support 1 viewport, that viewport should always be centered and both horizontal and vertical scrollbars on the RDM tool should be hidden.
Flags: qe-verify+
This also means that as the browser window starts getting smaller and smaller (in height), at some stage the viewport should stop being centered and be top aligned instead, so that the top toolbar remains always visible.
(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #0)
> As a user, I'm only interested in scrolling the page I'm
> inspecting/debugging in the RDM tool.
> If I make my browser window smaller and as a result scrollbars appear in the
> RDM tool, then it becomes harder for me to only scroll my test page.
> 
> I believe that whatever the dimensions of the browser window, the RDM tool
> should always manage to center itself nicely.
> 
> As long as we only support 1 viewport, that viewport should always be
> centered and both horizontal and vertical scrollbars on the RDM tool should
> be hidden.

Right, I think this makes sense to me for now, at least for 1 viewport.  With multiple, we need a different approach.

There is also a "fit" option on file (bug 1213951) that would zoom out the viewport enough to keep it all inside the window.
Summary: The RDM xhtml page probably shouldn't allow scrolling vertically at all, the viewport should scroll instead → Tool itself probably shouldn't allow scrolling vertically at all, the viewport should scroll instead
Sorry, I meant to say "With multiple, we **may** need a different approach."  I can imagine easily having more viewports than you can fit at once, at least with the laptop screen.

Anyway, it seems good to make a change here for single viewport.
(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #0)
> As a user, I'm only interested in scrolling the page I'm
> inspecting/debugging in the RDM tool.
I agree! I think this is a good approach for now.

I think in the future we may need to think about fit/full display modes (I'm not sure I'm getting those two terms right), in which case scrolling gets more trickly. I'll bring this up in today's standup.
Priority: -- → P1
Whiteboard: [multiviewport] [triage] → [multiviewport]
Going to take a chance at this. I've been working on a layout that removes the vertical scrollbar, displays a horizontal scrollbar when needed, centers viewports and makes sure they stick at the top when the screen becomes too small.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Attached image rdm.gif
Screen recording that shows how the layout behave with 1 or more viewports, and when resizing the parent window.
Comment on attachment 8714300 [details]
MozReview Request: Bug 1243691 - Tweak the general RDM layout to allow multiple viewports, center them and snap to top/left when needed; r=jryans

https://reviewboard.mozilla.org/r/33001/#review29799

Great, this seems to do everything we need!  Works quite well locally.

::: devtools/client/responsive.html/index.css:25
(Diff revision 1)
> +  position: sticky;

Cool, seems like a good use case for sticky.
Attachment #8714300 - Flags: review?(jryans) → review+
Keywords: checkin-needed
Forgot to push to try. Will ask for checkin later.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=941e1e3b05e0
Keywords: checkin-needed
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5cf12f457103
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Iteration: --- → 47.1 - Feb 8
QA Contact: mihai.boldan
I confirm that the RDM tool is always centered and that only a horizontally scrollbar is displayed when needed. 
Also, the viewports can be scrolled individual vertically.  
Verified with 47.0a1 (2016-02-17) on Windows 10x86, Mac OS X 10.9.5 and on Ubuntu 14.04 x86.
I am marking this issue Verified-Fixed.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Whiteboard: [multiviewport] → [multiviewport] [mvp-rdm]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: