sproutcore / abbot

SproutCore Build Tools [deprecated]
http://www.sproutcore.com
88 stars 44 forks source link

Statecharts by default when running sc-init #76

Closed TheCodeBoutique closed 12 years ago

TheCodeBoutique commented 12 years ago

Modified sc-init to have statecharts out of the box.

publickeating commented 12 years ago

I actually do just about this exact thing when starting a new project, so personally I like it. I will try to get FrozenCanuck to comment about the exact default states that should be included since he is the guru and he may give us some insight on even more default states to include, like Start, Loading, Ready, etc.

I do wonder about forcing state charts on a brand new developer, since the barrier to entry is already high. It’s a whole new set of theory they’ll have to grapple with. Anyway, I’m in favour of it, but I’d like to sleep on it for a bit and gather some feedback from the community. Ping me if you don’t hear anything after a week. Thanks a lot.

geoffreyd commented 12 years ago

Will this always run against the latest version? I know some people who use the latest abbot with older SC's. Maybe include a check to see if statecharts exist.

TheCodeBoutique commented 12 years ago

Tyler: I do agree the learning curve is already high. However, I am a firm believe that introducing the framework with statecharts by default will help an engineer who is not use to using them in their apps daily grow. The hardest part for some people that I know was not statecharts but starting to use them.

Geoffrey: Could statecharts be back ported? From what I've seen today. SC 1.4.5 has statecharts in its framework dir but a 1.4.5 app with statecharts did throw an undefined error.

sevifives commented 12 years ago

I'd love if every app had statecharts, but I agree with Keating: statecharts are a big step to take. The other matter is that you're creating a default state structure that I wouldn't necessarily use.

charlesjolley commented 12 years ago

@TheCodeBoutique maybe you could modify this pull request so that including the statechart for new templates is available as a command line option?

sc-init MyAll --statechart

Would be cool and a good first step.

TheCodeBoutique commented 12 years ago

Good idea. I did not think of doing it with a switch.

TheCodeBoutique commented 12 years ago

UPDATE: sc-init now has a states switch.

sc-init AppName (builds the standard app) sc-init AppName --template (builds the sc app with templates) sc-init AppName --states (builds the sc app with state charts)

ChadEubanks commented 12 years ago

@FrozenCanuck How do you feel about adding a state switch to abbots' sc-init? Specifically, what do you feel would be the best way to setup default states in a new app?

FrozenCanuck commented 12 years ago

Hey Chad,

First, I'm in agreement with updating abbot so that the sc-init tool can automatically add statechart support when using the optional --states flag. As has been pointed out, people new to SproutCore will likely find statecharts a heady topic when they're just trying to get themselves originated with SC in general, so having added statechart support as an optional add-on using --states makes the most sense. Okay, now on to your commits.

A few things. First, regarding the statechart itself, my personal preference is to have a statechart.js in the root folder simply because the application's statechart is such a prominent feature.

Second, in the statechart file, you explicitly set the root state, which you no longer need to do. The root state is automatically created if you don't explicitly provide one, and all of the states directly assigned to the statechart are added as substates to the root state.

Finally, I typically don't name states on a per SC.Page instance basis. Rather, I name states based on the general features and flow of the application. I understand what you're doing as you'd like to have a specific state responsible for each page which in turn represents some feature, and that's fine if that's how you like to set things up. In addition, the MainPageState has a substate called loadMainPageState, which doesn't really make sense. What are you loading? When the loading is done what happens next? Are you then ready? The main pane in the main page is already loaded when you download the app. Really you are displaying the main pane when you begin the application thereby making is ready for use. So instead it would be more advisable to simply have a basic ready state where upon entering the state you display the app's main pane.

Basically you'd end up having the following:

// statechart.js
MyApp.statechart = SC.Statechart.create({

  initialState: 'readyState',

  readyState: SC.State.plugin('MyApp.ReadyState'),

  // someOtherState: SC.State.plugin('MyApp.SomeOtherState')

});  

// states/ready_state.js
MyApp.ReadyState = SC.State.extend({

  enterState: function() {
    MyApp.getPath('mainPage.mainPane').append();
  },

  exitState: function() {
    MyApp.getPath('mainPage.mainPane').remove();
  }

});

Much simpler and cleaner. Also notice that the properties on the statechart and states classes corresponding to instance of states have an initial lowercase letter instead of uppercase, which follows normal property naming conventions. The suffix "State" is enough to indicate the property is referring to a state object.

One other thing you must do is update the main.js file. The main function initializes the statechart but the statechart is not hooked up to the app's root responder in order to receive application actions. The code should be the following:

MyApp.main = function main() {

  // Step 1: Tell your app it will load via states
  var statechart = MyApp.statechart;
  SC.RootResponder.responder.set('defaultResponder', statechart); 
  statechart.initStatechart();

  // Step 2. Set the content property on your primary controller.
  // This will make your app come alive!

  // TODO: Set the content property on your primary controller
  // ex: MyApp.contactsController.set('content', MyApp.contacts);

} ;

If you can make all these necessary changes then it'll be ready to be pulled.

Thanks for taking this on. Much appreciated :)

ChadEubanks commented 12 years ago

@FrozenCanuck @publickeating

Last commit proposes sc-init to have a new switch: sc-init appName --statechart with proper logic.

Note: the statecharts.js file needed the initial state and an initial substate. Without this I would receive the following error: state SC.State<ReadyState, sc700> has not initial substate defined. WIll default to using an empty state as initial substate. All though the console says this is a warning. The app would not load the mainPage.mainPane.

FrozenCanuck commented 12 years ago

@TheCodeBoutique

Correct, you will get an warning in addition to an default empty state if you do not set the initialSubstate property within an SC.State; however, in the SC.Statechart you just set the initialState property, like you did when you set it to be rootState. Again, the root state no longer needs to be explicit defined, and in the most cases you can just let the statechart implicitly construct it for you. As well, the names of properties referencing a state do not require a initial capital letter; a lowercase will suffice just like for all other properties not indicating the declaration of SC class. If you can make the following changes then your commit should be fine and we can make the pull:

  1. Place the statechart.js file within the app's root directory
  2. Set up the statechart with just an initial ready state (no root state)
  3. Make property ReadyState defined on the statechart to be readyState

Again, thanks for doing this.

ChadEubanks commented 12 years ago

@FrozenCanuck @publickeating

Should be good to go now.

ChadEubanks commented 12 years ago

@FrozenCanuck

How about now?

FrozenCanuck commented 12 years ago

Yep, looks good. If you can, adding some descriptive comments to the auto generated files that helpfully explain the statechart and ready state that would be icing on the cake, but this is fine. Thanks!

ChadEubanks commented 12 years ago

@FrozenCanuck

Created a statechart_project directory that is called when passing the --statechart switch. This has a statechart specific description that prints to screen when generating the sproutcore app.

ChadEubanks commented 12 years ago

@publickeating

Tyler, this should be good to pull into master now.

publickeating commented 12 years ago

Awesome. Thanks!