engine icon indicating copy to clipboard operation
engine copied to clipboard

Fix ElementTouchEvent to convert native DOM Touch objects to PlayCanvas Touch objects

Open Copilot opened this issue 2 months ago • 4 comments

ElementTouchEvent was passing native DOM Touch objects directly to consumers instead of converting them to PlayCanvas Touch objects. This caused touch.id to return undefined since native touches use touch.identifier.

Changes

  • Import PlayCanvas Touch class in element-input.js
  • Convert native touches in ElementTouchEvent constructor:
    this.touches = Array.from(event.touches).map(t => new Touch(t));
    this.changedTouches = Array.from(event.changedTouches).map(t => new Touch(t));
    
  • Add test verifying ElementTouchEvent provides PlayCanvas Touch objects with accessible id property

Example

Before this fix:

element.on('touchstart', (event) => {
  console.log(event.touch.id);  // undefined (native Touch uses 'identifier')
});

After this fix:

element.on('touchstart', (event) => {
  console.log(event.touch.id);  // 123 (PlayCanvas Touch exposes 'id')
});
Original prompt

This section details on the original issue you should resolve

<issue_title>ElementTouchEvent uses wrong type for its touches</issue_title> <issue_description>Hey everyone,

When I try to access the id-field on touches contained within an ElementTouchEvent, I always receive undefined. Upon further inspection, it turns out touches contained within ElementTouchEvent are of the DOM Touch-type rather than ElementTouchEvent.Touch as declared by the most recent PlayCanvas type definitions ([email protected] from NPM).

Here's a screenshot of VSCode showing the type of the touch-field on an ElementTouchEvent:

Image

Here's a confirmation through some debugging that the touch event I receive from my ElementComponent is in fact of type ElementTouchEvent, and that touches contained within are of the DOM Touch-type:

Image

It turns out that element-input.js in the engine just passes the native Touches it receives straight into a new ElementTouchEvent rather than converting them:

Image

Just for clarity, the constructor of ElementTouchEvent doesn't convert the touches either.

I guess either the type definitions pulled from NPM are incorrect, or the touches should have been converted to the appropriate type so that the id-field on touches can be used as expected.

Can someone please address this? Thanks a bunch! 💪 </issue_description>

Comments on the Issue (you are @copilot in this section)

  • Fixes playcanvas/engine#7927

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot avatar Oct 30 '25 09:10 Copilot

That looks like breaking change.

Maksims avatar Oct 30 '25 11:10 Maksims

@Maksims I think you have to address Copilot directly to suggest to it you think it's a breaking change (it claimed it's backwards compatible).

willeastcott avatar Oct 30 '25 11:10 willeastcott

I don't work with AI, I can clearly see, that public API docs suggest to return PlayCanvas's Touch but in reality it returns native Touch. It might be simply docs problem, and Copilot thought to fix it.

It will break, as people rely on these touch objects, and that they are native Touch objects, not PlayCanvas's.

Maksims avatar Oct 30 '25 11:10 Maksims

@Maksims This PR is an experiment to see how useful GitHub Copilot is as resolving an issue. If you don't want to participate in the experiment, feel free to drop out of the convo.

willeastcott avatar Oct 30 '25 11:10 willeastcott