angular-ui / ui-router

The de-facto solution to flexible routing with nested views in AngularJS
http://ui-router.github.io/
MIT License
13.53k stars 3k forks source link

RFC: Parallel states - Proof of Concept code #894

Closed christopherthielen closed 10 years ago

christopherthielen commented 10 years ago

Plunkr: http://plnkr.co/edit/IgePmCtVnojo19i3y6Ab?p=preview Updated Plunkr: http://plnkr.co/edit/YhQyPV?p=preview (2014-02-24, read comments posted on 2014-02-24) Ui-Router Fork: https://github.com/christopherthielen/ui-router

I have a "Tabs" use case for Parallel States in each tab. Basically, my company has an non-angular application which users sit in all day long. There are multiple components, each of which lives in a separate tab, and each of which has its own complex nested state. Users switch back and forth between tabs to perform their job.

For a semi-theoretical example, let's say there are three tabs. Tab 1 is always an inbox, calendar, and task list. Tab 2 allows the user to create, view and update records and follow workflows. Tab 3 is swapped out with a variety of tools that do business work, such as billing or requisitions. Tabs operate independently, but the user needs to refer back to related data in tabs 1 and 2.

I want routing/url management, however if the user bookmarks a URL, I only care about bookmarking that particular tab. I do not need the state of all 3 tabs stored in the URL; I only need the state of the current tab reflected.

I have developed a proof-of-concept that shows how this could be implemented in ui-router. In my proof-of-concept, a ui-view may be tagged with "parallel-state" and multiple ui-view(s) may be added to the parent state's template. When a state transition occurs between the parallel branches, the "exited" state's locals and DOM is retained.

The plunkr demonstrates the parallel routes, via a tabbed UI. Click around in the UI (select "Show inactive tabs" to get a better idea of what is happening)

I'd like to hear comments and opinions about the approach. I'm new to ui-router, so I'm sure I might have leaked or misused some abstractions that I didn't understand. I have not yet tested or accounted for all ui-router functionality, like state parameters and resolves.
However, does this look like a reasonable approach to parallel routes?

I'd reference the other parallel state issues, if I knew how. https://github.com/angular-ui/ui-router/issues/475 https://github.com/angular-ui/ui-router/issues/562 https://github.com/angular-ui/ui-router/issues/63 https://github.com/angular-ui/ui-router/issues/863

timkindberg commented 10 years ago

Just real quick... one of the coolest fucking things I've seen created with ui-router yet.

I am having trouble figuring out what features are coming from your modifications to the src and what features are coming from the boilerplate in your script.js plunkr file. Like, what could you do without all the extra helper functions everywhere? Or are those required? Obviously this is just an RFC, I'm just trying to get a feel for what is in included and what isn't.

So looking at it more. Seems like the parallel aspect is included in the src changes, so we get:

The boilerplate gets us:

I wonder if instead of using a parallel-state directive we just have an extra parameter on the config and tap into the named view feature that already exists, so the children say what view they plug into instead of the view saying what child state will plug into it.

In root.tabs template:

...
template: '  <div ng-show="isTabVisible(\'root.tabs.S1\')"><div id="root_tabs_ui-view_S1" ui-view="S1">Nothing Loaded</div></div>' +
...

Then in root.tabs.S1:

$stateProvider.state('root.tabs.S1', {
        controller: timerCtrl,
        template: 'whatever',
        scope: true,
        parallel: true,
        parallelView: 'S1',
        url: '/s1'
    });

I'd have more feedback but I'm curious what @nateabele, @MrOrz, and @empaempa think about it.

christopherthielen commented 10 years ago

Thanks for looking at this, and I'm glad you think it's cool. That hopefully means I'm on the right track.

Your analysis is correct. The modifications to ui-router allow parallel states that can be bound to ui-views. The module code implements the tabs state management and UI (i.e., it uses the parallel state support from the ui-router modification to implement parallel tabs).

I took a hint from some previous comments on the issue tracker that "tabs" themselves are outside the scope of ui-router, so I intentionally kept all the "tabby" stuff in my module. The controller for "root.tabs" contains most of the tabby logic and could (probably should) easily be converted to a directive. I have put about 12 hours into this so far, attempting to attack the problem from 4 or 5 different angles, so I wanted to make sure I'm on the right track before dumping more time into improvements.

Edit: Ooops, I misinterpreted your comment and made a huge reply based on my misinterpretation. I'm leaving this up for posterity

Regarding the markup convention I implemented, I tried a number of different mechanisms before settling on <ui-view parallel-state=".S1">.

Here are some thoughts:

Parallel states must be direct substates of the parent state. Your comment about the parallel-state attribute can be deconstructed to: How do you (a) declare the parallel states and (b) bind those states to ui-views from the parent view.

In my example, parallel-state=".S1" isn't actually a directive, it's just a data attribute that ui-view uses. I had at first used <ui-view=".S1"> but I thought that didn't provide an explicit "parallel" marker for readers of the markup, since parallel views are significantly different from <ui-view> and <ui-view="viewname">. When declaring a state with multiple named view references (note: not parallel states), those views are part of the state, as opposed to a "raw" <ui-view> which creates a placeholder for the next nested state to attach. <ui-view parallel-state=".S1"> is a hybrid of the two in that it uses the "raw" ui-view but is still parameterized, and doesn't confuse me as to what should happen. The "raw" nature of the ui-view tag (<ui-view parallel-state=".S1">) hints to me that it is not a placeholder for a view within the current state, but rather a placeholder for a child state. All that said, however, <ui-view=".S1"> feels reasonable to me as well.

I don't like that the ui-view in my proof-of-concept is explicitly referencing a child state as opposed to a view, which I think is your objection as well. My rationalization for referencing the state as ".S1" is that a raw <ui-view> is referencing the child state, implicitly (I realize this isn't really a good argument since multiple different child states could be loaded into that <ui-view>).

If we ran with your suggestion, parallel: true and parallelView: 'S1' are redundant. If we used only parallelView: "substate1" and then <ui-view=".substate1"> (note the dot notation to denote sub-state) would that smell better to you?

<ui-view=".substate1">

