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.

Sign up for GitHub

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

Jump to bottom

Events refactor followups #656

Closed
8 of 11 tasks
ericsoco opened this issue May 23, 2017 · 6 comments
Closed
8 of 11 tasks

Events refactor followups #656

ericsoco opened this issue May 23, 2017 · 6 comments
Assignees
@ericsoco
Milestone
v5.0

Comments

@ericsoco
Copy link
Contributor

ericsoco commented May 23, 2017 •
edited by howtimeflies0
Loading

This issue tracks followups to the event handling refactor:

Once landed, the following events-related work remains:
@ericsoco ericsoco self-assigned this May 23, 2017
@Pessimistress
Copy link
Collaborator

Pessimistress commented May 23, 2017 •
edited
Loading

npm run test-dist is failing because hammer.js accesses document.
Same problem also breaks isomorphic rendering when importing from deck.gl on the server side. Should we do a sanity check in EventManager?

@ericsoco
Copy link
Contributor Author

Probably, yes -- don't see any reason anything in EventManager needs to (or can be expected to) work on the server.

Also, for reference: hammerjs/hammer.js#930

@ericsoco
Copy link
Contributor Author

ericsoco commented May 24, 2017 •
edited
Loading

See also:

  • Unit tests.

@Pessimistress
Copy link
Collaborator

Pessimistress commented May 30, 2017 •
edited
Loading

There is a bug with mousemove:

  • Both pointermove and mousemove events are fired
  • When dragging, an additional move event is fired from hammer.js

This causes picking to be triggered multiple times each frame during drag.

EDIT: This has been fixed by #667

ericsoco added a commit that referenced this issue Jun 7, 2017
@ericsoco
Ensure multiple handlers can coexist for an aliased event, and can be…
6cc7267 
… removed independently. (#656)
@ericsoco ericsoco mentioned this issue Jun 7, 2017
Merged
ericsoco added a commit that referenced this issue Jun 7, 2017
@ericsoco
Ensure multiple handlers can coexist for an aliased event, and can be…
da6151c 
… removed independently. (#656)
ericsoco added a commit that referenced this issue Jun 7, 2017
@ericsoco
Ensure multiple handlers can coexist for an aliased event, and can be… (
1e2e97f 
#693)

* Ensure multiple handlers can coexist for an aliased event, and can be removed independently. (#656)

* Fix lint (max-depth).

* Don't store unnecessary refs to handlers, since Hammer is already caching them.
However, since Hammer doesn't have an API for determining if there are handlers registered for a given event, we must still keep track of whether or not aliased handlers are registered.

* Address comments.

* Fix lint
ericsoco added a commit that referenced this issue Jun 7, 2017
@ericsoco
Support overriding default recognizers. (#656)
9176ea3
ericsoco added a commit that referenced this issue Jun 7, 2017
@ericsoco
Support overriding default recognizers. (#656) (#696)
3a02cee 
* Support overriding default recognizers. (#656)

* Try that again.
@ericsoco ericsoco mentioned this issue Jun 7, 2017
Closed
Firenze11 pushed a commit to Firenze11/deck.gl that referenced this issue Jun 9, 2017
@ericsoco
Ensure multiple handlers can coexist for an aliased event, and can be… (
74b3a27 
visgl#693)

* Ensure multiple handlers can coexist for an aliased event, and can be removed independently. (visgl#656)

* Fix lint (max-depth).

* Don't store unnecessary refs to handlers, since Hammer is already caching them.
However, since Hammer doesn't have an API for determining if there are handlers registered for a given event, we must still keep track of whether or not aliased handlers are registered.

* Address comments.

* Fix lint
Firenze11 pushed a commit to Firenze11/deck.gl that referenced this issue Jun 9, 2017
@ericsoco
Support overriding default recognizers. (visgl#656) (visgl#696)
5d0bb8d 
* Support overriding default recognizers. (visgl#656)

* Try that again.
howtimeflies0 pushed a commit that referenced this issue Jun 9, 2017
@ericsoco
Ensure multiple handlers can coexist for an aliased event, and can be… (
a8a8e4e 
#693)

* Ensure multiple handlers can coexist for an aliased event, and can be removed independently. (#656)

* Fix lint (max-depth).

* Don't store unnecessary refs to handlers, since Hammer is already caching them.
However, since Hammer doesn't have an API for determining if there are handlers registered for a given event, we must still keep track of whether or not aliased handlers are registered.

* Address comments.

* Fix lint
howtimeflies0 pushed a commit that referenced this issue Jun 9, 2017
@ericsoco
Support overriding default recognizers. (#656) (#696)
8aa80dd 
* Support overriding default recognizers. (#656)

* Try that again.
@ericsoco ericsoco mentioned this issue Jun 15, 2017
Closed
@howtimeflies0
Copy link

@Pessimistress can you do a pass on the remaining items listed in this issue tracker?

@ibgreen ibgreen added this to the v.Next milestone Aug 18, 2017
@ibgreen
Copy link
Collaborator

ibgreen commented Oct 11, 2017

event handling is moved to mjolnir.js, let's track it there if needed.

@ibgreen ibgreen closed this as completed Oct 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ericsoco ericsoco

Labels
None yet
Projects
None yet
Milestone
v5.0
Development

No branches or pull requests
4 participants
@ericsoco @Pessimistress @ibgreen @howtimeflies0