filament icon indicating copy to clipboard operation
filament copied to clipboard

Published 20 hours ago • verified publisher icon filamentphp

feature: Add affixes to BooleanColumn and IconColumn

Open ralphjsmit opened this issue 1 year ago • 12 comments

This PR adds the ability to add suffixes and prefixes to boolean columns. This is handy in situations where you want to add some more context to a boolean, but where you don't want to create a new text column (e.g. because there's too much whitespace).

Schermafbeelding 2022-08-27 om 15 35 00

ralphjsmit avatar Aug 27 '22 13:08 ralphjsmit

It feels like for your use case you could use a badge column instead of boolean? I'm not sure if we should 'bloat' the boolean column with a prefix and suffix, feels wrong to me in a way.

zepfietje avatar Aug 27 '22 22:08 zepfietje

I think it should be on both, also IconColumn? do we have that or am i making it up

danharrin avatar Aug 27 '22 22:08 danharrin

It feels like for your use case you could use a badge column instead of boolean? I'm not sure if we should 'bloat' the boolean column with a prefix and suffix, feels wrong to me in a way.

No, because a badge column really displays a badge with a colored background, which looks much uglier.

ralphjsmit avatar Aug 29 '22 07:08 ralphjsmit

I think it should be on both, also IconColumn? do we have that or am i making it up

I'm not sure either, but I'll check and update the PR!

ralphjsmit avatar Aug 29 '22 07:08 ralphjsmit

Added IconColumn, so ready for review. @danharrin

ralphjsmit avatar Aug 29 '22 15:08 ralphjsmit

I'm interested now... can we somehow refactor BooleanColumn to extend IconColumn?

danharrin avatar Aug 29 '22 17:08 danharrin

I'm interested now... can we somehow refactor BooleanColumn to extend IconColumn?

Exactly my thought!

zepfietje avatar Aug 29 '22 17:08 zepfietje

I'm interested now... can we somehow refactor BooleanColumn to extend IconColumn?

I think that it should be possible. I'll take a look later this week.

ralphjsmit avatar Aug 29 '22 18:08 ralphjsmit

Maybe create a new component and call it IndicatorColumn?

ArtMin96 avatar Aug 31 '22 18:08 ArtMin96

Maybe create a new component and call it IndicatorColumn?

Personally I don't see the benefit of renaming IconColumn to IndicatorColumn. Or am I understanding you incorrectly?

Not sure what @danharrin thinks.

ralphjsmit avatar Sep 01 '22 14:09 ralphjsmit

Agree. Let's just make BooleanColumn extend IconColumn and see how that works out first.

zepfietje avatar Sep 01 '22 14:09 zepfietje

Maybe create a new component and call it IndicatorColumn?

Personally I don't see the benefit of renaming IconColumn to IndicatorColumn. Or am I understanding you incorrectly?

Not sure what @danharrin thinks.

For each component to be responsible for the actions assigned to it, BooleanColumn should be boolean, IconColumn should be an icon, and IndicatorColumn should be the indicator column. Also, in the case of further changes, problems should not arise due to connecting the BooleanColumn to the IconColumn. This is just an opinion, you know Filament better than me

ArtMin96 avatar Sep 01 '22 14:09 ArtMin96

What's the status of this PR, @ralphjsmit? 🙂

We've decided to deprecate BooleanColumn in favor of IconColumn::make()->boolean(): https://github.com/filamentphp/filament/issues/4273. So that will be handled separately from this PR.

zepfietje avatar Oct 08 '22 11:10 zepfietje

https://github.com/filamentphp/filament/pull/4431 has been merged and deprecates the BooleanColumn. Could you update this PR to add affix support to the IconColumn?

zepfietje avatar Oct 08 '22 22:10 zepfietje

Gonna close this, many merge conflicts so it would be easier to do the work on a new branch anyway

danharrin avatar Nov 02 '22 17:11 danharrin