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

$component controller and new router #159

Merged
merged 4 commits into from
Jul 13, 2016

Conversation

samithaf
Copy link
Contributor

Changes:

  1. updated spec's to use $componentController.
  2. Updated ui-router version to latest which allows routes to use components instead templates

samithaf added 2 commits July 12, 2016 10:11
… version to latest which allows routes to use components instead templates
@@ -5,3 +5,4 @@ node_modules
.settings
*.log
dist/
.idea
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things like this should be handling via global gitignore.

@fesor
Copy link
Collaborator

fesor commented Jul 12, 2016

Cool! The only thing that I don't like in $componentController is amount of boilerplate. Maybe it is possible to hide it somehow?

p.s. Also not sure about uiRouter beta release... but I also use it and everything seems to be ok.

@samithaf
Copy link
Contributor Author

@fesor In the very first look you feel like, there is lot of boilerplate goes on. However I believe it's all for good reason. Specially when you want to test a component with bindings and other dependancies like a service, it's very easy to unit test. Also I have added some additional unit tests for router as well.

I started using uiRouter alpha release at work sometime back and I have a very good size team as well. Also recently upgraded to beta version and so far no issues. So I believe that beta release is good enough.

The change in .gitignore I have revert it back.

Cheers.

@fesor fesor merged commit 2386cf7 into PatrickJS:master Jul 13, 2016
@fesor
Copy link
Collaborator

fesor commented Jul 13, 2016

@samithaf thanks again!

p.s. maybe we can hide boilerplate in some helper function but I'm not sure.

@pkishino
Copy link

Good work, but I think it would have been better to have completed this PR by updating ALL files..the generator template/hero files are still using the old style... looks a bit messy.
I"ll try to get this done today

@pkishino
Copy link

Added PR #165

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants