Address comments and our conversation. I'd like to submit this CL as soon as possible and add handling for orientation change next week. I'll add a OrientationEvent support to GWT and use that to catch the orientation change.
http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/Touch.gwt.xml File user/src/com/google/gwt/touch/Touch.gwt.xml (right): http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/Touch.gwt.xml#newcode25 user/src/com/google/gwt/touch/Touch.gwt.xml:25: <when-property-is name="user.agent" value="safari" /> On 2011/03/02 03:27:33, pdr wrote:
FF4 and IE9 may support touch events for tablets/touch devices. Could
you check
for the other browsers as well (FF3.5, etc.)?
It may be best to set the default to "maybe", and set the support of
IE6 and
other known-not-to-support browsers to "no" so that a user who has
defined a
permutation for "mobile-webkit" will get "maybe" as expected.
Done. Agreed. Its likely that all mobile browsers will eventually add support for touch events. http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/DefaultMomentum.java File user/src/com/google/gwt/touch/client/DefaultMomentum.java (right): http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/DefaultMomentum.java#newcode39 user/src/com/google/gwt/touch/client/DefaultMomentum.java:39: public Snapshot updateSnapshot(Snapshot snapshot) { The idea is that a subclass of Momentum might want to create its own subclass of Snapshot that contains more information about the previous state. Per our conversation, I'll add a Momentum#createState() method so the user can initialize the state to any subclass they want to use. That allows them to assume the State type in updateState() without an instanceof check. http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/Momentum.java File user/src/com/google/gwt/touch/client/Momentum.java (right): http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/Momentum.java#newcode26 user/src/com/google/gwt/touch/client/Momentum.java:26: * A snapshot of the current state. On 2011/03/02 03:27:33, pdr wrote:
I think users will find this name confusing.
Could State work? Or MomentumState?
Done. State works for me. http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/Momentum.java#newcode153 user/src/com/google/gwt/touch/client/Momentum.java:153: * This method can modify the existing snapshot by called On 2011/03/02 03:27:33, pdr wrote:
called -> calling
Done. http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/Point.java File user/src/com/google/gwt/touch/client/Point.java (right): http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/Point.java#newcode19 user/src/com/google/gwt/touch/client/Point.java:19: * A simple point class. I thought about that, but I wouldn't know where to add it, and it'll get lonely in a package by itself. When we're ready to add a graphics/vector library, we can add it. In the meantime, its not uncommon to create a generic class like this in a package where its needed. Added mult and div. I also refactored DefaultMomentum to use a common method for calculating the change in momentum of x and y, instead of coding the same calculation twice. http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/Point.java#newcode24 user/src/com/google/gwt/touch/client/Point.java:24: private final double y; I tried making it mutable, but there wasn't any difference in performance. Making them mutable makes it too easy to accidentally modify the wrong value. http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/TouchScroller.java File user/src/com/google/gwt/touch/client/TouchScroller.java (right): http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/TouchScroller.java#newcode44 user/src/com/google/gwt/touch/client/TouchScroller.java:44: * Adds touch based scrolling to a scroll panel. Dragging just keeps the view under the finger, but its susceptible to finger movement and touch events. We can do another pass to smooth it out using a timer, but for now I think its good enough and there are more important things to work on. http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/TouchScroller.java#newcode90 user/src/com/google/gwt/touch/client/TouchScroller.java:90: schedule((int) MS_PER_FRAME); schuduleFixedDelay will work. The problem with scheduleRepeating is that if the momentum takes a long time to calculate (because of other stuff pegging the CPU), you end up with 1ms delays that make the app feel unresponsive. http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/TouchScroller.java#newcode181 user/src/com/google/gwt/touch/client/TouchScroller.java:181: * Runtime check for whether the canvas element is supported in this browser. On 2011/03/02 03:27:33, pdr wrote:
canvas -> scrollable widget
Done. http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/TouchScroller.java#newcode183 user/src/com/google/gwt/touch/client/TouchScroller.java:183: * @return whether the canvas element is supported On 2011/03/02 03:27:33, pdr wrote:
canvas -> scrollable widget
Done. http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/TouchScroller.java#newcode254 user/src/com/google/gwt/touch/client/TouchScroller.java:254: * If the gesture takes too long, we update the recentTuochPosition to the On 2011/03/02 03:27:33, pdr wrote:
recentTuochPosition -> recentTouchPosition
Done. http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/TouchScroller.java#newcode256 user/src/com/google/gwt/touch/client/TouchScroller.java:256: * do this so that we don't based the velocity on two touch events that On 2011/03/02 03:27:33, pdr wrote:
based -> base
Done. http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/TouchScroller.java#newcode537 user/src/com/google/gwt/touch/client/TouchScroller.java:537: } else if (trackingTime > MAX_TRACKING_TIME / 2) { We calculate velocity based on the distance that the finger traveled. However, its possible for users to scroll, pause, and change direction, so every 200ms we move our starting point. The problem is, if we just move to the current point and the user immediately stops scrolling, then we would base the velocity on two points that are very close in time (or the same time). So, we put a point on deck halfway to timing out. 1/2 makes logical sense because anything less and we'd have to slide more frequently, and anything more and the points would still be too close. That being said, there is a bug in the code. We actually keep moving the onDeck time until tracking time expires. I also moved the calculation to a final field. http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/touch/client/TouchScroller.java#newcode558 user/src/com/google/gwt/touch/client/TouchScroller.java:558: */ On 2011/03/02 03:27:33, pdr wrote:
Why is this additional handler for click events necessary?
If the screen is scrolling and the user touches the screen, they want to freeze the scroll momentum, not select an item.
Could it be replaced with a check in onTouchStart() checking if the
velocity is
not zero?
Isn't that basically what it is? http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/user/User.gwt.xml File user/src/com/google/gwt/user/User.gwt.xml (right): http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/user/User.gwt.xml#newcode31 user/src/com/google/gwt/user/User.gwt.xml:31: <inherits name="com.google.gwt.touch.Touch"/> On 2011/03/02 03:27:33, pdr wrote:
Nit: not alphabetical order.
Done. http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/user/client/ui/ScrollPanel.java File user/src/com/google/gwt/user/client/ui/ScrollPanel.java (right): http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/src/com/google/gwt/user/client/ui/ScrollPanel.java#newcode82 user/src/com/google/gwt/user/client/ui/ScrollPanel.java:82: private native boolean isRtl(Element scrollable) /*-{ The user could change it at any time, so there isn't a good time to cache it. At least its only in the FF impl, which scrolls in the negative direction in RTL. That being said, code that calls getMinimum/MaximumHorizontalScrollPosition could reasonably cache the result for the duration of an animation. http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/test/com/google/gwt/touch/client/TouchScrollTest.java File user/test/com/google/gwt/touch/client/TouchScrollTest.java (right): http://gwt-code-reviews.appspot.com/1370801/diff/1016/user/test/com/google/gwt/touch/client/TouchScrollTest.java#newcode31 user/test/com/google/gwt/touch/client/TouchScrollTest.java:31: On 2011/03/02 03:27:33, pdr wrote:
Can you add a test or two for actions during the coasting/momentum
scroll? For
instance, that touching during a coast stops the scrolling.
Added a bunch of tests, including this use case.
Can you add a test that things work fine when a resize occurs during
the
coasting/momentum scroll? For instance, changing the orientation of a
mobile
device could change the panel's dimensions. Will everything still stop
where it
should?
Per our conversation, we'll stop momentum on orientation change. But for now, can I put a TODO in if I promise to add an OrientationEvent next week? http://gwt-code-reviews.appspot.com/1370801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
