chaplinjs / chaplin

HTML5 application architecture using Backbone.js
http://chaplinjs.org
Other
2.85k stars 231 forks source link

Keep beforeAction for shared code between methods #765

Open karellm opened 10 years ago

karellm commented 10 years ago

Hi,

I've seen this in the exemple app/docs:

class MyController extends Controller
    beforeAction: (route, params) ->
        # some shared code for all methods
        if route.method is 'method1'
            # do something

    method1: ->
        # render method 1

What do you guys think of having a beforeMethod1 method in the controller?

class MyController extends Controller
    beforeAction: (route, params) ->
        # some shared code for all methods

    beforeMethod1: ->
        # render method 1

    method1: ->
        # render method 1

I like that the logic for method1 stays grouped with method1. Plus it doesn't break current implementations.

paulmillr commented 10 years ago

This is not the primary use case for that. You don't need beforeMethod1, you can do this stuff in method1.

karellm commented 10 years ago

Ok maybe I am mistaken here. Isn't on major benefit of the beforeAction to make sure data is available before rendering?

I could see this useful to implement transition between views. We can ensure before* is done before initiating the transition.

For instance, do you consider this piece of code following the Chaplin way?

beforeAction: (params, route) ->
    $.when(super).then =>
        if route.action is 'signup'
            @promise = $.ajax # an api call

    if @promise and @promise.always
        @promise.always () => @publishEvent 'hidePageLoading'
    else
        @publishEvent 'hidePageLoading'

    return @promise

signup: (params) ->
    @promise.done (data) =>
        # new view

    @promise.fail (error) =>
        # handle error
paulmillr commented 10 years ago

Isn't on major benefit of the beforeAction to make sure data is available before rendering?

As a some universal way of checking this — maybe, but I never used it like that.

Personally in apps I develop I don't handle fetch errors much. If something is wrong then it's really wrong and a user should report to site admin. As for loading indicators, i'd rather did them in views.

karellm commented 10 years ago

In this case it is not really a fetch but a verification to an api. I have to handle the error, to either redirect or display an error message.

I'm curious to know how you handle transitions though. In my case, I have this @publishEvent 'hidePageLoading' that will hide a loader that cover the entire page on initial page load. Though I haven't get to implement nice transition between route changes after that (it is often hard to notice the route actually changed).

paulmillr commented 10 years ago

We don't handle them...transitions are long time in TODO list, they will be awesome to implement some day!

karellm commented 10 years ago

1/ How do you handle the data fetching then? I guess for models you rely on the new SyncMachine but no all data and async calls may be tied to a model. Do you have a sample code of this?

2/ Is there an draft on transitions already? I will need transitions in my app so I'd be interested in tackling this feature.

paulmillr commented 10 years ago

it's like this all the time:

action: (params) ->
  @model = new User {id: params.id}
  @view = new UserView {@model}
  @model.fetch().then @view.render

see gh-105