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

web; runtime; add browser_window_mouse(down|up|move) #101

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jeremyfa
Copy link
Member

@jeremyfa jeremyfa commented Aug 1, 2017

These options allow to handle mouse events (down, move, up) in the entire browser window instead of just the canvas. It makes it possible to track a started mouse event outside the canvas and finish it inside it and vice versa.

I needed this because I embed a luxe view inside a bigger html page and need to keep track of what the mouse is doing outside the canvas. Before that, when starting a mousedown event in the canvas (to drag/select/move something) and releasing the mouse button mouseup outside the canvas, the app would still think my mouse is pressed when it isn't. These options make it possible to handle it nicely.

These options are disabled by default. If enabled, I didn't foresee any regression regarding computing event positions as snow is not using MouseEvent values that depend on which element it was captured from anyway. Only window level values, then translating to canvas positions.

These options allow to handle mouse events (down, move, up) in the entire browser window instead of just the canvas. It makes it possible to track a started mouse event outside the canvas and finish it inside it and vice versa.
@ruby0x1
Copy link
Member

ruby0x1 commented Aug 1, 2017

Interesting! I don't know if I like it being 3 different config values for essentially the same thing..

Also I would mention that the typical approach I would use is the reverse, you can submit any events you want to the runtime, at any point. Listen on window (application specific) -> tell snow via dispatch_* functions.

@jeremyfa
Copy link
Member Author

jeremyfa commented Aug 1, 2017

I made them separate options because, for instance in my case, I don't need to catch mousedown events that start outside the canvas. Only the contrary. We could make it a single option, but it makes it a bit less flexible.

I see that we can send events via dispatch_* to snow. But then it would already be a more complex approach as I would not want to have twice the same events (ie. default snow mouseup event + custom window event sent by application via dispatch_*). That means I would need to check if an event is inside the canvas or not and only send it if not etc...

With this PR it is just about setting an option (or two, three) to true.

The dispatch_* might be viable if there is a way to disable default event handling of snow/web, in order to make a full replacement. But still, it seems like a lot of specific code just to solve a problem which I still consider pretty generic/common.

This option may also be helpful for any luxe game, embedded in a bigger page, that relies on mouseup events that can happen outside the canvas and would be lost without it enabled.

@ruby0x1
Copy link
Member

ruby0x1 commented Aug 1, 2017

Right, I think there are multiple "problems" mentioned that's why I am thinking about it. Mouse up happening outside the canvas is supposed to happen regardless, but it would be weird if it happened and you didn't initiate that click inside the canvas, i.e "random mouseup from all over the web page" isn't useful as "there was a click inside the canvas, that left the canvas, but should still fire mouseup".

I'll take a closer look when I can but I want to consider it a bit more closely, and I already know that I don't think the 3 config options are ideal here :)

@jeremyfa
Copy link
Member Author

jeremyfa commented Aug 1, 2017

Making it a single option would be ok. Thanks for the feedback!

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

Successfully merging this pull request may close these issues.

None yet

2 participants