Closed Bug 517912 Opened 13 years ago Closed 3 years ago

Add resistance to horizontal panning to keep sidebars from popping out

Categories

(Firefox for Android Graveyard :: Panning/Zooming, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: bcombee, Unassigned)

Details

Attachments

(2 files, 2 obsolete files)

No description provided.
Summary: Add → Add resistance to horizontal panning to keep sidebars from popping out
The idea is to add a non-visible "space" when moving left/right on the screen between hitting the edge of the content and having the sidebar appear.  This will speed up scrolling because keeping the sidebar on screen slows down the update process.  It also makes it easier to get to the left/right edge of content.
Flags: wanted-fennec1.0?
tracking-fennec: --- → ?
tracking-fennec: ? → 1.0-wm+
Flags: wanted-fennec1.0?
tracking-fennec: 1.0-wm+ → 1.0b1-wm+
Assignee: nobody → combee
Status: NEW → ASSIGNED
Attachment #402178 - Flags: review?(mark.finkle)
Attachment #402178 - Flags: review?(webapps)
I also renamed "rite" to "right" and added some comments from talking with NewBen.
The 100pix value probably needs to be updated to be DPI dependent for device.
Comment on attachment 402178 [details] [diff] [review]
Add 100pix of resistance to opening sidebars, also change snapping of sidebars to favor direction of movement

The code looks good to me.  Needs to be tested on mobile device but resistance feels about right on my computer.  Snapping behavior seems to work nicely.

My only nit: if I eat up my sidebar resistance and go to left sidebar but pan away from it (all in one drag), I expect the resistance to be back when I try to go over to sidebar again.

Also now that we have this should we tweak horizontal/vertical drag locking?  It feels a little less important now.

r+'ing because ultimately this is a step forward and I'd love to see feedback for this in b4.
Attachment #402178 - Flags: review?(webapps) → review+
Drag locking is still important when panning through a column but zoomed in so you're not near the sidebars.  I'd really like to try this out on a device before it goes into b4.
this doesn't disable drag locking, it's just on top
Comment on attachment 402178 [details] [diff] [review]
Add 100pix of resistance to opening sidebars, also change snapping of sidebars to favor direction of movement

r- after playing with on device.  When zoomed in the resistance doesn't seem to be working correctly.

Also, to make panning to sidebars a more willful action, we should ignore any kinetic energy when eating the barrier up.
Attachment #402178 - Flags: review+ → review-
(In reply to comment #8)

> Also, to make panning to sidebars a more willful action, we should ignore any
> kinetic energy when eating the barrier up.

I'm not entirely sure I agree -- I think that a really hard swipe ought to be able to get you there.  And we want to be showing a little bit of the side bar on a swipe over to the edge, even if it's not enough to cause it to snap out.  It's one of the ways we expect people to discover the side areas, and people generally like page-edge-bounce.

That said, I haven't played with it on the device yet.  Will do so!
The problem with showing even a little of the sidebar is that it really slows down vertical scrolling.  That's the big reason for the change in behavior.
On desktop, I was always getting resistance even when zoomed in
Attachment #402178 - Attachment is obsolete: true
Attachment #404697 - Flags: review?(webapps)
Attachment #402178 - Flags: review?(mark.finkle)
Comment on attachment 404697 [details] [diff] [review]
Updated to not let kinetic motion affect the side bars

Not a big fan of pushing the kinetic state through the system:

dragMove(dx, dy, scroller, isKinetic)


Since InputHandler or MouseModule knows the kinetic state, and it's sort of a singleton situation, we could add a isKineticActive getter on InputHandler or MouseModule and skip passing the param everywhere.
Comment on attachment 404697 [details] [diff] [review]
Updated to not let kinetic motion affect the side bars

>+  /** Precalculate how much of the pan movement should be reserved to
>+   * move away sidebars, and take away that horizontal movement from
>+   * the content containers. */

Thanks, this is a much better comment.  Nit on formatting: one-line javadoc style is /** content */, but if comment is more than one line /** gets its own line.

As discussed on IRC, I have some concerns that adding resistance will cause users to feel Fennec is sluggish.  We are trying to solve two things with this patch:
1) Panning omnidirectionally can be slow when sidebars are shown
2) Don't show sidebars when user doesn't want to see them

I believe part of #1 is caused by setting urlbar to position:fixed, creating lag for a second or two when sidebar is first shown.  This is fixed by not showing the urlbar so readily if a little bit of the sidebar is poking out.  I think #2 happens when a user swipes to get to the edges of the page and KE causes the sidebar to show.  This is fixed by removing KE for sidebar panning.  #2 also happens when user wants to vertically pan but gets horizontal panning too.  Maybe we could have a threshold before we start panning and decide which axis is locked.

I will file two bugs.
Attachment #404697 - Flags: review?(webapps) → review-
Moving to Vivian in preparation for my departure
Assignee: combee → 21
Component: General → Panning/Zooming
Attached patch WIP 1 (obsolete) — Splinter Review
I'm starting work on this with a similar approach to bcombee's original patch - I want to make toolbars snap shut *while* panning through a small region around the edge of the screen (compared to now, where they snap shut only after panning stops).

The attached patch is not complete, but it gets part of the effect right.
Assignee: 21 → mbrubeck
Attached patch WIP 2Splinter Review
Attachment #444213 - Attachment is obsolete: true
Comment on attachment 444334 [details] [diff] [review]
WIP 2

Okay, this is now working well enough to be worth reviewing.  It's a very simple patch that adds 40 pixels of resistance at the page edge; as you pan either direction through that 40 pixel margin, the sidebar snaps hidden.  It looks and feels good to me on the N900.

Unlike bcombee's revised patch above, this does not make any special changes for kinetic panning.  (Kinetic panning can still reveal the sidebar just like before, though it will now encounter resistance just like non-kinetic panning.)

The goal of this patch is to make it easier to pan without accidentally opening the sidebars so frequently.  It's not a complete solution but I think it's a useful step.  I also want to revisit bug 521108 (Improve pan axis locking) and try to reduce the situations where axis locking gets "unlocked".
Attachment #444334 - Flags: ui-review?(madhava)
Attachment #444334 - Flags: review?(webapps)
Comment on attachment 444334 [details] [diff] [review]
WIP 2

The code looks fine.

I played around with this on the phone.  Flicks to the right or left felt great, and up and down panning never accidentally brought up the side bars.

When moving my finger around in a circle, things felt clunky on the N900.  The magnetic attraction felt more like Fennec couldn't keep up with my finger.
Attachment #444334 - Flags: review?(webapps)
Attachment #444334 - Flags: ui-review?(madhava)
tracking-fennec: 1.0b1-wm+ → ---
blocking2.0: --- → ?
blocking2.0: ? → ---
tracking-fennec: --- → ?
tracking-fennec: ? → 2.0-
tracking-fennec: 2.0- → ---
Not currently working on this.  This could still be improved, but I think the changes from bug 607657 and others have made it a lot better.
Assignee: mbrubeck → nobody
Status: ASSIGNED → NEW
Closing all opened bug in a graveyard component
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.