Opened 8 years ago

Closed 7 years ago

#12363 closed defect (fixed)

[patch] [ccla] gfx moveable does not work on touch devices

Reported by: clinejm Owned by: Colin Snover
Priority: blocker Milestone: 1.8
Component: DojoX GFX Version: 1.6.0rc1
Keywords: gfx touch ios Cc: cjolif, snover
Blocked By: Blocking:

Description

I've created a patch to both dojox.gfx.Moveable and dojox.gfx.Mover that adds touch event support.

I've tested this code on iOS (iphone 4, iPad) as well as Desktop Safari, Chrome, FireFox? and Internet Explorer.

Attachments (2)

gfx-touch.diff (4.1 KB) - added by clinejm 8 years ago.
12363.diff (4.0 KB) - added by Colin Snover 7 years ago.
use dojo/touch

Download all attachments as: .zip

Change History (20)

Changed 8 years ago by clinejm

Attachment: gfx-touch.diff added

comment:1 Changed 8 years ago by bill

Summary: Add touch event handling to gfx[patch] Add touch event handling to gfx

I did the same thing for a few components in dijit.... although this change and those changes might be unneeded / require refactor after #12176.

Have you filed a CLA?

comment:2 Changed 8 years ago by clinejm

Hi Bill,

Yes, my work falls under the SitePen? CLA. Just ask Dylan.

comment:3 Changed 7 years ago by Patrick Ruzand

Milestone: tbd1.8

comment:4 Changed 7 years ago by Patrick Ruzand

#12945 is a duplicate of this ticket.

comment:5 Changed 7 years ago by Colin Snover

Summary: [patch] Add touch event handling to gfx[patch] [ccla] Add touch event handling to gfx

comment:6 Changed 7 years ago by dylan

I'm a bit shocked this never landed... makes gfx pretty unusable on mobile. If this has not been implemented, can we get an updated AMD-based fix for this in for 1.8? Or have touch events been enabled for GFX already?

comment:7 in reply to:  6 Changed 7 years ago by Eugene Lazutkin

Priority: highblocker

Replying to dylan:

I'm a bit shocked this never landed... makes gfx pretty unusable on mobile.

I am 100% with Dylan. We should make this feature a top priority because it affects everything including charting, which more and more people want to see on iPads and Android tablets.

comment:8 Changed 7 years ago by Colin Snover

Summary: [patch] [ccla] Add touch event handling to gfx[patch] [ccla] gfx moveable does not work on touch devices
Type: enhancementdefect

So who has bandwidth to update the patch, test, and land it?

comment:9 Changed 7 years ago by Patrick Ruzand

Some clarifications to the comments above as there are several wrong assertions:

  • gfx has been touch-enabled since the 1.7 release, and works well on android,iphone and bb.
  • this ticket concerns the dojox.gfx.Moveable and dojox.gfx.Mover classes *only*.
  • charting don't use Moveable/Mover? so it is not affected.

So I am a bit surprised to read this makes gfx pretty unusable on mobile. shape.connect('touchstart', foo) will work, even on canvas. It is only the built-in drag interaction that provides Moveable/Mover? that is broken.

IMO, since the scope of this ticket is quite limited as it only concerns the Moveable/Mover? classes, it should not be considered a blocker for 1.8. (but should be fixed in 1.8.1 I agree).

comment:10 Changed 7 years ago by bill

Priority: blockerhigh

comment:11 Changed 7 years ago by bill

Cc: cjolif snover added
Priority: highblocker

From the meeting, it turned out that snover needs this, so we said that if snover can supply an updated patch cjolif will check it in.

comment:12 in reply to:  11 ; Changed 7 years ago by Eugene Lazutkin

Replying to bill:

From the meeting, it turned out that snover needs this, so we said that if snover can supply an updated patch cjolif will check it in.

What is wrong with the current patch?

comment:13 in reply to:  12 Changed 7 years ago by Colin Snover

Replying to elazutkin:

What is wrong with the current patch?

dojo.connect, dojo.disconnect, dojo.stopEvent, the fact that this code has been heavily modified in the intervening 17 months…stuff like that.

comment:14 Changed 7 years ago by cjolif

csnover, I have managed to reach Patrick during his vacation. He kindly agreed to work on the patch rework. So don't spend time on this. Thanks.

comment:15 Changed 7 years ago by Patrick Ruzand

Resolution: fixed
Status: newclosed

In [29220]:

add touchevent support, fixes #12363

comment:16 Changed 7 years ago by Colin Snover

Resolution: fixed
Status: closedreopened

The changeset [29220] should not be attaching both mouse *and* touch listeners. dojo/touch exists to smooth over these differences.

Changed 7 years ago by Colin Snover

Attachment: 12363.diff added

use dojo/touch

comment:17 Changed 7 years ago by bill

Owner: changed from Eugene Lazutkin to Colin Snover
Status: reopenedassigned

Since pruzzand is on vacation, csnover can you check in?

comment:18 Changed 7 years ago by Colin Snover

Resolution: fixed
Status: assignedclosed

In [29382]:

Use dojo/touch for touch support in dojox/gfx/Moveable.
Fixes #12363. !strict due to existing invalid code.

Note: See TracTickets for help on using tickets.