fix: make hidden columns invisible initially #3175
Conversation
✅ No New Circular DependenciesNo new circular dependencies detected. Current count: 0 |
📦 Alpha Package Version PublishedUse Use |
🔍 Visual review for your branch is published 🔍Here are the links to: |
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where columns marked as hidden: true in their definitions were not being hidden initially when no user preferences existed in storage. The fix introduces a distinction between "no user preference" (undefined) and "user explicitly showed all columns" (empty array).
Key Changes:
- Made
orderandhiddenoptional inTableVisualizationSettingsto distinguish between undefined (no user preference) and empty array (explicit user choice) - Updated initialization logic to respect developer-defined hidden columns when user preferences are undefined
- Fixed storage loading to properly restore empty objects and arrays (changed
if (featureValue)toif (featureValue !== undefined))
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
collectionViewRegistry.tsx |
Changed default table settings from { order: [], hidden: [] } to empty object {} to indicate no user preferences |
types.ts |
Made order and hidden optional properties to support undefined state |
handleResetSettings.ts |
Updated reset handler to set empty object instead of explicit empty arrays |
useColums.ts |
Updated logic to check for undefined explicitly; columns not in saved order now maintain their original definition order |
useColumns.settings.test.ts |
Added comprehensive tests to verify behavior with undefined vs empty array vs populated settings |
TableSettings.tsx |
Changed to spread previous settings when updating to preserve other potential fields |
useDataCollectionStorage.ts |
Fixed storage loading to check !== undefined instead of truthiness, enabling proper restoration of empty arrays/objects |
table.stories.tsx |
Updated stories to use unique IDs and adjusted mock data structure |
mockData.tsx |
Adjusted which columns are marked as hidden by default in test scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <ExampleComponent | ||
| tableAllowColumnHiding | ||
| visualizations={[mockVisualizations.table]} | ||
| id="table-column-hidden/v1" | ||
| /> | ||
| ) |
There was a problem hiding this comment.
The frozenColumns prop was removed from this story. In the previous implementation, it was set to frozenColumns=2. This change means the story will now use the default frozenColumns value. If this was an intentional change to simplify the demonstration of column hiding without the complexity of frozen columns, it's fine. However, if the interaction between frozen columns and column hiding was important to demonstrate, the frozenColumns prop should be retained.
| it("should respect developer order when settings.order is undefined", () => { | ||
| const columnsWithOrder: TableColumnDefinition<unknown, never, never>[] = [ | ||
| { id: "column1", label: "Column 1", order: 3 }, | ||
| { id: "column2", label: "Column 2", order: 1 }, | ||
| { id: "column3", label: "Column 3", order: 2 }, | ||
| ] | ||
| const settings: TableVisualizationSettings = {} | ||
| const frozenColumns = 0 | ||
| const allowSorting = true | ||
| const allowHiding = true | ||
|
|
||
| const { result } = renderHook(() => | ||
| useColumns( | ||
| columnsWithOrder, | ||
| frozenColumns, | ||
| settings, | ||
| allowSorting, | ||
| allowHiding | ||
| ) | ||
| ) | ||
|
|
||
| // Should respect developer-defined order | ||
| expect(result.current.colsOrder).toEqual(["column2", "column3", "column1"]) | ||
| }) | ||
|
|
||
| it("should respect user order when settings.order is defined", () => { | ||
| const columnsWithOrder: TableColumnDefinition<unknown, never, never>[] = [ | ||
| { id: "column1", label: "Column 1", order: 3 }, | ||
| { id: "column2", label: "Column 2", order: 1 }, | ||
| { id: "column3", label: "Column 3", order: 2 }, | ||
| ] | ||
| const settings: TableVisualizationSettings = { | ||
| order: ["column3", "column1", "column2"], | ||
| } | ||
| const frozenColumns = 0 | ||
| const allowSorting = true | ||
| const allowHiding = true | ||
|
|
||
| const { result } = renderHook(() => | ||
| useColumns( | ||
| columnsWithOrder, | ||
| frozenColumns, | ||
| settings, | ||
| allowSorting, | ||
| allowHiding | ||
| ) | ||
| ) | ||
|
|
||
| // Should respect user-defined order | ||
| expect(result.current.colsOrder).toEqual(["column3", "column1", "column2"]) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
There's no test case covering when settings.order is an empty array ([]). This is different from settings.order being undefined. With an empty array, all columns will be treated as "not in saved order" and will maintain their original definition order. Consider adding a test to verify that settings.order = [] maintains the developer-defined order from the column definitions, similar to how settings.hidden = [] is tested for the hidden field.
Description
Screenshots (if applicable)
[Link to Figma Design](Figma URL here)
Implementation details