Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding oneveryevent callback #28

Closed
wants to merge 1 commit into from
Closed

Adding oneveryevent callback #28

wants to merge 1 commit into from

Conversation

mstefaniuk
Copy link

No description provided.

@jakesgordon
Copy link
Owner

Whats the use case for this feature? I want to try to avoid adding to the base library unless there is a common use-case (e.g. not just a specific one of edge case)

I'm also a little wary because for a SPECIFIC event, we have both before and after callbacks and this change is really implementing a GENERIC after event callback only, but then someone will request a generic before event callback....

So if you could add a bit more of a description please, and also at least one qunit test for the new functionality.

Then these are mostly cosmetic, but 'broken-windows' theory and all...

  • The call inside the NOTRANSITION block should use this as first argument (instead of fsm)
  • Rename oneveryevent to onfireevent - that way we can think of it as the event-level equivalent of onchangestate
  • The everyevent declaration should line up cleanly with its sibling declarations (changestate, enterstate, leavestate etc)

@mstefaniuk
Copy link
Author

I've sent you an email with description of my use case but I would repeat it there. I have UI with several states where all but one events should dispatch timeout event when user do not take any action. I have few events that not cause transition from state to state so onstatechange don't fit. All but one events should reset timeout. This is my use case.

@Emerson
Copy link

Emerson commented Dec 6, 2012

+1 for this request.

Maybe I'm misunderstanding, but is there currently a callback that is fired during every transition? Could we see some example code if there is? (nm, just noticed the onchangestate callback...)

In more detail, it would be nice to have callbacks for general transition events:

callbacks: {

    onenterAny: function(args...) {
        console.log('entering a state');
    },

    onleaveAny: function(args...) {
        console.log('leaving a state');
    }

}

I think there are tons of use cases for this. For example, what if I need to reinitialize a javascript modal on every step, but don't want to duplicate my code? Or perhaps I need to fire off an ajax request for every state and then append it to the page. The general onchange callback might not be fired at the correct moment.

jakesgordon added a commit that referenced this pull request Jan 26, 2013
… and state transitions (instead of having a callback per-event/state)

 * onbeforeevent
 * onleavestate
 * onenterstate
 * onafterevent

E.g.

 * `onbeforeevent` is called for every event, while `onbeforeGO`   is only called before the GO event.
 * `onleavestate`  is called for every state, while `onleaveRED`   is only called when leaving the RED state.
 * `onenterstate`  is called for every state, while `onenterGREEN` is only called when entering the GREEN state.
 * `onafterevent`  is called for every event, while `onafterGO`    is only called after the GO event.

NOTE: deprecated the legacy `onchangestate` callback (its the same as `onenterstate`)
@jakesgordon
Copy link
Owner

Done!

You can now declare 4 generic callbacks that will trigger for all events and all state changes.

  • onbeforeevent
  • onleavestate
  • onenterstate
  • onafterevent

Which mimic the existing specialized callbacks which simply replace the word 'event' or 'state' with the actual name of a specific event or state.

see the "Callbacks" section of the README for more details.

@Emerson
Copy link

Emerson commented Jan 27, 2013

Amazing! So happy to have this addition to the library!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants