-
-
Notifications
You must be signed in to change notification settings - Fork 269
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
Add Touch Event. Fixes issue #579 #580
base: master
Are you sure you want to change the base?
Conversation
public interface Touch extends JSObject { | ||
|
||
@JSProperty | ||
long getIdentifier(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't work. JavaScript does not provide int64 type (or at least, did not provide by the time touch event spec was written). I'd replaced this with int
, but you should test on real use cases if int
fits well or you need double
.
|
||
// From https://developer.mozilla.org/en-US/docs/Web/API/TouchEvent | ||
|
||
public interface TouchEvent extends Event { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, you created an event class. But how one's to use it. What about declaring corresponding event target interface, like I did it with MouseEventTarget, for example?
JSArray<Touch> getChangedTouches(); | ||
|
||
@JSProperty | ||
JSArray<Touch> getTargetTouches(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Is it really necessary to expose this property as
JSArray
? What about justTouch[]
? - If you still want JS native type, consider
JSArrayReader<Touch>
here
This PR can be closed. |
Hi,
I wanted to play with touch events today, so I added them myself. Please review code and merge upstream if you like it.
--Coder