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 JS Bin template in CONTRIBUTING.md #14111

Merged
merged 1 commit into from
Jul 12, 2014
Merged

Conversation

hnrch02
Copy link
Collaborator

@hnrch02 hnrch02 commented Jul 10, 2014

Refs: #14109

Link: http://jsbin.com/zazif/1/edit?html,output

That bin still refers to https://github.com/necolas/issue-guidelines for bug reporting guidelines, is that still valid?

Also, I switched to using a CDN for the Bootstrap files, as @cvrebert mentioned a couple of times that we don't endorse hotlinking to getbootstrap.com
The reason why I chose jsDelivr over BootstrapCDN is that they actually have a up-to-date /latest/ version. (See jsDelivr and BootstrapCDN.)

/cc @cvrebert

@juthilo
Copy link
Collaborator

juthilo commented Jul 10, 2014

/cc @jdorfman about BootstrapCDN's /latest/ being out of date.

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Jul 10, 2014

It's only for .min files though.

@cvrebert
Copy link
Collaborator

@youngtaek I really hate you and your email provider right now..

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Jul 10, 2014

@cvrebert It's not only you...

@cvrebert
Copy link
Collaborator

To make debugging easier, it's probably better not to use the minified code.

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Jul 10, 2014

The updated template I posted uses the unminified files.

@jdorfman
Copy link
Contributor

@juthilo fixing now.

@jdorfman
Copy link
Contributor

@juthilo @hnrch02 CDN Cache Purged. Sorry for the oversight.

@juthilo
Copy link
Collaborator

juthilo commented Jul 10, 2014

@jdorfman No worries, thanks for the quick fix!
@hnrch02 You can now use http://maxcdn.bootstrapcdn.com/bootstrap/latest/css/bootstrap.css ;)

@cvrebert
Copy link
Collaborator

@cvrebert
Copy link
Collaborator

Also, the bin is missing the viewport meta tag and the X-UA-Compatible meta tag.

@cvrebert
Copy link
Collaborator

It might be worth considering including the IE10 viewport hack too:
https://github.com/twbs/bootstrap/blob/master/docs/assets/js/ie10-viewport-bug-workaround.js

@cvrebert cvrebert added this to the v3.2.1 milestone Jul 10, 2014
@hnrch02
Copy link
Collaborator Author

hnrch02 commented Jul 10, 2014

Then we'd also need to include the HTML5 shim and Respond.js.

@cvrebert
Copy link
Collaborator

🤘 Good catch. It should really be based upon the basic getting started template.

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Jul 10, 2014

Bad news, the validation fails for the X-UA-Compatible meta tag.

@cvrebert
Copy link
Collaborator

@hnrch02 It's being overly pedantic, just ignore that. LMVTFY specifically allows X-UA-Compatible.

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Jul 10, 2014

Mkay, will update the PR.

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Jul 10, 2014

Should I include the IE10 viewport bug workaround?

@mdo
Copy link
Member

mdo commented Jul 12, 2014

For the example:

  • Add <meta http-equiv="X-UA-Compatible" content="IE=edge">
  • Add <meta name="viewport" content="width=device-width, initial-scale=1">
  • Don't bother with the IE10 viewport thing

Once set, then we can merge.

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Jul 12, 2014

@mdo How about this Bin?

@mdo
Copy link
Member

mdo commented Jul 12, 2014

Looks good.

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Jul 12, 2014

Okay, done.

mdo added a commit that referenced this pull request Jul 12, 2014
Update JS Bin template in CONTRIBUTING.md
@mdo mdo merged commit 4afe40d into twbs:master Jul 12, 2014
@cvrebert cvrebert mentioned this pull request Jul 12, 2014
@hnrch02 hnrch02 deleted the bug-report-template branch July 12, 2014 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants