Conversation
A helper method to the Column class to set column properties directly from an enum.
There was a problem hiding this comment.
Pull request overview
Adds a convenience factory on Column to build a column directly from a backed enum value, optionally applying Filament enum metadata (label/color/icon) when available.
Changes:
- Introduces
Column::enum(BackedEnum $enum)static helper. - Sets column name from
$enum->valueand conditionally derives label/color/icon from Filament enum contracts.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return $static; | ||
| } | ||
|
|
||
| public static function enum(BackedEnum $enum): static |
There was a problem hiding this comment.
BackedEnum is referenced without being imported or fully-qualified. Inside the Relaticle\Flowforge namespace this will resolve to Relaticle\Flowforge\BackedEnum and cause an autoload/Class not found error. Import BackedEnum (e.g. use BackedEnum;) or change the typehint to \BackedEnum.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
ManukMinasyan
left a comment
There was a problem hiding this comment.
Clean, focused addition. The Column::enum() factory method correctly handles:
- Both string-backed and int-backed enums via
BackedEnumtype hint - Graceful degradation when enum only partially implements Filament contracts (HasLabel, HasColor, HasIcon)
- Consistent fluent API (delegates to existing
make(),label(),color(),icon())
Minor suggestions (non-blocking):
- Consider using import statements for the Filament contracts instead of inline FQCNs
- A docblock on the new method would help discoverability
- Tests would be nice in a follow-up (basic enum, partial interfaces, int-backed enum)
LGTM -- ready to merge.
ManukMinasyan
left a comment
There was a problem hiding this comment.
Updating my review -- the implementation itself is clean and correct, but this needs test coverage before merging.
Required: Tests
The Column::enum() method introduces new public API that should be tested. At minimum:
- String-backed enum with all interfaces (HasLabel + HasColor + HasIcon) -- verify label, color, icon are set
- Enum with partial interfaces (e.g., only HasLabel) -- verify it doesn't error on missing interfaces
- Enum without any Filament interfaces -- verify it falls back to basic
make()behavior - Int-backed enum -- verify
$enum->valueworks as column identifier
Example test structure:
test('enum helper creates column from backed enum with label, color, and icon', function () {
$column = Column::enum(TestStatusEnum::Active);
expect($column->getName())->toBe('active')
->and($column->getLabel())->toBe('Active')
->and($column->getColor())->toBe('success')
->and($column->getIcon())->toBe('heroicon-o-check');
});Minor (non-blocking)
- Consider importing the Filament contracts at the top of the file instead of inline FQCNs
- A docblock would help discoverability
A helper method to the Column class to set column properties directly from an enum.
Allows a config like this and will set the column label, icon and colour if the standard interfaces are setup.