-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add touchenter event, add JSDoc, small refactor #4819
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
Conversation
# Conflicts: # src/framework/input/element-input.js
Rewrite old TypeDefs Fix not-definitely-assigned issue with _clickedEntities
|
Can anyone think of any objections to making Right now in engine/src/framework/input/element-input.js Lines 378 to 381 in b232b13
First it checks engine/src/framework/app-base.js Lines 519 to 522 in b232b13
Which usually assigns engine/src/framework/input/element-input.js Lines 435 to 443 in b232b13
It would remove some lines of code and simplify the |
|
I forked @mgawinKatana's project which handles now mouse/touch for testing this PR: https://playcanvas.com/project/1007930/overview/elementmousemove-fix https://launch.playcanvas.com/1587642 Is there a way to |
I'm afraid not, no. The way I've been doing it is either overrides in the devtools or putting the engine file into the project and having it load after the engine. Added ticket here: playcanvas/editor#925 |
# Conflicts: # src/framework/input/element-input.js
yaustar
left a comment
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.
We also need to bubble these events to button like we do for touch leave https://github.com/playcanvas/engine/blob/main/src/framework/components/button/component.js#L297
| } | ||
|
|
||
| this._fireEvent('touchmove', new ElementTouchEvent(event, oldTouchInfo.element, oldTouchInfo.camera, coords.x, coords.y, touch)); | ||
| // And fire touchenter if we've entered the previously left touched element again |
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.
I feel like we should fire touchenter if a user has clicked outside the element and moved into it as well. Seems odd to only fire it if we started in an element, leave and then re-enter
Fixes #4755
Firing
mousemoveeven when the mouse is outside theElementComponentis actually correct behaviour and other elements depend on this, e.g.ScrollViewComponent. So the first misconception in the issue is to imply "mouse must be over element" based on "mousemove". The events that actually give you this information aremouseenter/mouseleave.That works just fine, but when I tested this on iPhone, I realized there isn't even a
touchenterevent yet, so this PR is adding that for the same abilities across plattforms.I saw @jpauloruschel working with this and your last PR also changed the
mousemovebehaviour: ff00ed5IMO the current behaviour is exactly how it should be, otherwise any developer who can only rely on
mousemoveevents trying to implement e.g. a custom scroll element simply has no events to base the scrolling behaviour on.Maybe I am completely wrong, any thoughts/feedback?
I confirm I have read the contributing guidelines and signed the Contributor License Agreement.