Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Support mobile events #488

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pierre-moire
Copy link
Contributor

@pierre-moire pierre-moire commented Nov 25, 2019

Checklist

  • The code has been run through pretty yarn run pretty
  • The tests pass on CircleCI
  • You have referenced the issue(s) or other PR(s) this fixes/relates-to
  • The PR Template has been filled out (see below)
  • Had a beer/coffee because you are awesome

What?

Make the library work with mobile devices

Why?

Because it rockzz and I need it for a project. Also will close #453 and #301 requests

How?

By adding touch start, touch move and touch end event handling

Feel good image:

LOL

@pierre-moire
Copy link
Contributor Author

Everything works except the port linking: when I touchend over a "in" port, the link is not made. For the rest I could make it work in Chrome with simulated ipad mode that uses touch events.

@dylanvorster @antonioru I am totally new to Typescript so I would probably need help on the last Typescript warning in DragNewLinkState. Can you provide advice here?

@antonioru
Copy link
Contributor

@pierre-moire I kind of dislike typescript per se, but if you have any question just ask me :)

@pierre-moire
Copy link
Contributor Author

Yes thanks, there is a conflict in DragNewLinkState and I don't know exactly how I can resolve it:

image

@antonioru
Copy link
Contributor

Yes thanks, there is a conflict in DragNewLinkState and I don't know exactly how I can resolve it:

image

I wrote the isNearbySourcePort method, the error is given because you're using a MouseEvent to describe a TouchEvent.
You can possibly use both by doing something like this: event: MouseEvent | TouchEvent.

If you want to add the touch support, which is great, I suggest to follow @dylanvorster 's code style and abstract the whole thing into a new class to handle both cases, otherwise we'll have to check if is a TouchEvent or a MouseEvent every time we'll handle a user action.

@pierre-moire
Copy link
Contributor Author

There is no reason to create a separated DragNewLinkState just for mobile ; I will create a new function instead

@pierre-moire
Copy link
Contributor Author

@antonioru I'm kinda stuck for now ; I don't exactly know how I should change the structure

@tomodutch
Copy link

Any updates on the mobile support? I'm looking into this library for a future project and mobile support is a must. What's still left to be done? I may be able to help out with some typescript questions because it seems to be stuck on that.

@pierre-moire
Copy link
Contributor Author

The organization of code has yet to be determined. Several functions are referring to Mouse events so we need either to duplicate the functions so they can handle touch events, or modify existing functions to ensure they can take the 2 types of events.

If we make the events inherit from the same parent it can be probably easier but events data shape is different and typescript doesn't appreciate when data structure varies.

@Blackpinned
Copy link

Would this be able to simply read the data and show it visually? I only need it to be shown on mobile.

@renato-bohler
Copy link
Contributor

I also think that if we achieve visualization on mobile that's better than nothing.

@pierre-moire
Copy link
Contributor Author

It is correctly displayed on mobile already, but you won't be able to drag the canvas

@Blackpinned
Copy link

How would I go about adding your version then? Just download that from the repo instead of the normal one?

@renato-bohler
Copy link
Contributor

Whoops... with "visualization" I wanted to mean "displaying + dragging" 😅

@royair
Copy link

royair commented Jul 2, 2020

@pierre-moire is there any new with this feature?

@alxAgu
Copy link

alxAgu commented Nov 17, 2020

Any chance of getting this resolved in the foreseeable future (2020)? Kind of a showstopper.

p.s. A very promising library, great job!

@yuridevx
Copy link

Hi @pierre-moire @dylanvorster I'm willing to contribute on this issue, as I also need mobile support. Will you be able to approve / guide me through what was missing in this PR?

@sbayd
Copy link

sbayd commented Feb 3, 2021

Hi, @pierre-moire and @dylanvorster first of all thank you for great library and your effort in this PR.

I'm also willing to work on this issue if is it possible for one of you to guide me through current status of this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mobile touch events
9 participants