-
Notifications
You must be signed in to change notification settings - Fork 23
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
Remove mvc/View#mapChildViewEvent #123
Comments
Well it in theory it was for communicating between parent and child views so eh parent view could listen for child events. Seemed to make sense at the time :). A central event dispatcher could also do the trick Sent from my iPhone
|
Hey, Kelly! Didn't expect to see you here. Thanks for chiming in! Honestly I've only ever done DOM event handling with jQuery, but I was thinking: wouldn't doing something like |
The idea was to use the built in event dispatcher apart of the prototype of View to communicate up the chain. The problem is the events don't bubble past the parent view by default. This whole issue is avoided in practice if your okay with a childview knowing about its parent. |
And now George! So we were reusing the EventDispatcher part. That does make some sense, though I wonder why we didn't just use DOM events instead of EventDispatcher for Views in the first place. I vaguely remember reading something about performance and # of DOM events a while back, but the only slowdown I've ever seen had to do with large numbers of handlers actively being called. |
yeah, i'm fine without it as well. it was modeled after how model events On Thu, Jul 3, 2014 at 9:38 AM, Zach Shipley [email protected]
|
Hmm... would there be a down-side to y'all? My line of thinking is:
|
I'd like to know if anyone knows what this is for. Doing a
git blame
shows that Kelly added it under 7c64abd "refactoring view classes..." Of the projects [PropertyCross, autotrader-web, kbb2, kbb, lavaca-starter, nike-video, selectcomfortproto], it's only used in one view in kbb2. If I'm understanding right, I think that regular old DOM events (mapped withmapEvent
in the parent) would be just as effective here.The text was updated successfully, but these errors were encountered: