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

[5.x] Update to bootstrap 5.1 and drop jQuery #1179

Merged

Conversation

mmachatschek
Copy link
Contributor

@mmachatschek mmachatschek commented Feb 2, 2022

This PR updates bootstrap from 4.6 to 5.1 and applies the migration steps from https://getbootstrap.com/docs/5.1/migration/

Additionally I managed to remove jQuery and replace its usage with vanilla JS.

@mmachatschek mmachatschek force-pushed the update_bootstrap_drop_jquery branch from 470e696 to 3aef2a3 Compare February 3, 2022 11:25
@taylorotwell
Copy link
Member

Converting this to draft while @themsaid reviews it. Please mark as ready once reviewed.

@themsaid
Copy link
Member

Screen Shot 2022-02-10 at 5 31 58 PM

The header icons are out of place


Screen Shot 2022-02-10 at 5 32 23 PM

Code blocks are overflowing


Other than these, everything looks ok.

Fix overflowing text in json pretty output
@mmachatschek
Copy link
Contributor Author

@themsaid the overflowing text in the json output is also a problem on the 4.x branch. Fixed it.

I found one missing utility class that had the wrong prefix.

The navigation is working fine on my side, can you try it again. After switching to the new branch I had to do a full reload with cache invalidation in the browser.

@mmachatschek mmachatschek marked this pull request as ready for review February 10, 2022 18:05
@themsaid themsaid marked this pull request as draft February 11, 2022 10:32
@themsaid
Copy link
Member

Screen Shot 2022-02-11 at 12 31 58 PM

A black border appears under every table header now.

@mmachatschek
Copy link
Contributor Author

@themsaid this is new since bootstrap v5 https://getbootstrap.com/docs/5.1/content/tables/ every table has a border under the table-head.

Should I remove it?
https://github.com/twbs/bootstrap/blob/a7942190c746b4aeefc5e7680cb396b1885670aa/scss/_tables.scss#L44

@taylorotwell
Copy link
Member

Yes please remove that.

@mmachatschek mmachatschek marked this pull request as ready for review February 13, 2022 07:39
@taylorotwell taylorotwell merged commit 572a58d into laravel:master Feb 14, 2022
@mmachatschek mmachatschek deleted the update_bootstrap_drop_jquery branch February 14, 2022 20:33
driesvints added a commit that referenced this pull request Dec 12, 2023
driesvints added a commit that referenced this pull request Dec 12, 2023
@driesvints
Copy link
Member

We're very sorry @mmachatschek but we had to revert this PR because it majorly conflicted with #1295 in almost every Vue file. We decided to revert this one so we could merge 4.x into 5.x for preparation of Laravel v11 support. We're definitely still open to this PR but we'll need a fresh one.

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.

4 participants