Further Development #9
Conversation
…ore like Rails’ routes.rb
…Controller. Let them communicate via Pub/Sub. AppView is responsible for all view changes while the AppController is responsible for startup and disposal of controllers.
Use mediator.setUser to write the user. This should avoid accidental overwriting while allowing to read the user easily. Don’t know yet if that is necessary or a good solution. Properly test for ES5 support.
|
Why |
| require ['controllers/' + controllerFileName], handler | ||
|
|
||
| # Handler for the controller lazy-loading | ||
| controllerLoaded: (controllerName, action, params, ControllerConstructor) -> |
There was a problem hiding this comment.
This is an excellent refactor. This movement is correct, application_view was doing things that usually the app controller should do.
So, following a common App Controller pattern, I think this method should be called execute().
|
Don’t like the idea of having user on the mediator either. But I cannot think of a solution which ensures that the user is globally and easily accessible. A mediator doesn’t have to be limited to Pub/Sub in my opinion, the mediator object might also store data which might be shared between modules. Of course we could have, for example, a Since the I like the current mediator concept because it’s dead simple. Some central—data like the user—is shared on the mediator, the rest of the data may be shared using the mediator’s Pub/Sub mechanism (or by introducing additional special-purpose mediators). So most modules already include the mediator and can access the user. Ideally there should be a |
|
What I was suggesting is that controllers receive the user instance, so they can also handle it to their views on initialization. |
|
Sure, but the user may change during the lifetime of a module. I don’t see the advantage of passing the user from AppController down to models and views and save it there. The user might be |
…l be an exception anyway. Controller#dispose is always defined by controller.coffee.
|
Yeah. The only way is to not store the user and initialize everything passing the user as argument, and then rely on mediator.subscribe 'login' as usual. |
In the example collections, overwrite the fetch method and use that. Properly reset the collections on logout.
|
@paulmillr The excessive debug output was used during development and bugfixing. They are fine for this purpose. But for someone who’s reading and reusing the code, they are useless and just pollute the code. Sometimes they need to be replacing by better comments. |
It seems to be valuable for some people. I’ll remove detailed debug output soon but leave the interesting console calls intact (initialize, render and so on). This reverts commit 9a35ffe. Conflicts: coffee/models/likes.coffee coffee/models/posts.coffee
|
Cool. Is this going to be stable soon (I want to include it to brunch 1.0)? |
|
@paulmillr Sure, I’ll hope I can polish this up this next week. As a next step, I’d like to add a concept of persisting controllers (see issue #4). I think this is a important feature and we should at least provide a standard solution. |
Conflicts: coffee/lib/router.coffee
|
Thanks for the input, @cpsubrian, these are good ideas. I’ve read an article on the Rails logging today (http://www.paperplanes.de/2012/3/14/on-notifications-logsubscribers-and-bringing-sanity-to-rails-logging.html) and I agree with you that a Pub/Sub-based logging approach could help. I will also have to look at the socket.io logging you’ve mentioned. |
This will preserve comments in js source. Closes #30.
…`render`. `render` overwrites it anyway.
Collection: Improved the inserting algorithm so all items views are inserted after the non-view elements
Add twitter app example to readme
Conflicts: coffee/chaplin/views/collection_view.coffee js/chaplin/views/collection_view.js
Change console comments
- Standardize debug messages - Style: Replaced some standalone `@` with `this`
…ke in the collections
Remove most of the console.debug calls
Changed the README to reflect the current development
CollectionView: Remove unused method collectionIsSynced
…g DOM event handlers declaratively works again
- Remove duplicate call of getLoginStatus after loginAbort - Correctly pass the loginContext to loginStatusAfterAbort (fixes an exception when aborting the login) - Rename facebook_commment event to facebook:comment - Remove postToStream, you should not do this - Delete status and accessToken on disposal - Optimize event payload object
Re-enable View#delegateEvents
Cleanup the Facebook ServiceProvider
|
@molily lets merge this to master. dev branch is stable and we're not pushing new release anyway. |
|
because it can't be automatically merged, how about a simple renaming of branches? |
I made some changes on the core architecture as well as smaller changes and bundled them in this branch to discuss about them.
The most important thing is probably the split of the controller startup/disposal logic between ApplicationController and ApplicationView. I always felt that ApplicationView isn’t the right place since it’s not a view question, but a app-wide mechanism. ApplicationController as parent of other controllers makes more sense.
Second thing is the split between Router and routes. The routes used to lie in the Router class which is definitely not the right place. Now they are sourced out into a
routes.coffeewhich looks much like the Rails counterpartroutes.rb.Third thing is the mediator revamp.
mediator.useris now readonly in ES5-conformant browsers and the newmediator.setUsermethod should be used when setting the user. The idea is to prevent accidental overwriting. At least you will get a strict mode exception in ES5-capable browsers.What do you think?