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

[Feature/Bug fix] Added the ability to separate base/crud assets more easily #224

Merged
merged 7 commits into from
Feb 12, 2017

Conversation

OwenMelbz
Copy link
Contributor

Have added a new default layer of separation for crud specific css/js assets.

By default users have no way of easily customising the css/js of the CRUD level features

Additionally contributors have no way to make crud specific css/js fixes/features without effecting base.

This adds a series of files list, create, edit, form, revisions, show, reorder etc into the public directory that can be modified

@tabacitu
Copy link
Member

@OwenMelbz, I've gotten used to the idea of providing these JS and CSS files, so I'm going to merge this into 3.2

One thing that's related to this and I would have loved: getting rid of the JS that is currently in the blade files, and move it to external .js files. Just look at all the JS that ended up in list.blade.php.

The thing is, that JS is in blade files for a reason: it's intertwined with PHP, a lot. I see two options to separate this JS, but I don't like either of them:

  1. Make the .css and .js files actually .blade.php files that contain <style></style> and <script></script>. This would mean we store them inside /resources/views/backpack/crud/js and /resources/views/backpack/crud/css, which... doesn't feel right...
  2. Keep the PHP conditionals inside list.blade.php and all other blade files, but just store the result inside some global JS variable (var crud). Then in the .js files, check for those variables and do js operations according to them. This would work pretty ok for most operations, but there are also things like using translation strings, which are bulky. In the end, this would still mean we have JS both in the .blade.php file and in the .js file, so why bother? It would probably end up even more complicated and bring in a flood of questions.

Do you see any other option? Do you like any of the above?

Thanks!

@OwenMelbz
Copy link
Contributor Author

Yeah I know what you mean about the inlining of js, and even css there is a lot of.

I think the most acceptable reason is also the largest rewrite, it is pretty much what is outlined -> #148

But the concept would be to handle everything via data attributes, so json encoding it and passing it to the JS component which will then handle the logic, this could also work for translations.

However it is a large change, maybe a big ol refactor for 4 :D

So for maybe now, leaving them inline is the best choice

@tabacitu
Copy link
Member

Pff... yeah, I agree. Thank you :-)

@tabacitu
Copy link
Member

Merged into 3.2.
Thank you!

@tabacitu tabacitu merged commit 08fff59 into 3.2 Feb 12, 2017
@tabacitu tabacitu removed the ready label Feb 12, 2017
@tabacitu tabacitu deleted the feature-state-customisations branch November 18, 2019 08:34
This was referenced Apr 2, 2020
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.

2 participants