Skip to content
This repository has been archived by the owner on Jun 15, 2021. It is now read-only.

Added event namespacing #337

Merged
merged 5 commits into from Feb 16, 2015
Merged

Added event namespacing #337

merged 5 commits into from Feb 16, 2015

Conversation

cutandpastey
Copy link
Contributor

Adding namespacing to events will allow us to cleanup some of the state management in noting.

@@ -4,3 +4,4 @@
/vendor/
/examples/build.js
.tern-port
.npm-degub.log
Copy link
Contributor

Choose a reason for hiding this comment

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

should it be "debug"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doh!!

@theefer
Copy link
Contributor

theefer commented Feb 13, 2015

Not entirely sure I understand the use case, but the code looks right! 👍

var events = eventName.split(':');
while(events.length){
var currentEvent = events.join(':');
var listeners = this._listeners[currentEvent] || Immutable.Set();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this is quite right, I think the scope of this statement (due to the initialisation) is outside the while loop? We don't want the listeners to vary during the execution right?

@rrees
Copy link
Contributor

rrees commented Feb 13, 2015

@cutandpastey Excuse my ignorance but why do you want the sub-events fired?

If I pass in 'scribe:content-changed' why would I want to fire an event called 'scribe'?

In my mind this was more that the Scribe EventEmitter would add a namespace to the events being fired if one wasn't provided. So if I sent 'hello' to the Scribe EventEmitter then it would emerge as 'scribe:hello'

@rrees
Copy link
Contributor

rrees commented Feb 13, 2015

Good work on getting unit-testing working, it's going to make things much easier in future 👏

@cutandpastey
Copy link
Contributor Author

So in noting I want to fire something like note:toggle-all:action. This way all the noting controllers can collapse/toggle all their respective notes and I can have ONE handler on note:toggle-all which inverts the note collapse state. This will contain the actions within the controllers and contain state management to the global.

@rrees
Copy link
Contributor

rrees commented Feb 13, 2015

@cutandpastey Okay I understand so this is actually like a fountain event rather than namespacing?

@hmgibson23
Copy link
Contributor

👍

@rrees
Copy link
Contributor

rrees commented Feb 16, 2015

👍 with some wiki documentation and a green build!

@cutandpastey
Copy link
Contributor Author

Is it me or is it impossible to re-run a travis build without pushing a commit?

@rrees
Copy link
Contributor

rrees commented Feb 16, 2015

@cutandpastey You should see a refresh button on your "Job" status page you need to have logged in via your Github account to get the permissions though

@cutandpastey
Copy link
Contributor Author

@rrees let me know if this is all good: https://github.com/guardian/scribe/wiki/Components

Now its time to mash that Travis button

@rrees
Copy link
Contributor

rrees commented Feb 16, 2015

@cutandpastey Looks good :shipit:

cutandpastey added a commit that referenced this pull request Feb 16, 2015
@cutandpastey cutandpastey merged commit 87caa3a into master Feb 16, 2015
@jukecraft jukecraft deleted the jp-event-namespace branch September 4, 2018 11:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants