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 #1405

Merged
merged 14 commits into from
Apr 8, 2024

Conversation

mmachatschek
Copy link
Contributor

@mmachatschek mmachatschek commented Mar 29, 2024

This is the backported version of PR #1119

Visual comparison:

Before:

Bildschirmfoto 2024-04-04 um 21 30 58

After:

Bildschirmfoto 2024-04-04 um 21 31 06

before:
Bildschirmfoto 2024-04-04 um 21 31 27

after:

Bildschirmfoto 2024-04-04 um 21 31 38

before:

Bildschirmfoto 2024-04-04 um 21 31 52

after:

Bildschirmfoto 2024-04-04 um 21 31 58

Copy link

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

@mmachatschek mmachatschek force-pushed the update_bootstrap5_5.x branch from b58fb67 to 9838745 Compare April 2, 2024 17:25
@mmachatschek
Copy link
Contributor Author

mmachatschek commented Apr 2, 2024

@driesvints this is the resubmitted version of PR #1119

I currently don't have a full horizon setup available so I can not test this thoroughly enough. If you are ok with that, I could leave it open to others to pick up the work and add the final touches.

I explicitly set the bootstrap version to v5.1 as I originally only did the changes for boostrap v5.1, the 5.2 changes can be made in a followup PR by others. although I think maybe for v6 a complete overhaul with a new tailwind made ui is probably the way to go

@driesvints
Copy link
Member

Hey @mmachatschek. Thank you for your efforts. I unfortunately also don't have the time atm. Would be great if anyone could help out here.

@mmachatschek mmachatschek force-pushed the update_bootstrap5_5.x branch from a0966b0 to bc0213c Compare April 4, 2024 19:28
@mmachatschek mmachatschek marked this pull request as ready for review April 4, 2024 19:34
@mmachatschek
Copy link
Contributor Author

@driesvints i setup two basic laravel horizon apps on my side and pointed to the same horizon process just with different published versions of the ui. The visual comparison is fairly the same (after some tiny modifications) now.

Generally bootstrap made the ui a little bit denser so thats kind of a good point.

From my side its now ready for review

@mmachatschek mmachatschek force-pushed the update_bootstrap5_5.x branch from 8b0f692 to ab40b7d Compare April 4, 2024 19:37
Copy link
Member

@driesvints driesvints left a comment

Choose a reason for hiding this comment

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

@mmachatschek thanks for your efforts!

@driesvints
Copy link
Member

@mmachatschek might be a bit too much for you but any chance you can undo all of these indenting changes in the vue files etc? Would make this PR a whole lot smaller and easier to review.

@mmachatschek mmachatschek marked this pull request as draft April 5, 2024 08:14
@mmachatschek
Copy link
Contributor Author

mmachatschek commented Apr 5, 2024

@driesvints sorry about that, VScode applies auto formatting. I convert the PR to draft and reapply the changes. A checked-in prettier config would be nice so that editors automatically pick that up and don't mess up the code

@driesvints
Copy link
Member

@mmachatschek yeah we don't have a default one because we don't use prettier directly but styleci

@mmachatschek mmachatschek force-pushed the update_bootstrap5_5.x branch from 1f5db02 to 703da3a Compare April 5, 2024 18:39
@mmachatschek mmachatschek marked this pull request as ready for review April 5, 2024 18:39
@mmachatschek mmachatschek requested a review from driesvints April 5, 2024 18:39
Copy link
Member

@driesvints driesvints left a comment

Choose a reason for hiding this comment

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

Think this is a good update. Thanks @mmachatschek

@taylorotwell taylorotwell merged commit f19486e into laravel:5.x Apr 8, 2024
10 checks passed
@mmachatschek mmachatschek deleted the update_bootstrap5_5.x branch April 9, 2024 17:55
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.

3 participants