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

Small feature additions + Bugfix for Angular 1.3.0 injector. #19

Merged
merged 2 commits into from
May 20, 2014
Merged

Small feature additions + Bugfix for Angular 1.3.0 injector. #19

merged 2 commits into from
May 20, 2014

Conversation

taz
Copy link
Contributor

@taz taz commented May 10, 2014

  • Added support for $animateProvider
  • Added support for CSS Loading (And perhaps other file types in the future)
  • Added loadAll function for use when a state requires resolve on more than one asset.
  • FIX: Angular JS 1.3.0-beta.8 changed the way config blocks are handled (now invoked last)

* Added support for CSS Loading (And perhaps other file types in the future)
* Added loadAll function for use when a state requires resolve on more than one asset.
* FIX: Angular JS 1.3.0-beta.8 changed the way config blocks are handled (now invoked last)
@taz
Copy link
Contributor Author

taz commented May 10, 2014

I still think CSS support is a bit of a hack to be honest. Using the same method however you could also load other non-angular JS libraries and the like. Happy to discuss ways to make it less hackish.

I spent the better part of the day trying to work out why .config blocks weren't working before I realised it was beta.8 specific :-(

@ocombe
Copy link
Owner

ocombe commented May 10, 2014

I'm not sure if we should add support for css lazy loading, it's not really an angularjs feature. You can use script.js (or another external lib) to load them, (with the help of $q for exemple). But why not :-)

The loadAll should be load with an array as argument, I don't think that we should add more function names.
Or have two but add an internal test to swith to loadAll when load receives an array instead of a string.

Does your update for config block keep the support for older than beta8 versions of angular ? if not we may have to add a disclamer in the readme, and maybe make two versions of the libs to keep an old and a new version working.

@taz
Copy link
Contributor Author

taz commented May 10, 2014

I couldn't see support for listing other resource types in script.js, I would have preferred not to write a loader for it, but the functionality seems to be absent.

I see your point re: load vs loadAll, I'll adjust it tomorrow.

I tested the .config test against 1.2.16 as well. The change separates the config blocks into a new _configBlocks array. In previous versions they are just bundled into the regular invoke array, which the new method still executes. So all good there.

I just noticed an issue with http interceptors too. They load into the interceptors array OK, but never get added to the http service once its instantiated. Not sure how to get around that :(

@ocombe
Copy link
Owner

ocombe commented May 10, 2014

You may be right, script.js seems to load only JS files (hence the name) :-)

@ocombe
Copy link
Owner

ocombe commented May 19, 2014

If you have the time, change the loadAll to load with array and I'll merge this.

@taz
Copy link
Contributor Author

taz commented May 19, 2014

Hi Olivier,

Sorry, I've been a bit busy / distracted. I hope to get back to this before
the end of the week.

On Mon, May 19, 2014 at 11:51 PM, Olivier Combe notifications@github.comwrote:

If you have the time, change the loadAll to load with array and I'll merge
this.


Reply to this email directly or view it on GitHubhttps://github.com//pull/19#issuecomment-43506108
.

Removed an errant console.log that I missed. Oops.
Adjusted README.md to remove references to loadAll
@taz
Copy link
Contributor Author

taz commented May 20, 2014

Note: this doesn't resolve the issue with http interceptors... I'll open a new issue for that. I've found a solution, but it's not pretty.

@ocombe
Copy link
Owner

ocombe commented May 20, 2014

Thanks ! I'll test it tonight :)

@taz
Copy link
Contributor Author

taz commented May 20, 2014

Cool, I'm a bit swamped at work at the moment and about to head off to bed once I log this interceptor issue (which frankly I don't think there's a 'nice' solution for short of patching angular itself), but I'll have a look tomorrow if you find anything odd.

My late night is your early morning mid afternoon(?) and vice versa I suspect :-)

@ocombe
Copy link
Owner

ocombe commented May 20, 2014

Yeah, in France it's 15h20 (3h20pm) now !
I've also been overwhelmed by work these last weeks (I'm working on the famous-angular lib, https://github.com/Famous/famous-angular), we just released it yesterday so I might have some time free tonight until the issues start pilling into the github repo ;-)

ocombe added a commit that referenced this pull request May 20, 2014
* Added support for $animateProvider
* Added support for CSS Loading (And perhaps other file types in the future)
* Added loadAll function for use when a state requires resolve on more than one asset.
* FIX: Angular JS 1.3.0-beta.8 changed the way config blocks are handled (now invoked last)
@ocombe ocombe merged commit c45155d into ocombe:master May 20, 2014
@ocombe
Copy link
Owner

ocombe commented May 20, 2014

Thanks a lot, everything works fine !

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.

2 participants