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

Include ngOfficeUIFabric to be able to use the Office UI Fabric components? #145

Closed
ghost opened this issue Mar 1, 2016 · 13 comments
Closed

Comments

@ghost
Copy link

ghost commented Mar 1, 2016

I would like to add an option to the generator to be able to include ngOfficeUIFabric. This way a developer could easily leverage this repository to use the Office UI Fabric components in his application without needing to copy/paste HTML. Is that OK?

@jthake
Copy link
Contributor

jthake commented Mar 1, 2016

That sounds like a great idea. You'd only be adding this to the Angular language flow right?

@ghost
Copy link
Author

ghost commented Mar 1, 2016

Yes. So, if Angular is selected => next question is to include ngOfficeUIFabric.

Currently there are a ng and a ng-adal tech. Should I add 'ng-officeuifabric' as well? But what if a user wants both ADAL and Office UI Fabric?

@jthake
Copy link
Contributor

jthake commented Mar 1, 2016

mmm I would add thsi by default. we use fabric already i believe in these templates. Can ngOfficeUIFabric coexist fine with fabric already referenced? As I know this doesnt' do everything that Fabric can do directly just yet.

@ghost
Copy link
Author

ghost commented Mar 1, 2016

That is no problem. ngOfficeUIFabric does not include the OfficeUIFabric CSS files, only a set of Angular directives.
But you are right that not all directives have been implemented yet, there are 4 directives that have not been assigned yet. All other directives have either been implemented or are about to be released.

@jthake
Copy link
Contributor

jthake commented Mar 1, 2016

So I'd suggest just having us use this for the angular selection.

@ghost
Copy link
Author

ghost commented Mar 2, 2016

OK All good! Can this issue be assigned to me please?

Because of issue #146 the Angular module is not instantiated, so 146 needs to be merged first before I can submit this one.

Question about the text of the question:

Include ngOfficeUIFabric (Angular Directives for Office UI Fabric)? (Y/n)

OK?

@andrewconnell
Copy link
Contributor

LGTM from a pure code change... the new option is definitely welcomed IMHO

@ghost
Copy link
Author

ghost commented Mar 11, 2016

@jthake Can you please review the PR and if OK, merge it? I'd like to write a blog post about it.

@jthake
Copy link
Contributor

jthake commented Mar 11, 2016

appreciate your eagerness here. I'm buried with Build 2016 event planning right now. I need to test this myself and have not had time. This'll have to wait until at least next week if not after build in all honesty.

@beth-panx
Copy link
Member

Hey @rolandoldengarm could you take a look at the comments on the pr #157 you submitted? I'd be happy to test it again when you are done. Or else I will close both soon. :)

@ghost
Copy link
Author

ghost commented Dec 8, 2016

@beth-panx sorry as it has been a long time ago, I don't have the repo configured on my machine anymore. I'll need to set it up again first, will try to do it next week! Don't close it yet please :)

@beth-panx
Copy link
Member

Awesome!! Thank you 💯

@beth-panx
Copy link
Member

In the new structure, we included Office UI fabric in package.json which uses the new Fabric Core and Fabric JS. #157

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

No branches or pull requests

3 participants