$stateProvider.state('root.tabs.S1', {
        controller: timerCtrl,
        template: 'whatever',
        scope: true,
        parallelView: 'substate1',
        url: '/s1'
    });

Either way, I am definitely trying to link the ui-view to a specific substate, whether that is direct by referencing the substate name, or via a parallelView name (unique among all the child states) found by checking all parallel child states.

Food for thought, here are some of my failed approaches: (1) My first approach was to try with a directive that canceled $stateChangeSuccess event propagation. (2) My second approach baked into ui-router a "parallel state container" concept, of which all direct substates were made parallel. This boiled down to creating a state (root.tabs) that was marked as something like parallelcontainer: true. I had problems accessing those substates' views when rebuilding the view tree (during directive compile and updateView() steps), but that was early in my exploration into ui-router and in hindsight, it may be an achievable approach.
(3) My third approach was a variation on my second, where I defined multiple named views on the parent state, as peers to standard named views. Those views then referenced the state they wanted loaded in that view. views: { S1: { state: 'root.tabs.S1' }, S2: { state: 'root.tabs.S2' } } Again, I had troubles loading the appropriate controller/template because I was trying to load locals, etc for children states and it didn't make sense.

Finally, I decided to try to embrace the currently built-in "raw" ui-view mechanism. I noticed that multiple <ui-view> <ui-view> would get loaded twice with the current state, so I tried short circuiting the load process when the parallel-state attribute didn't match the $current state. That way the correct state's ui-view is loaded when $current matches that state, but is not touched otherwise.

christopherthielen commented 10 years ago

Tim, I just realized I completely misunderstood the point of your suggestion. I'm leaving my last comment up for posterity, maybe there's something discussion worthy in it.

I wonder if instead of using a parallel-state directive we just have an extra parameter on the config and tap into the named view feature that already exists, so the children say what view they plug into instead of the view saying what child state will plug into it.

If I had read your comment more carefully I would have noticed the most important part.

Is there a precedent for child states going "up the chain"? The way I currently think of views and states is that the parent state chooses a view whose template is responsible for plugging in the child, via ui-view placement.

christopherthielen commented 10 years ago

I converted the tab handler to directives and changed the ui-view syntax to <div ui-view=".substate"> Updated plunkr: http://plnkr.co/edit/OlBZkZ?p=preview

The relevant markup now looks like:

  // The tab list state
  $stateProvider.state('root.tabs', {
    controller: function ($scope, $state, $timeout) {
      timerCtrl($scope, $state, $timeout);
      $scope.isStateActive = function (statename) {
        return $state.includes(statename);
      }
    },
    template: '<b>root.tabs</b>: Started {{delta}} seconds ago' +
            '<input ng-model="data" type="text">{{data}}' +
            '<br><input type="checkbox" ng-model="showInactiveTabs">Show inactive tabs' +
            '<ul class="tabs" parallel-state-controls>' +
            '   <li ng-class="{ active: isStateActive(\'root.tabs.S1\') }" parallel-state-selector=".S1">S1 (Parallel State 1)</li>' +
            '   <li ng-class="{ active: isStateActive(\'root.tabs.S2\') }" parallel-state-selector=".S2">S2 (Parallel State 2)</span>' +
            '</ul>' +
      // Here is where the parallel states are bound to the UI.  I'm using the .SUBSTATE nomenclature to
      // mark a ui-view as following a parallel state tree.
      // Note: Wrap the ui-view in a div because ng-show doesn't seem to work on a ui-view
            '  <div ng-show="showInactiveTabs || isStateActive(\'root.tabs.S1\')"><div id="root_tabs_ui-view_S1" ui-view=".S1">Nothing Loaded</div></div>' +
            '  <div ng-show="showInactiveTabs || isStateActive(\'root.tabs.S2\')"><div id="root_tabs_ui-view_S2" ui-view=".S2">Nothing Loaded</div></div>' +
            '',
    url: 'tabs'
  });

With all the fluff removed, here is the bare bones markup:

<ul parallel-state-controls>
   <li parallel-state-selector=".S1">S1 (Parallel State 1)</li>
   <li parallel-state-selector=".S2">S2 (Parallel State 2)</li>
</ul>
<div ui-view=".S1">Nothing Loaded</div>
<div ui-view=".S2">Nothing Loaded</div>
timkindberg commented 10 years ago

Ha I love when someone is so excited they write about 2000 words and 2 directives before I even get back.

So let me start with a disclaimer. UI-Router is in big flux right now with the bower issue, @nateabele has about 100 local refactors he has yet to commit, and several medium sized features that are still in the 'baking' process. With all of that, this is probably not on a high priority list (mainly because its large size), but its really cool and its something I know devs have wanted multiple times, so its a good thing to explore (and you seem to have the smarts to explore it properly).

