yaru22/angular-hovercard — pull request #2 by IanShoe (2014-01-14 14:50:41+00:00)
Title: Some improvements to consider : D

Hey, I really liked your module but I wanted to make a few changes that could hopefully make it easier for others (like myself) to use. There's 3 changes I made and I'll quickly explain them here.

1. No more isolate scope - The reason for this change is that I wouldn't consider the hovercard to be stand alone but rather is expected to get detail data from the parent scope. So I have it inherit off of parent scope such as the user or some other data model rather than making a bi-directional binding attribute called "data" and including it in the scope object as data: '='. What I implemented should be pretty seamless when you make the details card also since you can do your standard {{bindings}}

2. Since we remove the scope isolation, many of the properties are now easily accessible through angular's provided $attrs in the link function. We can now remove things like ng-init and simply bind them within the link function. I believe this follows the more angular way.

3. I saw you had an html2js step in your gruntfile but I don't know if you use it. Either way, I like to place the template and preload it at the bottom for smaller projects like this. This step could also be better done with a concat/minify step in the grunt build, especially if you would like to keep the dom and javascript separate. I leave that up to you, but please please please make it so that it's not a requirement to change the templateUrl in the source itself!!

Thanks again and I really appreciate the module overall! Let me know if there's anything else you are considering or would like me to do!

Ian

P.S. I also made small update to the test and ran it and it all looks good : D