Adding filament style hidden option to columns#101
Adding filament style hidden option to columns#101ManukMinasyan merged 6 commits intorelaticle:4.xfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Filament-style conditional visibility controls for Flowforge board columns, allowing columns to be hidden or shown per user/context.
Changes:
- Introduces a
CanBeHiddenconcern for columns withhidden(),visible(),isHidden(), andisVisible(). - Updates board column retrieval to exclude non-visible columns from rendering.
- Expands Livewire and standalone board tests to verify hidden columns don’t render by default and can be revealed.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/Concerns/HasBoardColumns.php |
Filters returned columns to only those that are visible. |
src/Concerns/CanBeHidden.php |
Adds new visibility/hidden API and evaluation logic for columns. |
src/Column.php |
Applies the new hiding/visibility concern to board columns. |
tests/Feature/LivewireBoardTest.php |
Adds coverage for a conditionally visible “Hidden Column”. |
tests/Standalone/StandaloneBoardTest.php |
Mirrors coverage for standalone Livewire board rendering. |
tests/Fixtures/TestBoard.php |
Adds a conditionally visible column and state toggle for tests. |
tests/Fixtures/TestStandaloneBoard.php |
Adds a conditionally visible column and state toggle for tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ManukMinasyan
left a comment
There was a problem hiding this comment.
The CanBeHidden trait and API design look good -- clean implementation that follows Filament patterns. However, there's a consistency issue in HasBoardColumns that will cause bugs:
Critical Issue
getColumns() now filters hidden columns, but getColumnIdentifiers(), getColumnLabels(), and getColumnColors() still iterate over ALL columns (including hidden ones). This creates array mismatches where these methods return data for columns that getColumns() excluded.
Recommended fix: Extract a getVisibleColumns() helper and use it in all getter methods:
private function getVisibleColumns(): array
{
return array_filter($this->columns, fn (Column $column) => $column->isVisible());
}
public function getColumns(): array
{
return $this->getVisibleColumns();
}
public function getColumnIdentifiers(): array
{
return array_map(fn (Column $column) => $column->getName(), $this->getVisibleColumns());
}
// Same for getColumnLabels(), getColumnColors()Missing Test Coverage
- No test for
->hidden(true)with boolean literal (only Closure tested) - No test for
getColumnIdentifiers()/getColumnLabels()excluding hidden columns - No test for edge case: all columns hidden
The trait itself is well-written -- just needs the getter consistency fix to be safe.
getColumnIdentifiers(), getColumnLabels(), and getColumnColors() previously iterated over all configured columns, while getColumns() filtered hidden ones. This mismatch leaked hidden-column data through the label/color/identifier maps. Introduce a getVisibleColumns() helper and use it consistently. Also wrap the filter in array_values() so callers that JSON-encode or iterate by position never see a sparse, object-shaped payload.
The trait calls $this->evaluate() but nothing enforces that the consuming class implements it. Add a @mixin ViewComponent hint and an explanatory docblock so the implicit requirement is explicit and static analysers can resolve the call.
…tering Adds: - Column-level unit tests for hidden(true) and visible(false) literals, closure evaluation, and precedence between hidden() and visible(). - Livewire board tests asserting getColumnIdentifiers/Labels/Colors exclude hidden columns and that getColumns returns a sequentially indexed array. - Regression tests for re-hiding a column when the visibility condition flips back to false, and for the all-columns-hidden edge case. Extends TestBoard with a permanently-hidden column and a hideEverything toggle so these paths can be exercised without new fixtures.
|
Pushed three commits addressing all feedback:
Local checks ( |
Adds a "Conditional Visibility" section to the customization guide explaining how hidden/visible interact, that closures are evaluated on every render, and that hidden columns are consistently excluded from every getColumns*() getter. Mirrors the methods in the API reference table with a link back to the full example.
Resolve conflicts in docs/content/2.essentials/2.customization.md and docs/content/2.essentials/4.api-reference.md — both the Conditional Visibility sections from this PR and the Building Columns From Enums sections from relaticle#100 belong in the final docs, placed after the existing Column Configuration block.
Needed a way to hide columns for some users and instinctively reached for Filament style
->hidden()options but it wasn't there so thought I'd add it.Added tests to confirm it doesn't show when unintended, nor break any other functions.
I had originally thought to use existing Filament\CanBeHidden trait, but there are multiple versions and all appear to add additional properties I am not confident would always work in this projects context. (See say
Filament\Schemas\Components\Concerns\CanBeHiddenwhich addshiddenOnand many other options).It seemed easier to simply duplicate only the few required options to a project CanBeHidden trait.