[ntp4] Improve horizontal mousewheel event handling for touch devices.

[ntp4] Improve horizontal mousewheel event handling for touch devices.

BUG=95626
TEST=Swipe NTP4 pages, average speed swipes should only swipe a single page.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=106836

[csilv, 2011-10-21 23:46:20]

+estade for review

I spent the past couple days exploring how to address this issue.  While not
perfect, I believe the technique in this CL will largely address the issue.  (Or
at least, it is a good enough improvement to try out on dev/canary.)

On a touch pad, if you swipe fast enough, you can still cause an scroll an extra
page, but for average swipes this will do a single pages.  Previously, an
average swipe would scroll for 10 or more pages.

[Evan Stade, 2011-10-22 00:48:35]

http://codereview.chromium.org/8369016/diff/2001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/card_slider.js (right):

http://codereview.chromium.org/8369016/diff/2001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/card_slider.js:252: if
(this.mouseWheelCardSelected_) return;
why is this necessary? don't you want it to be possible to switch through many
cards with an aggressive swipe?

http://codereview.chromium.org/8369016/diff/2001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/card_slider.js:269: this.mouseWheelScrollAmount_
+= e.wheelDeltaX;
why do you add this just to subtract it again?

[csilv, 2011-10-22 01:24:32]

http://codereview.chromium.org/8369016/diff/2001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/card_slider.js (right):

http://codereview.chromium.org/8369016/diff/2001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/card_slider.js:252: if
(this.mouseWheelCardSelected_) return;
On 2011/10/22 00:48:35, Evan Stade wrote:
> why is this necessary? don't you want it to be possible to switch through many
> cards with an aggressive swipe?

No.  We really want a swipe to change a single page at a time.  You can see this
on Android or iPhone that use page swipe UIs... no mater how aggressive it is,
it's single page only.  If we were using touch events, this would be easy to do.
  But that's not an option.

http://codereview.chromium.org/8369016/diff/2001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/card_slider.js:269: this.mouseWheelScrollAmount_
+= e.wheelDeltaX;
On 2011/10/22 00:48:35, Evan Stade wrote:
> why do you add this just to subtract it again?

This was the original code that you wrote, but you raise a good point.  Since
this section will only be invoked for discrete devices, I can simplify this
logic quite a bit.  I'll do that.

[csilv, 2011-10-22 01:49:34]

http://codereview.chromium.org/8369016/diff/2001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/card_slider.js (right):

http://codereview.chromium.org/8369016/diff/2001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/card_slider.js:269: this.mouseWheelScrollAmount_
+= e.wheelDeltaX;
On 2011/10/22 01:24:32, csilv wrote:
> On 2011/10/22 00:48:35, Evan Stade wrote:
> > why do you add this just to subtract it again?
> 
> This was the original code that you wrote, but you raise a good point.  Since
> this section will only be invoked for discrete devices, I can simplify this
> logic quite a bit.  I'll do that.

Done.

[Evan Stade, 2011-10-22 01:57:45]

great

lgtm

http://codereview.chromium.org/8369016/diff/2001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/card_slider.js (right):

http://codereview.chromium.org/8369016/diff/2001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/card_slider.js:252: if
(this.mouseWheelCardSelected_) return;
On 2011/10/22 01:24:32, csilv wrote:
> On 2011/10/22 00:48:35, Evan Stade wrote:
> > why is this necessary? don't you want it to be possible to switch through
many
> > cards with an aggressive swipe?
> 
> No.  We really want a swipe to change a single page at a time.  You can see
this
> on Android or iPhone that use page swipe UIs... no mater how aggressive it is,
> it's single page only.  If we were using touch events, this would be easy to
do.
>   But that's not an option.

ok