Skip to content

Upgrade to Angular 1.3 Only #43

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

Merged
merged 2 commits into from
Aug 25, 2015
Merged

Conversation

erundle
Copy link

@erundle erundle commented Aug 25, 2015

Changing the project to support angular 1.3+. This required updating ngDocs and fixing the example code since angular did not like the previous "function" definition.

I made this change since we are looking in the future to add angular-bootstrap 1.3 to this project and angular 1.3 is a required.

Side Note: I also fixed a previous existing bug with warning/danger icons not showing up for notifications. This was discovered during the test of this update.

@@ -302,7 +304,7 @@ angular.module('patternfly.card').directive('pfCard', function () {
$scope.updateAvailable();
$scope.chartConfig.data.columns = [["Used",$scope.used],["Available",$scope.available]];
};
}
}]);

Choose a reason for hiding this comment

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

Nitpick: these closing brackets and the parenthesis don't line up with the angular.module above. This is also visible in the docs examples.

Copy link
Author

Choose a reason for hiding this comment

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

fixed

@dtaylor113
Copy link
Member

LGTM

@waldenraines
Copy link

Looks good to me minus the aforementioned nitpick.

@jeff-phillips-18
Copy link
Member

+1

@waldenraines
Copy link

LGTM 👍

jeff-phillips-18 added a commit that referenced this pull request Aug 25, 2015
Upgrade to Angular 1.3 Only
@jeff-phillips-18 jeff-phillips-18 merged commit 2277dc8 into patternfly:master Aug 25, 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.

4 participants