Skip to content

Ability to reject a state transition #14

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

Closed
timkindberg opened this issue Feb 14, 2013 · 14 comments
Closed

Ability to reject a state transition #14

timkindberg opened this issue Feb 14, 2013 · 14 comments
Assignees

Comments

@timkindberg
Copy link
Contributor

Look at the ability to reject state transitions, even though in my mind this has some caveats

  • If the transition is initiated via URL, rejection should really result in a redirect to some other (valid) URL rather than just leaving $location out of sync
  • If it's done via code, why is it happening in the first place
@ghost ghost assigned ksperling Feb 14, 2013
@jeme
Copy link
Contributor

jeme commented Feb 15, 2013

Ill pull something from our discussion over here then...

This is what I added over there...

    .state('contacts.list', {
      // parent: 'contacts',
      url: '',
      templateUrl: 'contacts.list.html',
      actions: {
        open: function( ... any dependency needed ... ) { $state.transitionTo('contacts.detail'); },
        close: function( ... any dependency needed ... ) { $state.transitionTo('home???'); }
      }
    })

Alternatively specifying transition handlers maybe...

   .state()
   .transistion('contacts.list', 'contacts.category', function ( ... any dependency needed ... ) {   });

Maybe that needs to be elaborated a bit in terms of Pre, In and Post... (before, between, after)...

Consensus

The first one is somewhat Crap in this API, but included it because it was a first thought...
But there might be something leaving a good amount of flexibility in the second one.

.transistion('contacts', 'contacts.category', {
    before: function ( ... any dependency needed ... ) {   },
    between: function ( ... any dependency needed ... ) {   },
    after: function ( ... any dependency needed ... ) {   }
})
.transistion('contacts.list', 'contacts.category', {
    between: function ( ... any dependency needed ... ) {   },
})
.transistion('contacts.category', 'contacts.list', {
    before: function ( ... any dependency needed ... ) {   },
    after: function ( ... any dependency needed ... ) {   }
})

It gives the ability to "obt-in" on specific state transition while leaving others un-touched... Then we properly need to discuss what that means to the current template/controller flow...

To abort a transition or return to the previous state, obviously those functions should be able to somehow abort... do another transition or something else...

But I am not sure if that could solve this... or If there is a simpler/more intuitive approach... In that case this should properly be in another issue as I would like to keep it alive for now...

@timkindberg
Copy link
Contributor Author

Whatever we may do with these transitions we need to consider how they play with onEnter and onExit and think about unifying the two concepts.

@nateabele
Copy link
Contributor

One thing I'd add to the above: we're going to have much more terse definitions if we're able to reason about multiple states and transitions at a time. For example:

.transition(["contacts.list", "contacts.category"], "contacts", {
    before: function(...) {},
    between: function(...) {},
    after: function(...) {}
})
.transition("contacts", "*", {
    before: function(...) {},
    between: function(...) {},
    after: function(...) {}
})

With something like the above, we wouldn't really even need an onEnter/onExit. Like @jeme said, we should probably spin this syntax discussion off into a separate issue.

If it's done via code, why is it happening in the first place

The traditional way to build a state machine is by defining events that specify valid state transitions. Using proper definitions, it is possible to determine which transitions are valid for each state, and preventing the state machine from reaching an unstable or unexpected state. I'll elaborate in a comment on #15, since that's the more relevant issue.

@michaelwinser
Copy link

I'm not sure that I agree. I do like keeping as much of the state logic
together. However, the benefit of an event based approach is that
components that aren't directly involved in the state transition still have
things they need to do around state transitions.

Perhaps there's a fundamental difference between the application state
logic and the events that components receive about those changes.

Michael

On Fri, Feb 15, 2013 at 9:41 AM, Nate Abele notifications@github.comwrote:

One thing I'd add to the above: we're going to have much more terse
definitions if we're able to reason about multiple states and transitions
at a time. For example:

.transition(["contacts.list", "contacts.category"], "contacts", {
before: function(...) {},
between: function(...) {},
after: function(...) {}}).transition("contacts", "*", {
before: function(...) {},
between: function(...) {},
after: function(...) {}})

With something like the above, we wouldn't really even need an onEnter/
onExit. Like @jeme https://github.com/jeme said, we should probably
spin this syntax discussion off into a separate issue.