Basically the less intrusive that the changes are in the source, and the less conceptual overhead it adds to ui-router, means the more likely it will get in quickly. @nateabele and I are really stubborn on API elegance (esp since there's already several concepts in ui-router that are not easy to grasp at first blush). Anyway, if that's all good with you we can continue to explore this.

Ok so your question from two comments up first:

Is there a precedent for child states going "up the chain"? The way I currently think of views and states is that the parent state chooses a view whose template is responsible for plugging in the child, via ui-view placement.

Yes I think this is how it currently works. A parent state template can define multiple named views as such:

<div ui-view='a'/>
<div ui-view='b'/>
<div ui-view='c'/>

But its the child state that decides how to fill those shells:

$stateProvider.state('child', {
  views: {
    'a': ...,
    'b': ...,
    'c': ...,
  }
})

To me, this is going 'upwards'.

Now for your comment above (with the new directives and markup). This does feel a lot better, perhaps we'd roll all the directives into the module. I'm not fully liking the ui-view=".S2" though. That's not what I'd originally proposed, but I don't like what i originally proposed either anymore. I kind of like using the 'pipe' as seen in #863, but we don't need anymore special syntaxes. I wonder if just using a new directive called ui-parallel-view would be a good option?

<ul parallel-state-controls>
   <li parallel-state-selector=".S1">S1 (Parallel State 1)</li>
   <li parallel-state-selector=".S2">S2 (Parallel State 2)</li>
</ul>
<div ui-parallel-view="S1">Nothing Loaded</div>
<div ui-parallel-view="S2">Nothing Loaded</div>

I removed the dot before the state names, but I'm not sure if that's good. They do help show that its children states when the dot is there. But then I think it may also make ppl feel as though they can use any relative or absolute state targeting string there (e.g. '^', '.child.grandchild', '^.sibling'); could they? Would they?

I think we need more brains in here.

christopherthielen commented 10 years ago

Yes I think this is how it currently works. A parent state template can define multiple named views as such: ... But its the child state that decides how to fill those shells:

Aah I see. I haven't used multiple named views, so I misunderstood what was going on with them.

You don't like ".S1", but is it primarily because of the potential user confusion on what they can put there? Of course those cases could be guarded during compile time, i.e.

          if (name.indexOf(".", 1) != -1 || name.indexOf("^") != -1)
            throw new Error("ui-view='" + name + "' illegal substate reference.  You can only reference direct child substates. ");

That said, the more I consider your idea, the more I agree it makes sense.

<ul parallel-state-controls>
   <li parallel-state-selector=".S1">S1 (Parallel State 1)</li>
   <li parallel-state-selector=".S2">S2 (Parallel State 2)</li>
</ul>
<div ui-parallel-view="S1">Nothing Loaded</div>
<div ui-parallel-view="S2">Nothing Loaded</div>

Is this more along the lines of what you're thinking?

var substate1 = {
   url: ..., name: ..., 
   views: {
     S1: { template: '', controller: function() {} }
   }
}
var substate2 = {
   url: ..., name: ..., 
   views: {
     S2: { template: '', controller: function() {} }
   }
}
timkindberg commented 10 years ago

Is this more along the lines of what you're thinking?

Yes it is. I'm still undecided if the substate should have to use the views property or if the ui-parallel-view directive can just specify the child state. Its more annoying to have to specify the views property but also more consistent API-wise with multiple named views.

I'll have to think more about this. Meanwhile hopefully some more people chime in before we go to far in any one direction.

christopherthielen commented 10 years ago

Updated plunkr: http://plnkr.co/edit/YhQyPV?p=preview

I've made some more progress on this:

I need this feature for a project I'd like to port to ui-router, so I'm going to continue plugging away (or should I say "plunk'ing away"?). Hopefully I'll get more feedback from the other players.

Tim, I took your disclaimer to heart:

So let me start with a disclaimer. UI-Router is in big flux right now with the bower issue, ... this is probably not on a high priority list (mainly because its large size), but its really cool and its something I know devs have wanted ...

Basically the less intrusive that the changes are in the source, and the less conceptual overhead it adds to ui-router, means the more likely it will get in quickly ... Anyway, if that's all good with you we can continue to explore this.

I revamped the way nested parallel states plug into their parent ui-view. I went with your suggestion about the parallel state having a named view which plugs into the matching named ui-view in the parent template. Now that I understand how regular named views work, this makes sense and seems consistent. It also simplified/removed a bunch of the code I was kludging before.

By extracting most of the code to a $parallelState, the integration points become simple and apparent. Have a look at my fork again. There is a lot of added code (plunkr code and parallelState.js), but the important conceptual changes are found in state.js and viewDirective.js.

The following 5 integration points are confined to three functions: registerState(), transitionTo(), updateView(). Check the diffs ( https://github.com/christopherthielen/ui-router/compare ) for exactly what changed in state.js and viewDirective.js. The 5 integration points are as follows:

1) during registerState, if the state is defined as parallel, then register it with $parallelState

  function registerState(state) { ...
    if (state.parallel) $parallelStateProvider.registerParallelState(state);

2) Reactivate parallel state

Normally, during transitionTo we check for the states that are "kept" and reuse those locals. Any non-kept additional substates are resolved by calling resolveState(). When those states are resolved, then the transition occurrs.

Instead, we now first check if we are transitioning back to a previously inactivated parallel state. If so, restore locals from $parallelState instead of calling resolveState(). Make a note that we did this by adding the state name to a local variable "restoredStates". We'll use that after the transition occurs to notify the states via onEnter/onReactivate

      var restoredStates = {}; // Used after all states are resolved to notify $parallelState
      for (var l=keep; l<toPath.length; l++, state=toPath[l]) {
        var restoredState = $parallelState.getInactivatedState(state, toParams);
        locals = toLocals[l] = (restoredState ? restoredState.locals : inherit(locals));
        if (restoredState) {
          restoredStates[state.name] = true;
        } else {
          resolved = resolveState(state, toParams, state === to, resolved, locals);
        }
      }

3) Register "exited" (inactivated) parallel states as inactivated

After transitioning states, we normally locate exited states and execute their callback "onExit".

Instead, check if we are "exiting" a parallel state that should continue to live as an inactivated parallel state. Instead of exiting the state and removing its locals, register the state as Inactivated via $parallelState, then execute the callback onInactivate (as opposed to onExit). Notify $parallelState of any states that were actually exited, so we can check if this orphans any descendant inactive parallel states.

        // Check if we are transitioning to a state in a different parallel state tree
        // fromPath[keep] will be the root of the parallel tree being exited
        var parallel = keep < fromPath.length && fromPath[keep].self.parallel;
        // Exit 'from' states not kept
        for (l=fromPath.length-1; l>=keep; l--) {
          exiting = fromPath[l];
          if (parallel) {
            $parallelState.stateInactivated(exiting);
          } else {
            $parallelState.stateExiting(exiting);
            if (exiting.self.onExit) {
              $injector.invoke(exiting.self.onExit, exiting.self, exiting.locals.globals);
            }
            exiting.locals = null;
          }
        }

4) Reactivate previously inactivated parallel state(s)

After transitioning state, we normally invoke the onEnter callback for any added states.

Instead, we first check if the newly added states were re-activated (via integration point #2, above). If so, notify the parallel state it has been reactivated, and remove it from the inactive parallel state registry

        // Enter 'to' states not kept
        for (l=keep; l<toPath.length; l++) {
          entering = toPath[l];
          entering.locals = toLocals[l];
          if (restoredStates[entering.self.name]) {
            $parallelState.stateReactivated(entering);
          } else if (entering.self.onEnter) {
            $injector.invoke(entering.self.onEnter, entering.self, entering.locals.globals);
          }
        }

5) Short circuit ui-view's updateView()

