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)

16453.patch (53.0 KB) - added by Patrick Ruzand 6 years ago.
add dojo/touch support to canvas + on() interface, patch by pruzand and csnover
16453.2.patch (56.9 KB) - added by Patrick Ruzand 6 years ago.
2nd revision. by csnover, bill, pruzand

Download all attachments as: .zip

Change History (11)

comment:1 Changed 7 years ago by Patrick Ruzand

Milestone: tbd1.9
Owner: changed from Eugene Lazutkin to Patrick Ruzand
Priority: undecidedhigh
Status: newassigned

Changed 6 years ago by Patrick Ruzand

Attachment: 16453.patch added

add dojo/touch support to canvas + on() interface, patch by pruzand and csnover

comment:2 Changed 6 years ago by Patrick Ruzand

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 Patrick Ruzand

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 Changed 6 years ago by 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>), 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 bill

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 in reply to:  4 Changed 6 years ago by Patrick Ruzand

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.

Last edited 6 years ago by Patrick Ruzand (previous) (diff)

comment:7 Changed 6 years ago by bill

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:

  1. 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 like on(mySurface, on.selector(".graphBar", touch.press), ...)
  1. 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 like on(document, "mouseout", ...) where the mouseout occurred on a GFX canvas shape rather than a real DOMNode.

comment:8 Changed 6 years ago by Patrick Ruzand

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).

Changed 6 years ago by Patrick Ruzand

Attachment: 16453.2.patch added

2nd revision. by csnover, bill, pruzand

comment:9 Changed 6 years ago by Patrick Ruzand

Resolution: fixed
Status: assignedclosed

In [31008]:

add Shape.on() interface, update canvasWithEvents to support dojo/touch, fixes #16453,#16456 !strict

Note: See TracTickets for help on using tickets.