If it's done via code, why is it happening in the first place

The traditional way to build a state machine is by defining events that
specify valid state transitions. Using proper definitions, it is possible
to determine which transitions are valid for each state, and preventing the
state machine from reaching an unstable or unexpected state. I'll elaborate
in a comment on #15 https://github.com/angular-ui/router/issues/15,
since that's the more relevant issue.


Reply to this email directly or view it on GitHubhttps://github.com/angular-ui/router/issues/14#issuecomment-13609347.

@ksperling
Copy link
Contributor

I think we need some real use cases for what these events would actually be used for. Off the top of my head

  • onEnter might be used to mark an item (e.g. a message or comment) as viewed by the user
  • onExit might be used to save changes that have been made

I think actions that the user triggers explicitly (e.g. delete this item, reply to this, ...) I'd think would still be implemented as a method on a controller which does some work and then calls $state.transitionTo() if necessary to navigate as well. But I'd see the state transition as a side effect of the explicit action, not the other way around.

@timkindberg
Copy link
Contributor Author

Maybe we should just be dispatching various events and let other components, views, scopes, etc do what they need to do. So we'll dispatch as many events that we think anyone would ever need and then document them.

@ksperling
Copy link
Contributor

There's currently a $stateChangeStart and $stateChangeSuccess event (similar to what $route has). They need some refinement in terms of what gets passed to the events, but should be a reasonable starting point for other components that want to just observe state changes without influencing them

@timkindberg
Copy link
Contributor Author

Would we be able to do a preventDefault (or preventStateChange) in a $stateChangeStart event listener?

@ksperling
Copy link
Contributor

Yeah, or maybe we could have a custom property on the event that the listener can write to, like e.redirect = ... which would reject the transition and perform that redirect instead.

Does anybody have any use cases for rejected transitions, other than "ensure the user is logged in" (which I think is probably better handled differently)?

@timkindberg
Copy link
Contributor Author

@ludinov had mentioned stopping them from leaving an unfinished form. Really any sort of unfinished work scenario ("are you sure you want to leave?"). May all the unfinished work scenarios would be handled differently too.

Also what about if a normal user tries to access a state only for admins?

On normal hrefs you can stop them by returning false. So if we have a ui-state-ref, people may assume they can prevent that too. I like the e.redirect, but you may not always want to redirect them somewhere else, you may just want to keep them in the same state.

@ksperling
Copy link
Contributor

I think access control would generally boil down to you making an ajax call to load or do something the user has no permission to do, in which case you can either throw an exception, which would cause the state transition to fail (if the call indirectly came from a 'resolve' property), or you can pop up a modal over the top of the page asking the user to log in, and then repeat the call when they do. The transition will then complete as normal -- it just took a bit longer because the user had to authenticate inbetween.

With an href, you're of course not really cancelling the href itself, all you can do is have a click handler and call preventdefault there. Once the browser itself has decided to navigate, that new href is going into window.location and there's nothing you can do.

Maybe when a transition gets rejected without an explicit redirect we can just reset $location to the URL for the current state.

Note that the "unfinished work" use case is really just a use-case for onExit(), as generally you'll ask the same question no matter where the user is going next.

@ksperling
Copy link
Contributor

This is of course assuming that there is a current state -- what do you do if the user does a fresh page load directly to an invalid URL?

@jeme
Copy link
Contributor

jeme commented Feb 18, 2013

This is of course assuming that there is a current state -- what do you do if the user does a fresh page load directly to an invalid URL?

That depends on what you mean by "invalid url"... If it's a "non-existing" resource, I redirect to a 404 state. (and so I have defined error states)... But as a simplification I choose to always have a state, so on a fresh page (re)load I inject a "inert" state which is then the "from" state.

ksperling added a commit that referenced this issue Mar 8, 2013
Support for specifying state.params explicitly
Pass stateParams to $stateChange* events
Support preventDefault on $stateChangeStart (#14)
Also more tests (#41)
@timkindberg
Copy link
Contributor Author

Looks like this can be closed, right @ksperling?

@christopherthielen christopherthielen removed this from the 1.5.0 milestone Nov 16, 2014
nateabele added a commit that referenced this issue Jun 26, 2015
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

No branches or pull requests

6 participants