[Bug Fix] Wrapper will not print false attributes

promatik commented 7 months ago

This is a fix for both;

WHY

BEFORE - What was wrong? What was happening before this PR?

Some links where empty:

AFTER - What is happening after this PR?

No more empty links.

HOW

How did you achieve that, in technical terms?

All wrapper_start files (for columns, fields and widgets) will check if the value of attribute is NOT false.

Basically if someone has a closure for an HREF, or any other attribute, that attribute will be always printed in the HTML Dom element, even if the value is false, or null.

With this PR, developers can control that by returning false on the closure, it's a strict validation so any falsy value will be printed anyway, 0, null (even though null is not really printed on the html).

null	`href=""`
0	`href="0"`
""	`href=""`
false

Is it a breaking change?

I don't believe so. This is only a breaking change for users who were returning false on some closure, and were expecting the attribute to be rendered empty.

How can we test the before & after?

By following @karandatwani92 suggestion here https://github.com/Laravel-Backpack/CRUD/pull/5391#issuecomment-1872219557

pxpm commented 7 months ago

I think this is a major BC 😢 Not only on the case you mentioned, but it would also prevent developers from adding ANY custom attribute with value false to the wrapper.

// ...
'wrapper' = > [
'customAttr' => false,
]

I will be merging the fix for linkTo() in a separate PR until we figure out if we can do this on a non-breaking way.

🙏

promatik commented 7 months ago

@pxpm as we discussed, this is now validation href only 👌

Laravel-Backpack / CRUD