Famous / famous-angular

Bring structure to your Famo.us apps with the power of AngularJS. Famo.us/Angular integrates seamlessly with existing Angular and Famo.us apps.
https://famo.us/angular
Mozilla Public License 2.0
1.92k stars 275 forks source link

implemented feature fa-router #137

Closed rajatkhanna82 closed 10 years ago

robinboehm commented 10 years ago

Uhhhhh .... nice ... will take a look at it this weekend! =)) :+1: thx for this!

robinboehm commented 10 years ago

No tests? :///// Wow ... it's hard to develop such a complex module without tests ... will you also add tests?

zackbrown commented 10 years ago

Yay, tests! Thanks for all your work on this, you guys. I still need to put this through some paces. @robinboehm, if you do get a chance to check this out, could you please share your thoughts on it?

steveblue commented 10 years ago

I'm confused, what will the advantages of this router be, say over ui-router? I doubt I'd sacrifice the many advantages of using ui-router.

zackbrown commented 10 years ago

The key difference is the ability to perform arbitrary Famo.us animations (or arbitrary logic, really, which could easily include promise resolution a la ui-router's resolve) on state transitions. This router is designed along the finite state machine model of ui-router, but with transitions between the FSM states as a first-class notion.

When you go from state A to B, for example, you can specify custom transition logic, both for A's transition out and B's transition in. Further, you can declare an adjacency matrix in the router's config to specify different transitions from A to B vs C to B, or A to B vs A to C [this adjacency matrix describes the edges of the graph, not just the states, so you get to declare the entire graph of the FSM.]

I'm really excited about the feature set and the design that these guys have worked out; I just need to test it some more before I'm comfortable merging it into the core library. I would also be glad to hear anyone's thoughts or opinions on the implementation/design.

steveblue commented 10 years ago

I would only adopt it if this new router extended the functionality of the existing ui-router. One of the main reasons we use ui-router is not just for nested views, but also the robust url matching. The $urlMatcherFactory is an essential part of ui-router I would not want to lose in favor of flashy transitions.

ashwell commented 10 years ago

This feels a little like trying to reinvent the wheel. Would it not be feasible to add transition logic to the already excellent ui-router as another module? Much in the same way ui-router.stateHelper adds extra functionality to ui-router (https://github.com/marklagendijk/ui-router.stateHelper).

zackbrown commented 10 years ago

The plan with this is:

  1. We're most likely not going to merge in this branch as it stands.
  2. @rajatkhanna82 and I have been working together to implement a refactor #155 that supports robust showing/hiding of any fa-directive. This work-in-progress is currently PR'd and is almost done, but has a little bit left to go.
  3. Once it's done, ui-router should be fully supported, even intra-scene-graph (inside of an fa-app.)
  4. We are going to look at adding an extension to ui-router like you suggest, @Ashwell, which includes the richer transition logic of fa-router.
zackbrown commented 10 years ago

Now that ui-router works in 0.3.0, I will go ahead and close this out.

(pattern: <fa-app><!-- render tree content --><ui-view></ui-view><!-- more render tree content --></fa-app>) The transition logic can be handled using fa-animate-enter and fa-animate leave. I'm not sure if there's much else we'll need to do with routing, but if anyone has any ideas let's chat.

zackbrown commented 10 years ago

p.s. you guys did an awesome job on this @rajatkhanna82 , @davis, and @adrice727 — not merging this was a tough decision, but I think we should lean on the rich functionality of ui-router if we can, and we ultimately found a way to do so. Thanks again for the contributions.

steveblue commented 10 years ago

Is there an example that shows how to transition between ui-state? Looking at the doc it seems pretty straightforward, just wondering if there was something I could show a junior engineer?

jmeiss commented 10 years ago

I would love to see an example as well if anybody ever tried to do so.

zackbrown commented 10 years ago

Here's a simple demo: https://github.com/zackbrown/famous-angular-ui-router-demo

In general, just add a <ui-view> somewhere inside of your <fa-app> (using routing outside of fa-apps is straight-forward and is just normal Angular routing). If you want transitions, put a fa-animate-enter and/or fa-animate-leave on the ui-view, and expose the appropriate functions on each controller scope. ui-router will invoke the respective enter and leave functions for each route at the same time, so you will be responsible for staggering the animations if you want (for example) one state to wait for the other to finish animating.

steveblue commented 10 years ago

thanks zack!

jmeiss commented 10 years ago

Awesome! It works like a charme!

Thanks :)

jmeiss commented 10 years ago

I just realised that when I navigate between pages, the second page's DOM is appended to first page DOM and it's not removed. When I continue to navigate, following page's DOM will be happened as well and nothing is removed. I can observe it as well with the demo app: https://github.com/zackbrown/famous-angular-ui-router-demo

Can you see it as well?

The result is an overcharged DOM which means the application is getting slower more you navigate.

zackbrown commented 10 years ago

That is a bug in 0.3.0 but should be fixed on master by https://github.com/Famous/famous-angular/commit/269c6d646f008ada50c6acead77da7f9cd1ae153

jmeiss commented 10 years ago

I've checkout the master but the bug is still here. I've tried with famous-angular-ui-router-demo as well but I still have the page's DOM I navigated through appended in .famous-angular-container element :s

Thanks for your help.

zackbrown commented 10 years ago

Sorry for the delay: I've been off the grid for the last week.

One more thing to check: Are you using fa-animate-enter or fa-animate-leave? If so, you need to signal to F/A when an animation is complete so that Angular can clean up—if you don't do this, the DOM elements will never go away.

The two ways to do this are a. return a duration in milliseconds from your animation function (after which time Angular will clean up the DOM elements) or b. pass in the special $done argument from your view (e.g. fa-animate-enter="myFunc($done)") and then call $done() from your function when your animation is complete.

If you're doing this and it's still not cleaning up, let me know and I can take a deeper look.

jmeiss commented 10 years ago

No worry, I'm sorry as well because I found the problem and forgive to reply.

If I recall, I solved it calling $done as you explain in your last reply, that way:

$scope.animateEnter = function animateEnter($done) {
    $scope.translateY.set([0, 0, 0], {duration: 300, curve: 'easeOut'}, $done);
};
$scope.animateLeave = function animateLeave($done) {
    $scope.translateY.set([0, -$rootScope.height, -100], {duration: 300, curve: 'easeOut'}, $done);
};

and I guess that my problem was coming from the fact I wasn't calling it, or the bad way.

Anyway, it's now working and the problem is solved.

Thanks so much for your time and your help!