Skip to content

tweak stuff to work with latest laravel in blade and bootstrap 4 #193

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
Closed
wants to merge 1 commit into from
Closed

tweak stuff to work with latest laravel in blade and bootstrap 4 #193

wants to merge 1 commit into from

Conversation

@rjacobsen2012
Copy link

@rjacobsen2012 rjacobsen2012 commented Oct 24, 2023

I fixed some minor spelling issues, made it work with bootstrap 4, and did some style tweaks for anyone who wants to use it.
MikeAlhayek
MikeAlhayek approved these changes Oct 24, 2023
View reviewed changes
Copy link
Member

@MikeAlhayek MikeAlhayek left a comment

Choose a reason for hiding this comment

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

Can you please add new line at each file where the workflow is complaining about?
Also, did you test the samples commands and make sure they generate properly?

It would be nice to see a PR adding support with Bootstrap 5.3.
@rjacobsen2012
Copy link
Author

rjacobsen2012 commented Oct 24, 2023

Can you please add new line at each file where the workflow is complaining about? Also, did you test the samples commands and make sure they generate properly?

It would be nice to see a PR adding support with Bootstrap 5.3.

I am not a huge fan of Bootstrap 5.3 since they removed a bunch of stuff everyone was using with no backwards compatibility. I ran the samples locally, and they worked. Where are you seeing the new line errors?
@rjacobsen2012
Copy link
Author

rjacobsen2012 commented Oct 24, 2023 •
edited
Loading

Can you please add new line at each file where the workflow is complaining about? Also, did you test the samples commands and make sure they generate properly?

It would be nice to see a PR adding support with Bootstrap 5.3.

Also, there seems to be a bug when adding the route names that adds an extra model name in the name. I looked to try to find and fix it, but not sure where you are doing that.

image

It should be amounts.update, amounts.index, amounts.edit, etc. Also, you should look at just making resourceful routes instead, and let the user add extra ones if needed. https://laravel.com/docs/10.x/controllers#resource-controllers
@MikeAlhayek
Copy link
Member

MikeAlhayek commented Oct 24, 2023

I have not tested out your changes. Just want to ensure they are tested fully.

Also, the same changes should made on the default theme should also be made on the collective theme as well.

Bootstrap 5.3 is pretty nice.
@rjacobsen2012
Copy link
Author

rjacobsen2012 commented Oct 24, 2023

I have not tested out your changes. Just want to ensure they are tested fully.

Also, the same changes should made on the default theme should also be made on the collective theme as well.

Bootstrap 5.3 is pretty nice.

I may see if I can update to latest Carbon as well.
image
@MikeAlhayek
Copy link
Member

MikeAlhayek commented Jan 24, 2024

I am going to close this PR. Sorry. master branch already contains template for Laravel 5.3 with theming support. In the next couple of days I am planning to release 3.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants

@rjacobsen2012 @MikeAlhayek