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

Update dependencies #89

Merged

Conversation

alechenninger
Copy link
Contributor

Related was changing from using angular-strap to angular-ui-bootstrap which is maintained by angular team and much simpler and more angular-y to use. You can see in the code it is a lot simpler.

The first commit is only bower_components, so it can be ignored. The second commit is where I modified app to consume updated deps including the bootstrap directive usage.

@alechenninger
Copy link
Contributor Author

Use this diff to review (has no bower_components in it): alechenninger@f4cf82e

@coveralls
Copy link

Coverage Status

Coverage remained the same at 22.22% when pulling f4cf82e on alechenninger:update-dependencies into 74ae58c on lightblue-platform:master.

@alechenninger
Copy link
Contributor Author

Another part of the motivation of moving to ui-bootstrap is that the alert component is also easier to use that angular-strap's counterpart, and I want to use this to address #65 .

@alechenninger
Copy link
Contributor Author

As part of the environment dropdown dependency refactor I added the ng-cloak directive to the dropdown. This helps address the ugliness that can happen as mentioned in #91 .

@coveralls
Copy link

Coverage Status

Coverage remained the same at 22.22% when pulling 22628d8 on alechenninger:update-dependencies into 74ae58c on lightblue-platform:master.

@dcrissman
Copy link
Member

It feels like this is a more complicated PR than the subject would indicate. Should this possibly be multiple PRs?

@alechenninger
Copy link
Contributor Author

I don't think so really. It just looks big because I changed / removed a bunch of dependencies. Updating dependencies involved finding compatible versions for everything. The original plugin for the dropdown select made that more complicated than it had to be, so I changed out that dependency for a different one. Now the dependencies are much simpler. And changing out that dependency actually only involved removing code. The only code that was added was one directive, "ng-cloak" on the dropdown.

@alechenninger
Copy link
Contributor Author

You can see what "really" changed from this single commit: alechenninger@f4cf82e

@alechenninger
Copy link
Contributor Author

Well, sorry, I lied a little. I had to change the index template a little bit for the new dropdown directive but, it's a very simple change, mostly rearranging what was already there.

@alechenninger
Copy link
Contributor Author

poke, nudge ... is anyone looking at this? Are there any questions? I'm waiting on this for some usability improvements that will need the angular-ui-bootstrap dependency which was swapped in.

dcrissman added a commit that referenced this pull request Apr 17, 2015
@dcrissman dcrissman merged commit 04f9e56 into lightblue-platform:master Apr 17, 2015
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.

None yet

4 participants