seriema / angular-apimock

Automatically route your API calls to static JSON files, for hiccup free front–end development.
johansson.jp/angular-apimock
MIT License
65 stars 8 forks source link

Add $location service #2

Closed ceoaliongroo closed 10 years ago

ceoaliongroo commented 10 years ago

There are some benefits If use the module use $location instead window.location http://docs.angularjs.org/guide/$location

seriema commented 10 years ago

Yes, the reason we didn't use it was because at the time we need to call location (in .config()) it isn't available. If you know how to make it work, please let us know.

seriema commented 10 years ago

We've changed things since then I remember now. It might be possible, will try it out! Hopefully it will allow unit-testing of that part. Busy creating demos atm.

ceoaliongroo commented 10 years ago

@seriema Thanks,

I did a refactor of the code and add the unit test for your review.

Also, i'll create an issue #4 to extend the test and check or prepare the work with external data services.

seriema commented 10 years ago

This issue can be the request for a pure $location refactor. If you do just the $location bit with the tests you have it would be awesome. Please sync down the latest code though, just did some big updates.

seriema commented 10 years ago

Two things to think about is routing. 1) Query parameters like apimock=true don't sit in window.location.search but window.location.hash. Need to verify that this works with $location. 2) When using ngRoute it doesn't do a page refresh when adding ?apimock=true so the parameter isn't picked up. Need to make sure routes update the mock flag. See here for a sample where this breaks.

ceoaliongroo commented 10 years ago

@seriema about: 1) I understood the explanation, but still is difficult see the issue or the new feature that you propourse without a code example.

Could you create a fail spec in the test? Thanks

2) You could try to add reloadOnSearch=false in the route

    $routeProvider
      .when('/', {
        templateUrl: 'views/movies-table-list.html',
        controller: 'MoviesCtrl',
        reloadOnSearch: false
      })
      .otherwise({
        redirectTo: '/movies/list/'
      });
  });

Could you give access to the demo code?

All the best...

seriema commented 10 years ago

1) We don't need to worry about that anymore. It seems to be working with $location.

2) Interesting! Two issues with that though. It would require ngRoute (we want to minimize dependencies) and it seems to be set on a per-url-basis (we need something generic).

The demo code is in the gh-pages-dev branch. Thinking about moving them here to a /docs folder or something.