Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

TemplateView event handling #382

Closed
rvalle opened this Issue · 16 comments

6 participants

@rvalle

It is possible to handle events from SC.TemplateView, for example:

      click: function(){
          App.statechart.sendAction('doSomenthigCool');
          return YES;
      }

However, this does not work as expected:

      click: function(){
          if(mustHandleThisEvent){
             App.statechart('doSomethingEvenCooler');
             return YES;
          }
          else return NO;
      }

Apparently, the event is not been propagated up in the view hierarchy for parentViews to have the opportunity to handle it.

@etgryphon
Owner

Two things:

#1: You're template view should prop the event up the chain, So I think you found a bug.
#2: The example you give here should be done all in the statechart. You shouldn't add a lot of application logic in your event handling in a view. That is what the statechart is for.

@FrozenCanuck FrozenCanuck reopened this
@FrozenCanuck
Collaborator

Whoa. I think I hit a wrong button or something when trying to send my original comment. Anywho...

So, first, I'm not sure what App.statechart('doSomethingEvenCooler') is. Did you mean App.statechart.sendAction('doSomethingEvenCooler')?

@rvalle

yeah, sorry for the mistake, I was just trying to illustrate the use case.

I am basically capturing events from templates that I turn into actions for the statechart. The templates are, to a certain point, complex, therefore I use classNames to know what action to trigger and/or whether an action should be triggered at all.

When no action is to be triggered, I cannot let the event go.

I understand from the suggestion 2 that I could just let the event go all the way up to the pane where the statechart would be attached and the handle the event straight at the statechart.

@FrozenCanuck
Collaborator

Ah, okay. I see what you're up to now. When constructing you view and triggering events you need to be mindful of a few things.

The first and most important thing to always remember is that a view should never be directly coupled to some external object. That hard coupling makes if harder to maintain the view, makes the view less flexible to change, and the view becomes difficult to reuse. So the explicit call to the App.statechart should be seen as a no-no.

Next, when you want to convert an browser event into application specific actions, it's best to define your view's interface so that you can set a target and action that are used to fire a custom action against a specific target. In addition to the target and action pair, your view should allow actions to filter through the SC root responder since it will go through the appropriate channels in order to route your action to the right target. This is exactly what the SC.ButtonView does.

Finally, if you need to provide more fine-grain control of when or when not an action should be fired, consider the use of a delegate so that an external entity is given the opportunity in which to help decided whether the view should actually fire an action against a target. Again, it's best that your let your view be flexible in how it behaves rather than your view always dictating terms.

Note that the application's statechart should not handle raw browser events. That is for the view's to handle. View's are to convert raw browser events into application specific actions.


http://frozencanuck.wordpress.com/2011/02/03/sproutcore-making-use-of-delegation/

@etgryphon
Owner

McDelegate strikes again... ;)

What do you mean by 'statechart should not handle raw browser events'...like 'click'? you can internally target actions fine.

@erichocean

"Note that the application's statechart should not handle raw browser events."

Okay, this made me laugh, since you're the guy telling everyone to set up the defaultResponder on their pane's to point to the statechart so that every mouse and keyboard event that ever happens gets sent to the application's statechart.

@FrozenCanuck
Collaborator

That's certainly a colorful response.

So, yes, I do say the root responder's defaultResponder should be hooked up to the app's statechart in order to handle actions that come by way of the root responder's sendAction method. The events that are handled by views (mouse up, mouse down, key up, etc) can propagate all the way up to the pane, and, if set, its default responder. However, they do not make there way over to the app's statechart so long as the app's statechart is not the default responder of the app's panes. Therefore, given such a scenario, it is not the case that every mouse and keyboard event that ever happens gets set to the application's statechart. If you do set the app statechart as a pane's default responder, then, yes, that will clearly happen.

Even if a pane's default responder is the app's statechart, the applicability of the app's statechart explicitly handling such browser events is not common, say, perhaps outside of key commands using meta keys. Even if you do decide to allow your app's statechart to explicitly handle browser events like click, mouse up, etc, you then effectively externalize some or all of view's responsibility to handle such events. This then couples part or perhaps all of a view's internal event handling behavior with the app's statechart, which is not ideal. If the app's statechart is to have a hand in any view's event handling logic, then it should ideally be reflected through the view's interface, such as its public properties, or via some form of delegation.

At the end of the day these are principals I happen to follow in order to help create cleaner separation of concerns, reduce coupling, and improve maintainability. That being said, I'm always open to learning about approaches others may have that tend towards the same goals.

@erichocean

"So, yes, I do say the root responder's defaultResponder should be hooked up to the app's statechart in order to handle actions that come by way of the root responder's sendAction method."

I think you should clarify that more often. The example code I have seen from you IIRC showed you hooking up the defaultResponder property on the main pane to the app's statechart. If people are instead doing this:

SC.RootResponder.responder.set('defaultResponder', 'MyApp.statechart');

Then we are in agreement as to best practice.

It's possible I've misread (or misremembered) your posts on the matter as well, but googling for 'SC.RootResponder.responder' on your blog did not reveal any statechart articles with that phrase mentioned. The one article with both statechart-related content and SC.RootResponder.responder showed you setting the defaultResponder on the SC.MainPane instance to the app's statechart (http://frozencanuck.wordpress.com/2011/02/03/sproutcore-making-use-of-delegation/).

FWIW, I think you would notice the error more quickly if the tracing output on statecharts could be configured to display actions/events that were not handled. In general, tracing can be dramatically improved in Ki and I hope that at some point you will get the opportunity/itch to do so.

@FrozenCanuck
Collaborator

I plead guilty to such information in the past. This sometimes happens when I bang out information quickly and not scrutinize it as carefully as I should. But, when using a statechart to represent an application states, it should generally be:

SC.RootResponder.responder.set('defaultResponder', 'MyApp.statechart'); 

I will be sure to go back and make corrections where I can.

As a side note, I'm always open to constructive feedback whether its for the blog posts I write or the code I create. I don't claim perfection as much as I try to strive for it :-).

@erichocean

No worries. I'd just had to correct a few mentoring clients on this issue and found it funny when you specifically mentioned how you wanted to do things the exact opposite of how you were doing them.

I'm a happy Ki customer otherwise. :)

@rvalle

Nice to have triggered such an interesting thread.

Thanks for your comments.

@wagenet

So is this a bug or not a bug?

@tomdale tomdale was assigned
@FrozenCanuck
Collaborator

Not a bug. Will close.

@rvalle

Are you sure? According to egyptron point 1.

The BUG makes impossible to implement the delegate pattern at TemplateView (which is the right way to go as discussed), because if an event is handled by the delegate won't be possible to discard the event and have it up the chain.

This bug stops TemplatesViews to be used as Controls, which could be convenient.

Or may be I am wrong...

@FrozenCanuck
Collaborator

I may have incorrectly analyzed the code snippet you supplied. Would it be possible for you to provide more code? Is your template view mixing in the SC.StatechartManager? Apologies for causing ya some headaches :-P.

@rvalle

No worries, it's all fine!

As far as I understand the main issue is that:

    App.MyView=SC.TemplateView.design({
      click:function(){
        return NO;
      }
    });

Should propagate the Event up in the chain and it doesn't. Any view should do this. I don't think the problem is related to the StatechartManager.

@rvalle rvalle reopened this
@rvalle rvalle closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.