-
Notifications
You must be signed in to change notification settings - Fork 660
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
Conversation
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. |
b58fb67
to
9838745
Compare
@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 |
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. |
Popper.js is needed for tooltips, using the browser native title attribute removes the need to keep this
a0966b0
to
bc0213c
Compare
@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 |
8b0f692
to
ab40b7d
Compare
There was a problem hiding this 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!
@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. |
@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 |
@mmachatschek yeah we don't have a default one because we don't use prettier directly but styleci |
1f5db02
to
703da3a
Compare
There was a problem hiding this 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
This is the backported version of PR #1119
Visual comparison:
Before:
After:
before:
after:
before:
after: