#13828 closed defect (fixed)
[CLA][PATCH] Refine dojo/gesture and dojo/gesture/*
| Reported by: | Evan | Owned by: | Evan |
|---|---|---|---|
| Priority: | high | Milestone: | 1.7 |
| Component: | Events | Version: | 1.7.0b1 |
| Keywords: | Cc: | Bryan Forbes, Kris Zyp, bill, Chris Mitchell, Kenneth G. Franqueiro | |
| Blocked By: | Blocking: |
Description
dojo/gesture and dojo/gesture/* need to be refined based on folks' reviewing comments:
- Remove the registry
- Resolve outstanding issues regarding swipe gesture
- Use on.emit() to fire and bubble gesture events
- Write reference guide documentation
Attachments (1)
Change History (31)
comment:1 Changed 9 years ago by
| Owner: | changed from Kris Zyp to Evan |
|---|
comment:2 Changed 9 years ago by
| Summary: | Refine dojo/gesture and dojo/gesture/* → [CLA][PATCH] Refine dojo/gesture and dojo/gesture/* |
|---|
Changed 9 years ago by
| Attachment: | #13828.patch added |
|---|
Initially finished version, still wrapping up tests/doh
comment:3 Changed 9 years ago by
| Cc: | Chris Mitchell added |
|---|
Hi Guys,
Would you take a quick look and let me know your thoughts regarding the changes? Mainly include:
1.Registry is removed, and moved dojo.gesture content to dojo/gesture/Base as a parental class.
2.Changed swipe to keep firing events during touch move, the previous version is actually a flicker gesture(we may added later)
3.Used on.emit() to fire and bubble gesture events
4.Added dojo.touch.cancel handle
BTW, the patch code is also uploaded at https://github.com/evanhw/touch/tree/master/dojo
Thx!
comment:4 follow-up: 5 Changed 9 years ago by
I checked it over a bit. Questions:
- Not sure what "defaultEvent" or "subEvents" attributes in subclasses of gesture/Base is for? Maybe I'm missing something, but they don't seem necessary. Likewise with init(), _events and _remove(). I'm just not seeing why you need an infrastructure more complicated than dojo/touch uses.
- You are firing synthetic DOM events as a side effect when on() is called for a node. Is the idea that an app would call
on(node, gesture.swipe, function(){/*empty function*/})and then instead setup handlers on <body> viaon(dojo.body(), "swipe.end", ...)? I don't really see the value here. - I expected the swipe module to return {move: ..., end: ...} hash rather than being used as swipe (which is the move gesture) and swipe.end. Although either way works.
- I thought a swipe implied a short, fast gesture but it looks like the swipe code will fire on any touch move, even a long slow one?
- the
lang.getObject("gesture", true, dojo);in gesture/Base seems unnecessary - in tap.js there's a clearTimeout() inside a setTimeout() callback that is unnecessary.
- in the line
this.fire(e.target, {type: "tap.doubletap"});, it seems unusual to have an event type containing a dot; it's inconsistent with the browser native event doubleclick (which is called "doubleclick", not "click.doubleclick")
comment:5 Changed 9 years ago by
Thanks, Bill,
Replying to bill:
I checked it over a bit. Questions:
- Not sure what "defaultEvent" or "subEvents" attributes in subclasses of gesture/Base is for? Maybe I'm missing something, but they don't seem necessary. Likewise with init(), _events and _remove(). I'm just not seeing why you need an infrastructure more complicated than dojo/touch uses.
"defaultEvent" and "subEvents" mainly to accommodate with the Object event, e.g. since we have the following usage:
- on(node, dojo.gesture.tap, func);
- on(node, dojo.gesture.tap.hold, func);
- on(node, dojo.gesture.tap.doubletap, func);
doesn't that already indicate a main-sub relationship e.g tap.hold.
Also we can't provide
- on(node, dojo.gesture.taphold, func)
since that violates module return way.
Maybe we can have a simper list, but still in the form as e.g. ["tap", "tap.hold", "tap.doubletap"] to be consistent with the Object event usage?
The logic is a bit complex than touch because unlike dojo.touch which is simply listening to native events, besides gesture event, dojo.gesture also needs to listen, process and dispatch "press"|"move"|"release"(including remove them - user won't explicitly do that).
- You are firing synthetic DOM events as a side effect when on() is called for a node. Is the idea that an app would call
on(node, gesture.swipe, function(){/*empty function*/})and then instead setup handlers on <body> viaon(dojo.body(), "swipe.end", ...)? I don't really see the value here.
No, that's a not provided usage. "swipe.end" is simply the event.type user got from callback in
- on(node, gesture.swipe, function(event){})
- I expected the swipe module to return {move: ..., end: ...} hash rather than being used as swipe (which is the move gesture) and swipe.end. Although either way works.
Hmm, was just thinng dojo.gesture.swipe is more clean to use.
- I thought a swipe implied a short, fast gesture but it looks like the swipe code will fire on any touch move, even a long slow one?
Yep, based on my new understanding, swipe is just what you do when swiping the screen of iPad or iPhone, no matter fast or slow, the viewport will be slides.
A faster one but only fired when touchend is a flicker.
- the
lang.getObject("gesture", true, dojo);in gesture/Base seems unnecessary- in tap.js there's a clearTimeout() inside a setTimeout() callback that is unnecessary.
Gonna have a check, thx!
- in the line
this.fire(e.target, {type: "tap.doubletap"});, it seems unusual to have an event type containing a dot; it's inconsistent with the browser native event doubleclick (which is called "doubleclick", not "click.doubleclick")
Yep, I have the similar concern, but here just want to be consistent with Object events usage as:
- on(node, dojo.gesture.tap, func);
- on(node, dojo.gesture.tap.hold, func);
- on(node, dojo.gesture.tap.doubletap, func);
Of course we can directly fire "tap", "taphold", "doubletap" or "swipeend" etc. if we don't worry about that.
comment:6 Changed 9 years ago by
I talked to Evan about this a long time offline, here's what I learned:
1. DOMNode synthetic events
Although gesture/Base calls on.emit(node, ...), generating synthetic events on the DOMNode with names like "tap.hold", these synthetic events are for internal use only, specifically to deal with bubbling. For example, they are used to make sure that in the second on() callback below, the callback only executes when dojo.stopEvent() is *not* called:
on(inner, dojo.gesture.tap.hold, function(e){
console.log('inner is tap.hold');
if(xyz) dojo.stopEvent(e);
});
on(outer, dojo.gesture.tap.hold, function(e){
console.log('outer is tap.hold');
});
In the case above, the native touchstart, touchmove etc. events from the inner node bubble to the outer node, but with an _locking attribute set. So, the outer handler ignores them. But when dojo.stopEvent() is not called, the "tap.hold" synthetic event also bubbles upwards.
Apps are not meant to listen directly for an event with a string name of "tap.hold".
2. reference counting
Considering app code that does this:
handle1 = on(node, tap, ...) handle2 = on(node, tap, ...) handle3 = on(node, tap.hold, ...) handle4 = on(node, tap.doubletap, ...)
On the first (and only the first) call to on(node, tap or tap.hold or tap.doublehold), dojo/gesture/tap (via dojo/gesture/Base) will setup 4 internal listeners for touchstart, touchmove, touchend, touchcancel. Essentially, it sets up node to publish tap, tap.hold, and tap.doubletap notifications. And then the first on() call and subsequent() on calls are merely setting up subscriptions listening for those published events.
The internal touchstart, touchmove, touchend, and touchcancel listeners will be removed when handle1, handle2, handle3, and handle4 have all been removed.
I would have preferred if each on() call above generated it's own private internal listeners for touchstart, touchmove, touchend, touchcancel. However, it seems like the current design is necessary for the bubbling described above in #1. We need exactly one synthetic "tap" event to be propagated from the inner node, regardless of how many tap listeners have been registered to listen to tap.
The code does seem quite complicated to me though, and I do question though whether all this complication is worth it just to support proper bubbling. OTOH, for something like the dojo/gesture/tap code, I see how it's the same chunk of code that monitors touch events and fires either tap, tap.hold, or tap.doubletap, depending on what touch events occur, so it might be silly to run that code three times. Anyway, I'd be interested to hear what others think.
My remaining issue is with swipe vs. flick:
Just looking at (for example) gmail on iphone, there's are two ways to scroll:
- moving my finger slowly: scrolling starts when I start moving my finger, and stops as soon as I lift up my finger
- scrolling by a quick flick action: scrolling starts when I start moving my finger, but continues after I remove my finger (slowing down and eventually stopping over time)
I'm unclear how to do that with the current gestures. Is it possible?
comment:10 follow-up: 19 Changed 9 years ago by
I don't suppose it would've been possible to move gesture from dojo to dojox while preserving all the SVN history (i.e. using svn mv)? Seems a shame to have all history left behind in the old removed location, but I'm not sure if our usual checkout with externals makes such a move difficult.
comment:18 Changed 9 years ago by
| Priority: | highest → normal |
|---|
All done except for more doh test cases and ref doc, so not a 1.7 blocker, changing priority from highest to normal
comment:19 Changed 9 years ago by
Replying to kgf:
I don't suppose it would've been possible to move gesture from dojo to dojox while preserving all the SVN history (i.e. using
svn mv)? Seems a shame to have all history left behind in the old removed location, but I'm not sure if our usual checkout with externals makes such a move difficult.
Yep, that's feasible, I've done that so all previous changeset history are preserved.
comment:28 Changed 9 years ago by
comment:29 Changed 9 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
Docs are added at dojo/touch and dojox/gesture
