CRUD
CRUD copied to clipboard
WIP: Remove spaces and new lines from column render
WHY
BEFORE - What was wrong? What was happening before this PR?
reported here: https://github.com/Laravel-Backpack/CRUD/issues/4270
Backpack columns are pieces of html when given to the datatables to render.
They include the formating your wrote on the blade file like \n and \r etc so it will be presented in the "page source-code" as you wrote them.
When displaying in browser it's fine because browser know how to interpret it. Trying to export that is not so funny! 🙃
Thing is, in datatables columns we don't need the "source code formated", we just need everything to be there, even if in one single line.
AFTER - What is happening after this PR?
We strip \n\r and eveything than's more than one white space from the returned html.
The only downside I found out until now is that something.......like....this will be transformed into: something.like.this (dots represents white spaces), anyway does anybody know any use case for double spaces ??
HOW
How did you achieve that, in technical terms?
transformed the returned html from view
Is it a breaking change?
I hope not
@tabacitu I need your opinion on this before I commit any more time here. There is also this comment: https://github.com/Laravel-Backpack/CRUD/issues/4270#issuecomment-1192525886
I think this is freaking awesome @pxpm . Is it really so small, the change? Hell yeah! 🎉 Why do you have doubts - is there a downside I'm not seeing?
Devil's advocate... would anybody WANT the newlines on... Show, for example? 🤷♂️ I don't think so... right? Wait, how about custom_html... or like a big textarea in the Show operation? Wouldn't you want newline characters there? 👀
@tabacitu I don't think it has any influence in show because we include the view directly without using that function.
That function is only used for datatables columns in getEntriesAsJsonForDatatables() in Search trait.
Damn! Good thinking @pxpm . Sounds like a really good solution then. Yes, let's do it.
What else does this need, to be merged?
@tabacitu I have only one concern that I found and mentioned earlier, I think it's no biggie, but I need approval to have peace of mind:
The only downside I found out until now is that "something.......like....this" will be transformed into "something.like.this" (dots represents white spaces)
There is also the solution to have all the field definition in one line, it feels safer to me because we don't need to str_replace the white lines, but the field will become .... something unusual!! 🙃 I reported it here: https://github.com/Laravel-Backpack/CRUD/issues/4270#issuecomment-1192525886
I have not the safe in mind to tell you merge this without us brinstorming abit about the solutions.
Cheers
Ping! I'm hoping https://github.com/Laravel-Backpack/CRUD/pull/4659 will provide an alternative solution to this, JS instead of PHP. Check it out please 🙏