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

Node.prototype.emit is broken [Bug or Question] #342

Closed
jd-carroll opened this issue Jun 25, 2015 · 6 comments
Closed

Node.prototype.emit is broken [Bug or Question] #342

jd-carroll opened this issue Jun 25, 2015 · 6 comments

Comments

@jd-carroll
Copy link
Contributor

Node.prototype.emit does not emit events globally any more. It will only emit events to the node's children. Is this a bug or intentional? (I didn't see any PR/Issue for this and the jsdoc still reads global)

Went in with this commit: 35c6db1#diff-20164165518dcfb14298730df6349123

@DnMllr
Copy link
Contributor

DnMllr commented Jun 25, 2015

This is intentional, sort of. I didn't mean for this to be a breaking change, I wanted to have a smoother deprecation towards the state that this is in now, but it seems to have gotten rushed in with the transform changes that landed in 0.6 and I missed the documentation.

The idea is that UIEvents flow upwards and custom events flow downwards, such that you can make a closed environment of events with controller classes listening to UI Events above, and view classes responding to state changes below. This is a fairly necessary change if famous is to be used as a component based application framework. If people are writing custom components and event dispatch is global by default, then suddenly we have to be concerned about namespace collisions. One cannot just drag in somebody else's code without figuring out what events they broadcast.

If you still want to broadcast events globally, the possibility exists by supplying the Dispatch with the path of a scene (which will just be a selector) like so:

Dispatch.dispatch('body', 'eventName', {hello: 5});

You can actually use that method to target any location in the scene graph, but I think the safest way to do that would be to make sure you are Dispatching to one of your components children and not to some random place.

There should definitely have been a more graceful switch to this design and I'm sorry for the inconvenience. I'll be issuing a PR ASAP to fix the documentation.

@jd-carroll
Copy link
Contributor Author

@DnMllr I couldn't agree more and that was the theory behind #319. I guess I would have liked to see more of a generic emit(up)/broadcast(down) framework for passing events up the tree rather than restricting to DOM/GL events only. (I'm certain I can hack into that if necessary)

From my standpoint we can close this, however I would mention that all of your examples are now broken (or atleast the ones with events). So from that standpoint, I will leave this for you to close if you wanted to track those changes.

@steve-the-edwards
Copy link

Yes but why only the children of the node on the path? Why not the node specified by path itself? How is a non-UI Event to be sent only to a leaf node?

@michaelobriena
Copy link
Member

@steve-the-edwards You can use Dispatch.emit to send to send events to a specific path.

@steve-the-edwards
Copy link

I think you were meaning Dispatch.dispatch(), yes? Or Node.emit() both of which will send to children of that node, and not the node itself. But that is what we are talking about here I believe :) : #366

@michaelobriena
Copy link
Member

@steve-the-edwards you are correct. I am going to close this for now and keep further conversation in #366

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

No branches or pull requests

4 participants