Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Suppress mousemove events when there aren't any views in the DOM that ha... #2164

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants
Contributor

endash commented Feb 25, 2013

...ndle them.

See #2148

** not ready for merge--needs more test coverage.

Owner

stefanpenner commented Mar 3, 2013

This is a good start. Some low hanging fruit: Most of the new properties and methods added to View to support this feature seem to be private/internal. Thoughts on prefixing them with _ and documenting them as such?

I think exposing the eventDispatcher via the container is a great idea. The future of a more powerful/configurable eventDispatcher is very appealing. Especially when once considers touch/gesture fun.

Anyways, I think this is the correct direction, any other feedback?

Contributor

endash commented Mar 3, 2013

I'm still at a bit of a loss of how to properly test the dependency on the event dispatcher being in the container

Contributor

endash commented Mar 3, 2013

Also the reason I didn't make the new props private is because they actually only get accessed externally... by the state object. Not really sure how to handle the notion of private methods/properties in this case.

Owner

wagenet commented Mar 5, 2013

It would be good to get some feedback from @kselden and @ebryn on this.

I assume touchMove is the same problem, can it also be included?

Contributor

endash commented Mar 20, 2013

@SamSaffron Sure. I just kept it barebones knowing there'd likely be feedback.

Owner

krisselden commented Jun 21, 2013

You can now just reopen the EventDispatcher with the events you want delegated: a34b27c

@krisselden krisselden closed this Jun 21, 2013

@krisselden krisselden reopened this Jun 21, 2013

Owner

krisselden commented Jun 21, 2013

misunderstood, you want it targeted per view not app

Owner

wagenet commented Jul 19, 2013

This still seems like a useful idea. It sounds like the core team needs to discuss this a bit before we can merge anything.

Owner

krisselden commented Jul 20, 2013

Upon further review, this still turns it off/on for all views, but you specify it in a particular view. Turning it on/off for all views is already covered by the commit I referenced.

Owner

krisselden commented Jul 20, 2013

I think this should be closed, since it just turns it on/off for all views I think it should be specified in the event dispatcher when you create the app, you set the events you want delegated for your app.

Contributor

endash commented Jul 20, 2013

Not quite the same, since the PR allows the events to be turned on when needed, and off when not. That said, this was my solution for someone else's problem, sooooo I can't say I have a very strong opinion.

Owner

wagenet commented Jul 28, 2013

I think this concept is generally good. We should try to get a little more feedback on this and actually make a decision soon.

Owner

wagenet commented Oct 12, 2013

There hasn't been a lot of progress on this. I still think it's a good idea. If someone wants to follow up on this more, starting a discussion on the forums would be a good idea. Thanks for giving this a shot @endash.

@wagenet wagenet closed this Oct 12, 2013

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