Closed shopapps closed 2 weeks ago
Hey, just thought i add some documentation for you :-)
If the Column Header contains HTML, the csv export renders this. Adding strip_tags to line 133 of FilamentExport fixes this.
I think it would be more helpful to have something like formatHeaders
which would be applied to $this->getAllColumns()->map(fn ($column) => $column->getLabel())..
. I don't think we need explicit state
variable if we can always call $column->getState()
(it looks like just shortcut). Wdyt @alperenersoy ?
Ref $state you are correct it is purely a convenience, but it is also the expected filamentphp approach so without following documentation many would expect to be able to do something along the lines of:
=> fn(string $state) :string => formatMyStateString($state),
:-)
@shopapps The point about Filament standartization is relevant to me. It make sense to follow the convention. So i would leave as you did it and only added feature with formatHeaders
to not strip_tags
by default (but being able to do it somehow).
for formatStates
documentation I am not sure if it needs a whole other section since it's documented under the Full Usage
section and it's pretty much a self-explanatory thing.
and for headers, I think customization with sth like formatHeaders
would be better than hard-coding strip_tags but also we are doing it for view columns in https://github.com/alperenersoy/filament-export/blob/main/src/FilamentExport.php#L397. I did not check the code but if we know it's an HTML label we can use strip_tags
by default to make it more readable. But also having formatHeaders
to customize it further would be nice in this case.
@shopapps sorry for not reviewing it earlier. I was just struggling to keep myself motivated.
@alperenersoy I've created https://github.com/alperenersoy/filament-export/pull/134
Added example to readme.md Added 'string $state' as a possible dependency injection param for closures passed to ->formatStates()