1138815 - Fine-tune the swipe behavior of swipeable panel in keyboard

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Keyboard, defect)

Description

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

+++ This bug was initially created as a clone of Bug #1100779 +++

We implemented swipeable page view in Bug 1100779, and this bug is to fine tune the swipe behavior so that it would depend on velocity and distance to decide if it needs to swipe to the next page or not, per UX review.

Comment 1

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Attachment: patch V1

This patch attempts to address the UI review comments, now it should depend on both velocity and distance to determine whether to go to the next (previous) section of emoji.

Omega, could you please help review this behavior again?
Thanks.

Comment 2

[Omega Feng [:Omega] [:馮於懋]]

Comment on attachment 8575854 [details] [review]
patch V1

It works better now. However, I found another issue: Use two fingers to drag the panel, and you can find that the panel is continuously panning, not by page. We should treat two finger as one finger behavior.

Comment 3

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Comment on attachment 8575854 [details] [review]
patch V1

Patch updated to address the 2-finger issue you mentioned.

Omega, please help give it a try.
Thanks.

Comment 4

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Comment on attachment 8575854 [details] [review]
patch V1

Hi Tim,

It took a while for me to fine-tune the behavior of the swiping, and now after Omega's agreement, this should be good enough.

Could you please give some early feedback on the current approach?
Will work on the tests if this looks good to you.

Thanks.

Comment 5

[Omega Feng [:Omega] [:馮於懋]]

Comment on attachment 8575854 [details] [review]
patch V1

Better now, thanks! :)

Comment 6

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Comment on attachment 8575854 [details] [review]
patch V1

The patch has been updated to add some tests and address the review comment.
Tim, could you please help review it again?

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8575854 [details] [review]
patch V1

I am having problems with touchstart event listener.

Comment 8

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Comment on attachment 8575854 [details] [review]
patch V1

Thanks for the advice.
Patch updated to move the event listening to swiping_detector.

Please note that SwipeablePanelView still needs to keep the start point of the panning as this.startX since it might change it when the user overscrolls, as the following line does,
https://github.com/mozilla-b2g/gaia/pull/28792/files#diff-569ecb8fbc0b7340a3490cf87434e9b1R150

Comment 9

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8575854 [details] [review]
patch V1

I don't like the way _handlePen is organized also the startX and deltaX thing, but I will leave it for you to figure out.

Comment 10

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

I am not sure how to address comment 9 for now, so I'll just land this first to address this UX issue and then come back to this later.
Thanks for the suggestion.

Comment 11

[GitHub Autolander Bot]

http://docs.taskcluster.net/tools/task-graph-inspector/#HT0yyqwfSaCTTq0u4wlhrA

The pull request failed to pass integration tests. It could not be landed, please try again.

Comment 12

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/493754a7ef39ca63d5aebee6bf5c6a4dd0913f9b