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

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

Closed
wants to merge 1 commit into from
Closed

Conversation

rjacobsen2012
Copy link

I fixed some minor spelling issues, made it work with bootstrap 4, and did some style tweaks for anyone who wants to use it.

Copy link
Collaborator

@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

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

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
Collaborator

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

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
Collaborator

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants