Skip to content

Conversation

@dtaylor113
Copy link
Member

...to README.md

@dtaylor113
Copy link
Member Author

Hi, do we require angular-bootstrap now?

<script src="bower_components/angular-bootstrap/ui-bootstrap-tpls.min.js"></script>

Copy link
Member

Choose a reason for hiding this comment

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

Only the patternfly.charts module is optional. I'm wondering if we should show the addition of 'patternfly' to the myApp module. Pretty basic and I'd think angular devs would know to do this but calling it out for charts leads me to think we should call it out for the base.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what you are suggesting?

Copy link
Member

Choose a reason for hiding this comment

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

I'm suggesting we might want to add, adding the patternfly module to the dependencies:

angular.module('myApp', [
'patternfly'
]);

and then state that optionally, add the charts dependency.

@jeff-phillips-18
Copy link
Member

LGTM. Issue above was more of a question than anything else.

README.md Outdated
Copy link

Choose a reason for hiding this comment

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

I don't think this gives you everything since they still need to add in angular etc.

@dtaylor113
Copy link
Member Author

Hi, addressed Eleni's comments.

Still have the question if we require angular-bootstrap?

       <script src="bower_components/angular-bootstrap/ui-bootstrap-tpls.min.js"></script>

Thanks,

  • Dave

erundle pushed a commit that referenced this pull request Oct 6, 2015
Added "Using Angular-PatternFly In Your Application" section
@erundle erundle merged commit 36ace7f into patternfly:master Oct 6, 2015
@dtaylor113 dtaylor113 deleted the readme branch October 13, 2015 19:34
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.

3 participants