Opened 7 years ago
Closed 6 years ago
#16453 closed enhancement (fixed)
[patch][ccla]Support dojo/touch in canvasWithEvents renderer
| Reported by: | Patrick Ruzand | Owned by: | Patrick Ruzand |
|---|---|---|---|
| Priority: | high | Milestone: | 1.9 |
| Component: | DojoX GFX | Version: | 1.8.1 |
| Keywords: | Cc: | Eugene Lazutkin, Colin Snover, bill, cjolif | |
| Blocked By: | Blocking: |
Description
The current canvasWithEvents implementation needs to be updated so that it supports the dojo/touch api.
Attachments (2)
Change History (11)
comment:1 Changed 7 years ago by
| Milestone: | tbd → 1.9 |
|---|---|
| Owner: | changed from Eugene Lazutkin to Patrick Ruzand |
| Priority: | undecided → high |
| Status: | new → assigned |
Changed 6 years ago by
| Attachment: | 16453.patch added |
|---|
comment:2 Changed 6 years ago by
| Cc: | Eugene Lazutkin Colin Snover bill cjolif added |
|---|---|
| Summary: | Support dojo/touch in canvasWithEvents renderer → [patch][ccla]Support dojo/touch in canvasWithEvents renderer |
comment:3 Changed 6 years ago by
This patch adds on() interface to gfx API, and also adds support to dojo/touch with canvasWithEvents renderer.
The canvasWithEvents part is a major revision of the previous event dispatching code, so that it fits with the dojo/touch API that works on real DOM nodes. To this purpose, the canvas node is added a dojoElementFromPoint(x,y) method that does hit testing on the shapes. The dojo/touch module also supports this generic hit testing API, so, when a node is returned by a document.elementFromPoint() call, it checks whether this node implements dojoElementFromPoint, and if so, call it. In response, canvasWithEvents returns a "fake" node that is processed as real dom nodes. This way, a non-DOM scene as gfx/canvas can be processed by dojo/touch.
[summary of ongoing discussions]
bill has strong reserves with this approach, mostly motivated by the already too much complex dojo/touch implementation and the risks there is to add additional level of complexity in a such fragile module. Instead of modifying dojo/touch, gfx should either define and implements its own event abstraction, or move the code required to make it work with canvasWithEvent out of dojo/touch into gfx. Since canvasWithEvents implements hit testing and is already listening to touch.move, and emitting enter/leave/over/out events, it should be possible to make it emits "dojotouchend", "dojotouchover" etc. type events, which would fool any touch.* listeners down the line.
comment:4 follow-up: 6 Changed 6 years ago by
I asked Patrick to update the ticket description to say exactly what API's you are claiming to support, in terms of the possible targets passed to on() (Rect, Group, Surface, <canvas>, <body>), and the event object passed to the listener callback (evt.target, evt.relatedTarget, evt.gfxTarget.). Also, whether on.selector() is supposed to work. Plus, do events bubble through hierarchies of shapes (Rect --> Group --> Group --> Surface --> <body>)? If there's too much of a gap between what users expect to work vs. what actually works, it might be better doing nothing at all, or waiting until the feature works better before checking it in. Also, test_bubbling2.html should be updated to test everything that the patch claims to support.
It's true I'm not happy with the complication to dojo/touch to support these "fake" nodes... seems like something that should be in dojo/on itself, if anywhere in core. But more so than that, my issue is that dojo/touch (in the patch) only half-supports these fake nodes. Specifically, the event passed to the listener (i.e. the listener registered via on(foo, touch.*, callback)) doesn't have target and relatedTarget set to the fake nodes, but rather to the <canvas> node. gfx/canvasWithEvents fixes the event object, but really dojo/touch needs to stand on it's own. If dojo/touch::dualEvent() patched the event object I'd feel a lot better. Maybe something as simple as lang.delegate(evt, {target: evt.dojoTarget, relatedTarget: evt.dojoRelatedTarget}).
comment:5 Changed 6 years ago by
PS: Oh, also need to list (and test) whether touch.enter and touch.leave are supposed to work. I see the pseudo node (shape.rawNode) has a parentNode property, so dojo/mouse::eventHandler()'s call to dom.isDescendant() check will work (at least, until #14629 is implemented), but AFAICT shape.rawNode.parentNode is null for top level shapes within the canvas, so that looks broken.
comment:6 Changed 6 years ago by
Replying to bill:
I asked Patrick to update the ticket description to say exactly what API's you are claiming to support, in terms of the possible targets passed to on() (Rect, Group, Surface, <canvas>, <body>),
The scope of this patch is limited to gfx, and adds the new dojox/gfx/shape.Shape.on() method + Surface.on() and deprecates the previous connect/disconnect interface. Which means you don't pass a gfx shape to on() (it has never been the case), but instead: myShape.on("mousedown", ...); myShape.on(touch.press, ...)
and the event object passed to the listener callback (evt.target, evt.relatedTarget, evt.gfxTarget.).
The event object passed to the listener callback is not modified by this patch. evt.target is the "real" event emitters (the raw node), and evt.gfxTarget the gfx shape associated with the target.
Also, whether on.selector() is supposed to work.
no. Why should it be ? From a general manner, you are not supposed to use dom api directly on gfx. And we are not proposing a DOM-based canvas, only make the canvasWithEvents renderer compatible with dojo/touch.
Plus, do events bubble through hierarchies of shapes (Rect --> Group --> Group --> Surface --> <body>)?
Limited to gfx hierarchy. Rect --> Group --> Group --> Surface.
I would like to emphasize that the scope of this patch is limited to make the canvaWithEvents GFX renderer compatible with dojo/touch API, and not make a DOM canvas implementation which could be used with all dom-related dojo APIs.
comment:7 Changed 6 years ago by
you don't pass a gfx shape to on() (it has never been the case), but instead: myShape.on("mousedown", ...); myShape.on(touch.press, ...)
OK. For the record, I guess my uneasiness is because:
- The touch API is always used like on(node, touch.*, ...), so that's what users are likely to do. And actually, it will work for the most part (ex:
on(mySurface, touch.press, ...)), but fails in other cases likeon(mySurface, on.selector(".graphBar", touch.press), ...)
- The patch actually does emit bubbling events on <canvas>, from the line:
on.emit(canvas, type.type, event). So, it is supporting events on DOMNodes, and even trying to support things likeon(document, "mouseout", ...)where the mouseout occurred on a GFX canvas shape rather than a real DOMNode.
comment:8 Changed 6 years ago by
After some discussions, I have came up with another patch that has much less impact on dojo/touch as asked by bill (but still modifies it so that the emitted "dojotouchmove" event gets the right information from the synthetic event it receives).
add dojo/touch support to canvas + on() interface, patch by pruzand and csnover