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

Suggest change event flow #818

Closed
wants to merge 1 commit into from
Closed

Suggest change event flow #818

wants to merge 1 commit into from

Conversation

mixed
Copy link
Contributor

@mixed mixed commented Jul 2, 2015

Hi.
I suggest change event flow.

The PanRecognizer have "pan(start|move|end|cancel)" and "pan(left|right|up|down)" event. The PinchRecognizer have "pinch(start|move|end|cancel)" and "pinch(in|out)" event.

The "emit" method of Recognizer is fire "pan(start|move|end|cancel)", "pinch(start|move|end|cancel)".The "emit" method of Manager is fire "pan(left|right|up|down)", "pinch(in|out)" .

The "emit" method of PanRecognizer call step is
1. Manager#emit call
2. Recognizer#emit call
So. The event flow of PanRecognizer is "pan(left|right|up|down) -> panstart -> (left|right|up|down) ....-> panend".

The "emit" method of PinchRecognizer call step is
1. Recognizer#emit call
2. Manager#emit call
So. The event flow of PinchRecognizer is "pinchstart -> pinch(in|out) .... -> pinchend -> pinch(in|out)".

Event flow is different from each other. [sample]

I suggest that same event flow.

"pinchstart -> pinch(in|out) .... -> pinchend"
"panstart -> pan(left|right|up|down) ....-> panend"

equal(eventflow,"startleftend");
start();
});
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit pick your missing a new line at end of file

@arschmitz
Copy link
Contributor

@runspired This change makes sense to me its what mouse events do. What do you think?

@arschmitz arschmitz added the bug label Aug 7, 2015
@arschmitz arschmitz self-assigned this Aug 7, 2015
@runspired
Copy link
Contributor

Seems to make sense but I'm on my phone

@runspired
Copy link
Contributor

Looked closer, this is good, and fixes several open issues

@runspired
Copy link
Contributor

Would fix #824

@arschmitz arschmitz closed this in 92f2d76 Aug 7, 2015
@mixed
Copy link
Contributor Author

mixed commented Aug 10, 2015

@arschmitz @runspired
Thank you.

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

Successfully merging this pull request may close these issues.

3 participants