Normally, after a successful state transition, all ui-view(s) will update themselves. The ui-view might re-initialize itself (and thus, all its sub-views) if the state mapped to it has changed.

Instead, check if the state transition is changing from this parallel subtree (or "universe") to another peer parallel subtree. In other words, the state was on a substate of one parallel tab, then the state transitioned to another substate of a parallel tab, a peer tab to the first. If so, then short circuit the ui-view's updateView. This leaves the DOM intact and doesn't rebuild the view subtree.

        var eventHook = function (evt, toState, toParams) {
          if (viewIsUpdating || $parallelState.isChangeInParallelUniverse(view, evt, toState)) {
            return;
          }
timkindberg commented 10 years ago

Wow man! Just amazing. People are really going to like this and I really appreciate you working on this so thoroughly. This really epitomizes "contributing".

I know you've already put so much effort in and honestly it is usable the way it is, and not overly confusing (which is good for a new concept like parallel states), but I did have some more thoughts, but don't feel like you have to entertain every little whim that comes to my mind, but I'll say them anyway.

The two new directives (parallel-state-controls and parallel-state-selector) feel a bit odd to me. Now maybe this is because you felt you needed to keep them as separate from the src changes, but I'm really rethinking that notion. Now if we keep them that's fine—that was actually another side question I had: were you thinking that these directives would be part of ui-router core? Anyway, these two directives really just do a couple things (from a end users perspective); 1) parallel-state-controls allows you to do a 'deep' reactivation and 2) parallel-state-selector just activates a parallel state.

I was thinking instead of parallel-state-controls, we could just have another state config option called retainDeep: true, which during reactivation will automatically navigate to the deepest child that was active before inactivation, retaining all locals as well. Just like you are already doing but basically using a state config boolean instead of a directive.

Additionally, is there a reason why we couldn't just use ui-sref instead of parallel-state-selector?

I'm fine with baking all of this functionality into ui-router. I think others will be too, screw whatever has been said before. This is a slightly newer spin on the concept that I believe works with core. Now this is all naive to whether this is technically possible, so maybe its not doable.

Some other thoughts:

christopherthielen commented 10 years ago

Tim, I love the idea of baking in deep state re-activation as state configuration. This could be useful for non-parallel states too.

I made some changes, updated the plunkr, and committed src changes to my fork.

Baked in deep tab state re-activation

I implemented your suggestion about configured state deep re-activation and nuked both tab control directives. This feels much cleaner! I don't think I like config option's name however, retainDeep, and I don't like any alternatives I've come up with either. For now my code is using deepStateRedirect but I'm sure there's a better name out there.

state.js integration rewrite

The 5 integration points remain the same, however I rewrote the code to hopefully be easier to comprehend.

I am now returning a parallelStateTransitionType object var ptType = $parallelState.getParallelTransitionType(keep, fromPath, toPath); which has { to: (bool), from: (bool) } to indicate if the transition is to and/or from a parallel state.

I created a function to return the type of "re-entering inactive state" transition is occurring. It indicates if we are re-activating a state with the previous locals, or if we need to re-initialize because of updated state params. This is due to a bug I found where a parallel state transition to the same parallel state with different params wasn't treated properly. (I've added a state param to root.tabs.deep and controls in root.tabs.deep.deep.nest in my example which demonstrates the fix) var pEnterTransition = pEnterTransitions[l] = ptType && ptType.to && $parallelState.getEnterTransition(state, toParams, ancestorParamsChanged); Enter transition can be either "reactivate" or "updateStateParams". During updateStateParams, we treat it as an exit/enter, not a reactivation. If an ancestor updated their state params, all descendants will also exit/enter. ancestorParamsChanged = (pEnterTransition == "updateStateParams"); Now we have set up a nice way of checking how to load locals:

         if (pEnterTransition == "reactivate") {
           locals = toLocals[l] = $parallelState.getInactivatedState(state, toParams).locals;
         } else {
           locals = toLocals[l] = inherit(locals);
           resolved = resolveState(state, toParams, state === to, resolved, locals);
         }

And a nice way to determine if we should onExit or onInactivate:

          // Treat parallel transition type "updateStateParams" as an exit/enter
           if (ptType && ptType.from && pEnterTransitions[l] !== "updateStateParams") {
             $parallelState.stateInactivated(exiting);
           } else {
             $parallelState.stateExiting(exiting);

And a nice way to determine if we should onEnter or onReactivate:

          if (pEnterTransitions[l] == "reactivate") {
             $parallelState.stateReactivated(entering);
           } else {
             $parallelState.stateEntering(entering, toParams);
             if (entering.self.onEnter)
               $injector.invoke(entering.self.onEnter, entering.self, entering.locals.globals);
            }
questions

Could a parallel state fill more than one named ui-view? Seems like it could/should. Would that currently work?

Yes, I see no reason it wouldn't work currently but I haven't tried it.

What is scope: true?

Nothing to see here, move along. (copy/paste stupidity)

state transition thoughts

I have been thinking about controllers that want to know when they're being transitioned away from. Currently, a controller can intercept stateChangeStart and check if they have dirty data that needs to be saved, etc. With state inactivation, this concept no longer makes sense. We need to add a way to check if the controller is about to be inactivated, as opposed to about to be exited.

timkindberg commented 10 years ago

So. Fucking. Awesome. Its so conceptually easy to pick up now, I can't believe you've done all of this man. I really still want @nateabele to take a look, I think he's a bit busy lately (aren't we all).

I was wondering if instead of setting parallel to true or false, maybe set it to either "shallow" or "deep". That would eliminate the deepStateRedirect property.

I'm really surprised others haven't chimed in yet either.

christopherthielen commented 10 years ago

But then we are adding complexity. I'm thinking of a comment by @nateabele in issue 618... are those suggestions a "10-lines-of-user-code" scenario?

