Skip to content

update wiki Bootstrap story #5239 - #5248

Closed
GoNode5 wants to merge 4 commits into
angular:masterfrom
GoNode5:master
Closed

update wiki Bootstrap story #5239#5248
GoNode5 wants to merge 4 commits into
angular:masterfrom
GoNode5:master

Conversation

@GoNode5

@GoNode5 GoNode5 commented Mar 6, 2017

Copy link
Copy Markdown

Fix #5239

@googlebot

Copy link
Copy Markdown

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@delasteve delasteve left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for adding this.

I noticed the output was all in a pre-formatted block. The review comments should help you out.

"styles.css"
],
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add ``` here.

"../node_modules/tether/dist/js/tether.js",
"../node_modules/bootstrap/dist/js/bootstrap.js",
]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add ``` here

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

My first wiki pull apperently. Will try again. Thanks for your help.

to see which versions of jQuery are supported.
https://v4-alpha.getbootstrap.com/getting-started/javascript/

``json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Forgot a ` here.

@delasteve

Copy link
Copy Markdown
Contributor

There's another section at the bottom that references version 3 and version 4. While your in that file, would you mind making those inline with the others and make them version 3.x and version 4.x?

Thanks!

@clydin

clydin commented Mar 6, 2017

Copy link
Copy Markdown
Member

If this is included, a note pointing to some of the angular bootstrap projects should also be included. As they would be most likely preferred by many users.

@GoNode5

GoNode5 commented Mar 6, 2017

Copy link
Copy Markdown
Author

These are individual projects/component that have anything todo with the setup of Angular-cli.
The setting are specific to get Angular-cli working in combination with Bootstrap.

@clydin

clydin commented Mar 6, 2017

Copy link
Copy Markdown
Member

Those are alternatives to the bootstrap javascript that provide angular friendly directives/components.
If one of those is used, your added instructions are unnecessary.

@filipesilva
filipesilva requested a review from Brocco March 8, 2017 21:49
@Brocco

Brocco commented Mar 8, 2017

Copy link
Copy Markdown
Contributor

I tend to agree with @clydin that most people will prefer one of the Angular specific implementations of bootstrap's components.

If this is a route that you would like to go down, I would suggest creating a new story

Using Bootstrap Components

That covers these 3 scenarios:

Then link to this story as part of the existing bootstrap setup story.

@Brocco

Brocco commented Mar 9, 2017

Copy link
Copy Markdown
Contributor

I've been thinking about this a little more... I'm OK with adding JS-based logic to this document directly, but I still feel strongly that we should present all 3 options mentioned above.

The point of this document is providing a way for people to setup Bootstrap, not to be opinionated about the means in which they set it up. Can you please add implementation instructions for the other two options? As well as a brief paragraph above the 3 explaining what the options are in an unbiased manner?

@GoNode5

GoNode5 commented Mar 9, 2017

Copy link
Copy Markdown
Author

The question was to add a note about how to include the basic Bootstrap JS, because Bootstrap would not always work as aspected. In case of https://ng-bootstrap.github.io, you still need to add the css but not the JS files (but I have them both and seems to work). In the whole it becoming more complicated than the original remarks.
The third parties should instruct how to install in combination with Angular-cli. I don't have a bias but my experience with https://ng-bootstrap.github.io that is missed a lot of options and still alpha. That is why I'am a bit hesistant. It could also become a longer list to maintain. Maybe an 'works with Angular-cli component list'?
Ok did another try. (this my first pull request wiki...)

@filipesilva

Copy link
Copy Markdown
Contributor

@Brocco can you re-review please?

@Brocco Brocco left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have a few comments and wanted to mention that this will also need rebased.

- ng bootstrap https://ng-bootstrap.github.io (version 4.x only)
- ng2 bootstrap http://valor-software.com/ng2-bootstrap/ (version 3.x & 4.x)

You should check with the libraries for instructions how to include their project with Angular-cli. You probably only need to

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you're not going to provide specific instructions for each of these solutions, please remove this line and leave just the links.


# version 4.x
"scripts": [
"../node_modules/jquery/dist/jquery.js",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please make the spacing consistent (this is a minor nit-pick, but I've also requested other changes 😀)

@hansl hansl closed this Feb 8, 2018
@angular-automatic-lock-bot

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing addition of bootstrap.min.js to the scripts array in with bootstrap wiki-story

7 participants