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.

Sign up for GitHub

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

Jump to bottom

Support jQuery v3 in Bootstrap v4 #20191

Merged
merged 5 commits into from
Jun 29, 2016
Merged

Support jQuery v3 in Bootstrap v4 #20191

merged 5 commits into from
Jun 29, 2016

Conversation

cvrebert
Copy link
Collaborator

@cvrebert cvrebert commented Jun 28, 2016 •
edited
Loading

@cvrebert cvrebert added this to the v4.0.0-alpha.3 milestone Jun 28, 2016
@XhmikosR
Copy link
Member

XhmikosR commented Jun 28, 2016 •
edited
Loading

Definitely like this!

I remember though, there were some issues with 3.x alpha. Do those still exist?

On a side note, can't we use the slim version? We'd need to switch CDNs to jQuery's one for this though.

@cvrebert
Copy link
Collaborator Author

What is this slim version you speak of?

@XhmikosR
Copy link
Member

https://blog.jquery.com/2016/06/09/jquery-3-0-final-released/

Apparently, they haven't added it yet to downloads

@cvrebert
Copy link
Collaborator Author

I have no objection to switching to https://code.jquery.com/jquery-3.0.0.slim.min.js

@XhmikosR
Copy link
Member

XhmikosR commented Jun 28, 2016 •
edited
Loading

@cvrebert: so it seems the slim build is on Google CDN it's just not shown in the frontend.

https://ajax.googleapis.com/ajax/libs/jquery/3.0.0/jquery.slim.min.js

You also missed docs/assets/js/vendor/jquery.min.js.

@cvrebert
Copy link
Collaborator Author

I remember though, there were some issues with 3.x alpha. Do those still exist?

jQuery Team fixed jquery/jquery#2828, and they backpedaled a bit on the breaking change to .show()/.hide().
#20019 would apply to v4 too, but I'm not sure it's valid; I've asked for clarification.
All our unit tests pass, but obviously any further testing would of course be helpful.

I read through https://jquery.com/upgrade-guide/3.0/ and nothing else seemed applicable.
jQuery Migrate 3 (https://jquery.com/upgrade-guide/3.0/#jquery-migrate-plugin ) didn't flag anything when running the test suite.

@XhmikosR
Copy link
Member

All right, so it seems we can merge this and check the slim build after that. For 3.x I guess we still need to stick to 1.x branch, right?

@cvrebert
Copy link
Collaborator Author

you also missed docs/assets/js/vendor/jquery.min.js

Good catch. Fixed.

For 3.x I guess we still need to stick to 1.x branch, right?

For the docs at least, yeah, because IE8.
Making 3.x compatible with 3.x (without changing the default) is #16834.

@cvrebert
Copy link
Collaborator Author

Looks like we still have some heisenbugs. There are a couple failures on Sauce:

@cvrebert cvrebert mentioned this pull request Jun 28, 2016
Closed
@XhmikosR
Copy link
Member

Yeah, I saw those too but I thought it was just Saucelabs to blame.

Better wait until everything is sorted.

dmethvin added a commit to dmethvin/bootstrap that referenced this pull request Jun 28, 2016
@dmethvin
Fix second scrollspy unit tests for async .then() in jQuery 3
76fc2be 
Fixes twbs#20191
@dmethvin dmethvin mentioned this pull request Jun 28, 2016
Merged
cvrebert pushed a commit that referenced this pull request Jun 28, 2016
@dmethvin @cvrebert
ScrollSpy unit test: Use single done() in a then() instead of multipl…
8900d95 
…e done()s (#20198)

Fixes test flakiness on OS X Safari and Android when using jQuery 3.
Fixes #20182 more
Refs #20191
@cvrebert
Copy link
Collaborator Author

Ok, build is green after rebasing since #20198.

@cvrebert
Copy link
Collaborator Author

cvrebert commented Jun 29, 2016 •
edited
Loading

Since all the close/dismiss buttons in the v4 docs use <button> instead of <a>, I think we can ignore #20019 for v4.
Does that sound good to folks?

@XhmikosR
Copy link
Member

If that's true, it makes sense, yeah.

@cvrebert cvrebert merged commit ed3ba47 into v4-dev Jun 29, 2016
@cvrebert cvrebert deleted the bs4-jq3 branch June 29, 2016 05:19
@cvrebert cvrebert mentioned this pull request Jun 29, 2016
Closed
@XhmikosR
Copy link
Member

Should I go ahead and make a PR with the slim version?

@cvrebert
Copy link
Collaborator Author

👍

cvrebert pushed a commit that referenced this pull request Jul 8, 2016
@dmethvin @cvrebert
Port #20198 to v3
bc05ef1 
ScrollSpy unit test: Use single done() in a then() instead of multiple done()s

Fixes test flakiness on OS X Safari and Android when using jQuery 3.
Fixes #20182 more
Refs #20191
chiraggmodi pushed a commit to chiraggmodi/bootstrap that referenced this pull request Apr 8, 2019
@dmethvin @chiragatlas
Port twbs#20198 to v3
a615851 
ScrollSpy unit test: Use single done() in a then() instead of multiple done()s

Fixes test flakiness on OS X Safari and Android when using jQuery 3.
Fixes twbs#20182 more
Refs twbs#20191
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
feature js meta v4
Projects
None yet
Milestone
v4.0.0-alpha.3
Development

Successfully merging this pull request may close these issues.
2 participants
@cvrebert @XhmikosR