No internal changes required, which means you have complete control over the implementation, which avoids future RFCs requesting additional configurability for whatever theoretical solution we might implement, which means I have more time and energy to spend implementing things this module isn't currently capable of, rather than merely making things that are already possible 10-lines-of-user-code easier.

  • Now that I've analyzed this problem more, does "parallel" conceptually still make sense, or does something more like "retain" better convey what is happening? The crux of the feature is as follows
    If a transition from some state A -> state C "pivots" over an ancestor state B where B is parallel: true, then retain the DOM tree of B and track the states behind that B tree as inactivated states.
timkindberg commented 10 years ago

I think persisting can come later, we don't want to do too much until there is a valid outcry for it. I think both 'parallel' and 'retain' make sense to me. 'retain' may be a bit easier conceptually—"I want my stuff to stick around". Some people call them "sticky views" as well. So something like "sticky" could also work. "parallel" is certainly more technical sounding. We still need @nateabele to take a look too.

MrOrz commented 10 years ago

Hi all,

I've kept an eye on the thread for a while but not until now do I have some spare time trying this out. It's a really exciting proposal and I found that the parallel option on states (in contrast of on "views" in #562) pretty flexible. @christopherthielen you did a great job! :+1:

First Try on Modal Dialogs

The first thing I tried is to use @christopherthielen's branch to recreate the modal dialog scenario mentioned in #562 . It is achievable by adding a root state on top of the detail state and the index state. parallel: true is only required on the state that lists all the items, as shown in this plunker -- The list page and the detail pages in action looks pretty good.

Follow Up

However, when I extend the example above to a more realistic situation, problems start to emerge. In this example I added multiple states, most of them are listing items, which are commonly seen in e-commerce websites. Since they all opens the item detail in modal dialogs, all of them requires parallel: true in their state config. In a website I am currently building, this literally means I need to add parallel: true to each and every states, except the one or two states that shows its view in modal dialogs.

Similarly, my proposal #562 suffers from the complexity in state configuration: the retain: true must be duplicated for each and every view for the states displayed in modal dialogs (for the example above, the states are root.detail and root.buy). State decorators may help, but the entire setup is still counter-intuitive.

Intuition When Dealing With UI

On the other hand, what makes me excited is that Chris' initial proposal shed light on a whole new direction: specifying the view behavior in template, instead of in state configuration. I think it's more intuitive to add html attributes to ui-view than in state config because when we deal with presentational user interfaces, we would point at a visible view in the physical monitor and say "hey, I want this block to stay unchanged", rather than seeing it from a state-machine perspective, which is more conceptual and abstract.

I suddenly realized that what I really need back in #562 should be a boolean attribute like ui-retain alongside with ui-view directive. With the presence of the ui-retain attribute, the ui-view never updates itself unless entering a state specifically assigns a new template / new resolve data to it, or when its parent view updates. Since it's a boolean attribute, we don't have to mix state information into view.

I think the boolean attribute should also work in the minimal example in Chris' plunker, but I have not yet tested with code. Is there point I am missing above? I'd like to hear from you.

nateabele commented 10 years ago

Jeez, did I really not respond to this until now? Sorry about that. I have a more extended reply once I'm able to digest all of this. In the meantime I will say, the code looks great, but a lot of what this is touching is about to change significantly.

christopherthielen commented 10 years ago

@MrOrz thank you for taking the time to look at this and offer your thoughts.

@nateabele I look forward to your thoughts. How in-flux are we talking here? Is it worth attacking other issues at this point? I have been working 774 (animation and anchor tag oddities), but don't want to waste effort.

thoughts on your Follow Up comments

For the "modal" use case, my thought was to create a standard top-level modal state, then a top-level parallel state with substates for "everything else".

I updated your 'Follow Up' plunk here: http://plnkr.co/edit/i0sd6j (note, I had to fixa bug in isChangeInParallelSubtree where parallel transitions weren't detected if they pivoted across the "implicit root state". I commited the fix to my branch and updated my plunk's angular-ui-router-tabs.js)

Instead of each "products" state marking itself as parallel, you can mark a parent state as parallel. I suppose that parent state could even be marked abstract too (but I didn't do that in the plunk).

instead of:

implicit root
             |
     ------root ---------------
   /     /    \         \       \
detail  buy   popular   latest   suggest
              PARALLEL  PARALLEL PARALLEL

I sructured your states as follows, and got some reusability:

            implicit root
           /             \
        modals            products
        /    \            PARALLEL
     detail  buy         /     |   \
                      popular  |    suggest
                             latest
parallel declaration on state vs template/view

I think there is a lot of room for discussion here and many ways we could approach this. We need to consider multiple facets, including but not limited to: What is most intuitive? What is most consistent with the current API? What approach offers the cleanest implementation?

I agree with you that it feels initially the most intuitive to mark the ui-view in the template as parallel/retain, which explains why I approached my first implementation attempt that way. I suspect, however, that there are there cases where a state tree should absolutely know that it has to support "retain". My use case involves tabs that stick around for a long time. Those tabs have user-input dirty-checking and warn the user if they're about to lose their changes. In this case, is it important to be explicit about parallel parents, and for the states to "know" they have a parent parallel state, or is having a parallel view as some ancestor enough? What I don't like is subtle behavior change when some substate of a state tree is (or is not) placed inside a parallel ui-retain ui-view at the whim of some parent template.

I don't feel super strongly about this, but I feel that configuration at the state level smells slightly better to me.

Edit: I should clarify, in my dirty checking example, the controllers mapped by the state should only warn the user about dirty input if the state is about to onExit, not onInactivate.

timkindberg commented 10 years ago

@MrOrz I know that the #562 kind of fizzled out, sorry about that. How do you feel about your implementation compared to this one? Thanks for taking a look!

MrOrz commented 10 years ago

I initiated #562 under a specific use case of my own website. All pages in this website would open a drawing or an article in a modal window. That's why my proposal wanted to alter views' behavior, instead of states'.

Chris' implementation, on the other hand, provides a more comprehensive solution to the problem. By manipulating states directly, locals and context can be stored and retrieved and even deep state re-activation can be achieved beautifully. Although such functionality is not required in my use case, Tim's implementation surely outperform mine for its flexibility. It took me some time to understand the concept though, because my thoughts were too focused on how a specific "view" should behave (that's probably why I came up with the new ui-retain idea.) Also the term "parallel" seems as if multiple states can be simultaneously activated in a single state machine, which confuses me a little bit.

timkindberg commented 10 years ago

Yeah I may like the "sticky" or "retain" concept better than "parallel". @christopherthielen is it built in to allow multiple parallel states that all show at the same time; aka that don't use the 'tabbing' 'only show one at a time' feature?

christopherthielen commented 10 years ago

is it built in to allow multiple parallel states that all show at the same time; aka that don't use the 'tabbing' 'only show one at a time' feature?

It is. In my plunkr, you can see this by checking "Show inactive tabs"

timkindberg commented 10 years ago

Ah ok so that is the default. Its up to the dev to show/hide the appropriate views if they so choose.

ashaffer commented 10 years ago

@christopherthielen @MrOrz

http://plnkr.co/edit/joqCreywFtnUCFMzVzBF?p=preview

This is an implementation of pinterest-style routing on top of your parallel change. This allows you to refresh with a dialog open, and then arrive at the full sized display of what was previously in the modal.

Without something like this, you end up being forced to link/route directly into a modal (or other similar state), which is a little strange, especially when the route doesn't encode any information about what's supposed to be behind the modal.

Edit: this also resolves #317 I think.

timkindberg commented 10 years ago

@ashaffer so you like it then?

ashaffer commented 10 years ago

@timkindberg yes, very much.

christopherthielen commented 10 years ago

@ashaffer cool :+1:

MrOrz commented 10 years ago

@ashaffer This is much better than my implementation of pinterest-like modal in #562 ! At first I would think it's straight-forward to hack ui-view and leave the states alone; now I start to appreciate the elegance to manage all the views by their states :)

braco commented 10 years ago

@ashaffer: Just FYI, I believe that implementation will incorrectly pop up a modal with:

  1. open modal
  2. reload for full page
  3. click anything
  4. click back in browser: modal pops up, full page expected
ashaffer commented 10 years ago

@braco Good point. I think the resolution is that instead of defaulting to modal display when there's a prior state, you have to explicitly describe which prior states cause the modal to open. This has the slight quirk that if in the 'click anything' step, you click on one of the states in the modal list, then pressing back will take you into a modal (when you had been at the full page description). For my purposes that quirk is acceptable, but it may not be in all cases.

MrOrz commented 10 years ago

@braco I tried the same manipulation sequence on facebook:

  1. find a post of a photo
  2. reload for full page of the photo
  3. click away to list of photos
  4. click "back" in the browser.

And the photo shows up in a modal. I think facebook think the quirk is ok too :wink:

braco commented 10 years ago

@MrOrz it would depend on the use - a picture in a gallery or a product in a flat listing are contrived use cases. If the page state was complicated, say five calls down into a contextual infinite scroll, one would need to restore and manage all of these secondary states as well.

Even worse, we'll get the same behavior if you are deep linked into a detail context:

  1. Click first link to site, which is a details page
  2. Navigate away
  3. Click back, you're now in a modal

This is bad. What context is that supposed to be in? How could we possibly know what the context should be?

This is all probably an argument against parallel: true as well. If you have product or detail modals on your site that can be clicked from anywhere, everything would need to be parallel, no?

christopherthielen commented 10 years ago

I'll cherry pick a comment because I haven't dug into the full use case yet:

This is all probably an argument against parallel: true as well. If you have product or detail modals on your site that can be clicked from anywhere, everything would need to be parallel, no?

I wouldn't think of it that way. Instead, think of it as "everything would live under a common parent parallel state"

See my previous comment about restructuring states https://github.com/angular-ui/ui-router/issues/894#issuecomment-36537936

CMCDragonkai commented 10 years ago

I was thinking of a state persistence service where states remember the way it was presented. So for example if in state 1, someone had clicked on an accordion which opened up a box, if they navigated to state 2 and then back to state 1, the accordion would still be open. In fact even the scroll position of state 1 could be remembered. This basically allows all kinds of multitasking to occur on a webpage without the need for multiple tabs. It would also supersede autoscroll.

It seems that your concept of parallel states addresses the idea above. I would like to hear your comments on this.

Furthermore I was thinking that by default, nothing should be remembered. If you bind yourself to events in the controllers, you need to unbind them using the $destroy event. However this functionality should be an opt in not opt out. So maybe if there was a way to export the "$scope" variables inside the controller's instance which would then be remembered by a state history service.

svemir commented 10 years ago

We went down that path for many years and in the end realized it creates more problems than it solves. In our new apps we place such state elements in the URL so the users can use back button, tabs, and bookmarks if they need something preserved.

CMCDragonkai commented 10 years ago

@svemir Persisting the state's composition could be done via the URL as well. But that's kind of what ui-router already does. You'll just create lots of "states" with unique urls. However this does result in a lot of urls to maintain and a lot of urls that have the exact same content. And some things are superficial like preserving relative scroll position that really shouldn't be in the URL.

Doing this will push SPAs to act more like desktop applications that can do multitasking.

christopherthielen commented 10 years ago

@CMCDragonkai My perspective is that trying to persist ephemeral ui state, such as scroll position, accordion state, etc is a slippery slope and likely will never 100% achieve what I believe is your intent. For example, how far do you take view-state persistence? You could potentially want to retain lots of non-model view-state between transitions such as dropdown toggled open/close, filter selections, even taking it as far as text selection. All that stuff requires extra coding and maintenance overhead.

I prefer the parallel/stick/retained model because the browser is already really good at remembering all that stuff.

On the other hand, I could see benefit for user experience in persisting selective view-state data (via url, or whatever), but more in "restore previous session state" use case context, and less for "tabs" or the "modal" use cases.

Persisting ephemeral view-state in url/cookie/whatever pushes the complexity onto the application developer. Multiple parallel/sticky/retained states pushes complexity onto the framework.

CMCDragonkai commented 10 years ago

Of course, the developer would selectively choose what's important to be remembered with regards to ephemeral ui state. Not everything would be captured.

My idea was just something like an exports object. Where you simply call up a state persistence service, and you just say "hey I'm this state, and this is what I'm looking like right now!". Export an object describing what you think the state looks like, it might not be 100% perfect unless you specify everything the state looks like. I'm not sure how to deal with child states and parent states relationships in this case.

Then when one navigates back to the original state. The state service then injects back all those parameters. Then you pass those parameters to $scope level properties. So if there like boolean $scope.openedAccordion = false. Well then you just set it to $scope.openedAccordion = statePersistence.state.openedAccordion. (On startup false, but now it's true).

timkindberg commented 10 years ago

@CMCDragonkai sounds like you are getting closer and closer to the solution ;) I could see value in a service like that.

mfrye commented 10 years ago

Damn this is an awesome idea. I could see a lot of use for this.

christopherthielen commented 10 years ago

I ported my parallel code from 0.2.8 to 0.2.10 ui-view and updated my plunkr to match. I'm still very much interested in helping get something like this into the main project. If I get some more direction, I can help work towards that goal.

https://github.com/christopherthielen/ui-router (now in the issue-894 branch) http://plnkr.co/edit/YhQyPV

nateabele commented 10 years ago

@christopherthielen So, this past week I finally had the mental capacity to fully digest this. First of all, awesome effort putting this together (btw, I just noticed you were in Minneapolis -- I was there last week, we totally could have done a jam session).

My thoughts are: the code looks good (could obviously be a little cleaner if it was more tightly integrated, but good job keeping it separate for clarity), and the concept is close, but the API and the semantics need a little help.

Specifically, $state shouldn't know anything about uiViews, and uiViews shouldn't know anything about $state, so something like parallel-state=".S1" is kind of a non-starter. In fact, one of the design goals is for people to be able to use $view and uiView as a stand-alone view system, independent of the rest of the state/routing infrastructure (which I'm almost there on).

Secondly, rather than declaring parallel states with parallel: true, I think you'd get more control with less complexity by having each parallel state explicitly declare its siblings. Consider the tree below, adapted from your plunkr:

                              root.tabs.people.person
                                 / 
                                /
                      root.tabs.people
                       /
                      /
    root.notabs      /
     /              /
  root --- root.tabs --------------- root.tabs.deep -- root.tabs.deep.deep  -- root.tabs.deep.iii.deep -- root.tabs.deep.iii.deep.nest
               |   \                              
               |    \                              
               |     \
               |      \        root.tabs.subtabs.S1
               |       \          / 
               |      root.tabs.subtabs
               |                  \
               |              root.tabs.subtabs.S2
               | 
               |
                \               root.tabs.outersubtabs.S1
                 \                    /
                root.tabs.outersubtabs
                                      \
                                   root.tabs.outersubtabs.S2

With that tree, if your state definitions looked like this (excerpted):

.state("root.tabs.people", {
    siblings: ["root.tabs.deep"],
    /* ... */
}).state("root.tabs.deep", {
    siblings: ["root.tabs.people"],
    /* ... */
}).state("root.tabs.subtabs", {
    siblings: ["root.tabs.outersubtabs"],
    /* ... */
}).state("root.tabs.outersubtabs", {
    siblings: ["root.tabs.subtabs"],
    /* ... */
})

...that means that any two sibling states could be active in parallel, and any child states within those sibling states (i.e. cousin states, not immediate siblings) could also be active in parallel. Here's a short breakdown of examples of which states could be active in parallel and which couldn't, given the configuration above:

That would also provide more transition control when activating states:

As far as views, I think implementing all of this will make the most sense once the view system is fully decoupled from the state machine, at which point each uiView will have an absolute path from the root uiView, and that absolute path is what states will use in their view definitions rather than the current goofy state-based view targeting syntax. There may still be some semantic issues here, if more than one active state is targeting the same view, but we can figure out how to resolve that once we've tackled some of the stuff above.

I know I've taken forever to respond to this, and the foregoing was a bit long-winded, but let me know if that all makes sense. Thanks again.

christopherthielen commented 10 years ago

@nateabele

Decoupling $ViewDirective and $state

Specifically, $state shouldn't know anything about uiViews, and uiViews shouldn't know anything about $state, so something like parallel-state=".S1" is kind of a non-starter.

Gotcha. parallel-state=".S1" is long gone in favor of named view targetting. Today I started re-working the code to remove any dependancies from state to uiView. Have a look at my progress: https://github.com/christopherthielen/ui-router ... _viewDirective.js is now unchanged from ui-router/master_.

I switched from short-circuiting uiView's updateView to implementing a $parallelState.processTransition function which precomputes most of the decisions surrounding a parallel state transition. This precompute determines all the enter/exit transitions and finds which states will be inactive after the transition. The inactive states' locals are added to the root state's root.locals using prototypical inheritance (state.js:622)

    root.inactiveLocals = {};
    root.locals = inherit(root.inactiveLocals, { resolve: null, globals: { $stateParams: {} } });

During transitionTo, root.inactiveLocals is populated with the views/locals that will be inactive during the transition (state.js:817)

        // TODO: This does not restore root.inactiveLocals after failed transitions.
        for (var name in inactiveLocals) { delete inactiveLocals[name]; } // Rebuild root.inactiveLocals each time...
        for (var i = 0; i < parTrans.inactives.length; i++) {
          var iLocals = parTrans.inactives[i].locals;
          for (name in iLocals) {
            if (iLocals.hasOwnProperty(name) && name.indexOf("@") != -1 && !locals[name]) {
              inactiveLocals[name] = iLocals[name]; // Add all inactive views not already included.
            }
          }
        }

Now, locals has all the inactive named views (at the pre-root state level), plus all the active named views from the currently active state tree in their standard prototypical inheritance structure (using the existing $state mechanism).

When uiView directive checks if (!firstTime && previousLocals === latestLocals) return; // nothing to do (viewDirective.js:209), the check returns true for inactivated states, so they never call cleanupLastView(). That entire ui-view integration point is now handled in the 'standard' view targetting way instead of the short-circuit kludge I had in there before.

at which point each uiView will have an absolute path from the root uiView, and that absolute path is what states will use in their view definitions rather than the current goofy state-based view targeting syntax.

Hopefully this decoupling of the parallelState code will make it easier to port when the views are decoupled fully from $state.

Parallel State configuration and/or "Sticky State" concept

Secondly, rather than declaring parallel states with parallel: true, I think you'd get more control with less complexity by having each parallel state explicitly declare its siblings.

I took a few days to think over what you posted regarding state configuration and sibling declarations. My gut reaction was that it felt more complicated than parallel: true. After a few days it still seems that way to me. Read over my thoughts and consider this other perspective, then let me know if you still think declaring parallel-sibling states makes the most sense.

There are quite a few ways to approach how to structure sticky/parallel states/views. Although the code I'm writing mentions parallel everywhere, I've started to gravitate towards thinking of the "parallel states" more along the lines of "sticky states". Currently, I think this perspective offers the best balance of simplicity and customization. When I think of "sticky state" I think that once activated, each sticky state lives until its parent state goes away.

When approached this way, the sticky state doesn't have to know anything about its siblings. In the use case I started with, each tab is a separate application/module of a overall app. Each module essentially does not know or care about the others. I can have up to 12 modules open in tabs at a time. If each module had to declare which modules are peer siblings, my declarations would be huge (12*12). Of course, this could be solved by defining some sort of sibling group which all 12 modules are part of, but I feel that maybe declaring sibling groups is overkill, and more than the typical user will want.

The sticky state perspective also allows for uses cases such as the "modal dialog" (see my response to MrOrz here: https://github.com/angular-ui/ui-router/issues/894#issuecomment-36537936) where the shared modal doesn't really want to be parallel-ly active in relation to the code invoking it. Instead, the creator of the modal wants to be sticky while the modal is active, but the modal doesn't want to be long-lived in relation to its creator. Rather, it wants to go away once the user navigates anywhere else, even back to the sticky state that it was created from.

Different behaviors for grouped parallel-sibling states can be achieved by structuring the states appropriately. Consider 4 states A, B, C, D which want parallel group relations like (A, B), (C, D) where A and B are parallel in relation to each other and likewise for C and D. One could use sticky state where A and B are sticky with a common parent X, and C and D are sticky with a common parent Y. X and Y can be siblings with parent Z

$state.go("root.people.person", null, { exclusive: true }): transitions to root.people.person, exits all other branches of the state tree

Good point, I haven't given much thought to exiting these long-lived states yet. When applied to my initial use case, I actually want to exit one state/tab, while leaving the other (up to 11) states active/inactive.

$state.go(["root.people.person", "root.tabs.deep.deep"]): transitions to both states simultaneously (the second is a direct descendant of the first's valid sibling) — retains states that can validly coexist, exits all others

I definitely agree there is potential in this sort of invocation. I think this could be implemented regardless of "parallel states" vs "sticky state".

Looking forward to everyone's thoughts and input.

ashaffer commented 10 years ago

@christopherthielen Have you considered putting parallel: true on the destination states, rather than the source? It seems to me that in general its the destination states which are intrinsically either parallel or not (e.g. a modal, or some nested state within a given page), whereas pages are almost never strictly parallel, they are only parallel relative to a particular destination.

christopherthielen commented 10 years ago

@ashaffer I guess I don't completely understand what you mean. Are you saying, "why not mark the modal states instead of the states that open the modal"? If that's the case, I have some thoughts:

1) I think what you are suggesting is that the modal should trigger the "parallel" behavior between the modal any other state that was active before the modal was invoked. Then, the previous state can be reactivated when the modal is closed. This would allow us to have a bunch of "extra" states that don't affect the "main" state.

2) If we marked the "destination" states, what happens to the source DOM/state after navigating elsewhere from the modal? I guess the original state gets deconstructed at that time...

3) How would we then configure states to accomplish the "tabs" use case, where each tab retains its dom and state, independent of the others? With the tabs use case, I believe the "source" states are intrinsically parallel.

4) Consider the "sticky state" concept. Does it smell better to you if we think of the source states (the non-modal states) as "sticky"?

ashaffer commented 10 years ago

@christopherthielen,

  1. Yes, that's exactly what I mean.
  2. Ya, I think that's right, navigating away from the modal would simply destroy the modal state and the original state.
  3. It seems like the tabs use-case is equivalent regardless of whether parallel/sticky is specified on the source/destination, unless i'm misunderstanding some aspect of the problem.
  4. I do agree that to specify this behavior on the source state 'sticky' is a better metaphor. But it still seems cleaner to me to specify it on the destination state (in which case the term 'parallel' probably makes more sense).

That being said, I feel like i'm not fully understanding your counter-example use-case, so perhaps that will clear things up.

christopherthielen commented 10 years ago

@ashaffer re: 3) have a look at my http://run.plnkr.co/GNqC1oxYiT4m59gy/#/tabs . I have 4 peer states, all tabs, of which 3 are "sticky". The fourth tab (outersubtabs) is "not sticky". I'm not sure how I could specify this using only "destination" tagging. For tabs use case, the tabs don't care where they are invoked from, only that they remain sticky regardless of where navigation occurs afterwards. For my use case, I can't have any/all of my tabs destroyed if the user happens to navigate to some other state.

I see your point though, with the simple modal use case. It smells better to mark the modal state once, which gets re-used all over the place, rather than mark the parent of "all the states that might ever invoke the modal state. maybe.".

I feel these two use cases are cognitively different enough that they might warrant different approaches, syntactically. Of course, the same mechanism could be used, regardless.

ashaffer commented 10 years ago

@christopherthielen Ah, ok. How would you feel about just having both options? sticky: true to specify it on a source state, parallel: true to specify it on a destination state? Would that create any sort of weird conflicts?

christopherthielen commented 10 years ago

@ashaffer I think it's technically feasible using the recent view targeting sticky implementation. I would defer to @nateabele and @timkindberg as for what makes sense to ui-router; that is, sticky, parallel, source, destination, or some combination.

snekbaev commented 10 years ago

really hope to see this implemented in near future, my current project could benefit a lot. Now I have to keep track of user's actions and manually restore the state, quite a bad way to mimic that it is the "same" view as when he navigated away

samuelhorwitz commented 10 years ago

I believe this would be very useful for my current project as well. This project is still using ngRoute, but I would switch to ui-router if this gets implemented because it seems like it could solve the pitfalls of jumping back to the top of an infinite scroll list among other quirks that take away from user experience.

timkindberg commented 10 years ago

I'm still really excited about this! Hope we can get this fully baked soon.