filamentphp / filament

A collection of beautiful full-stack components for Laravel. The perfect starting point for your next app. Using Livewire, Alpine.js and Tailwind CSS.
https://filamentphp.com
MIT License
19.4k stars 2.97k forks source link

Fix CheckboxList does not display descriptions when using an enum for dynamic options #14501

Closed standaniels closed 1 month ago

standaniels commented 1 month ago

Closes #14500

As this is my first pull request I need help with checking if this PR breaks existing functionality.

Description

When rendering a CheckboxList field with dynamic options where an enum is returned by the options closure, the descriptions are not displayed. This issue occurs even though the enum implements both HasDescription and HasLabel.

Visual changes

None.

Functional changes

ceejayoz commented 1 month ago

I think this behavior might be intentional - not every CheckboxList needs descriptions, even if the enum has them.

You can already do ->options(Enum::class)->description(Enum::class) and it'll pick it up.

standaniels commented 1 month ago

I agree not every CheckboxList needs descriptions, but when you pass an enum class to the options method descriptions are automatically added if this enum implements HasDescription. See the test in the first commit of this PR:

it('can render descriptions when enum implements HasDescription', function () {
    expect(Options::class)->toImplement(HasDescription::class);
    livewire(TestComponentWithFixedOptions::class)
        ->assertSeeText('Foo Description')
        ->assertSeeText('Bar Description');
});

This tests passes in the current version of Filament if you do ->options(Enum::class).

EDIT Also, descriptions only accepts array | Arrayable | Closure. So ->descriptions(Enum::class) doesn't work.

danharrin commented 1 month ago

The reason why it's like this is because evaluating the options closure twice (once to fetch options, once to fetch descriptions) could cause queries to be duplicated etc. ->options(EnumName::class) should work, but ->options(fn () => EnumName::class) won't. If you would like to use a dynamic enum like this then you will need to define your own function in ->descriptions() to return the array of descriptions.

standaniels commented 1 month ago

Alright, I understand. Thanks for the clarification!