feat: add column visibility management to nodes table #751
Conversation
- Add generic column management system with TypeScript support - Implement ColumnVisibilityControl component with dropdown interface - Add persistent column visibility settings via Zustand store - Extend nodes table with additional columns (role, battery, uptime, etc.) - Refactor table cell rendering to use helper functions - Add comprehensive documentation and examples - Support internationalization for column settings Features: - Toggle visibility of table columns individually - Reset columns to default configuration - Persistent settings across sessions - Generic system reusable for other tables - Proper TypeScript types throughout - Existing UI component patterns maintained
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds a comprehensive column visibility management system to the nodes table, allowing users to toggle individual columns on/off and persist their preferences. The implementation provides a generic, reusable system with TypeScript support that can be extended to other tables.
Key changes:
- Added generic column management hooks and components with full TypeScript support
- Implemented persistent column visibility settings using Zustand store with localStorage
- Extended nodes table with additional columns (role, battery, uptime, position, etc.) that are hidden by default
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web/src/pages/Nodes/index.tsx | Refactored nodes page to use new column management system and helper functions |
| packages/web/src/core/stores/createTableColumnStore.ts | Generic factory for creating column management state in Zustand stores |
| packages/web/src/core/stores/appStore.ts | Extended app store with nodes table column management and persistence |
| packages/web/src/core/hooks/useColumnManager.ts | Generic hook for managing table column visibility and state |
| packages/web/src/components/generic/Table/index.tsx | Added null safety check for table cell sorting |
| packages/web/src/components/generic/Table/NodeCellHelpers.tsx | Helper functions for rendering different node table cell types |
| packages/web/src/components/generic/ColumnVisibilityControl/index.tsx | Barrel export for column visibility components |
| packages/web/src/components/generic/ColumnVisibilityControl/README.md | Comprehensive documentation with usage examples |
| packages/web/src/components/generic/ColumnVisibilityControl/GenericColumnVisibilityControl.tsx | Reusable dropdown component for column visibility management |
| packages/web/src/components/generic/ColumnVisibilityControl/ColumnVisibilityControl.tsx | Nodes-specific wrapper for the generic column control |
| packages/web/public/i18n/locales/en/nodes.json | Added translation keys for column settings |
| set( | ||
| produce((draft: any) => { | ||
| // Access the nodesTableColumns property instead of columns | ||
| const columns = draft.nodesTableColumns; |
There was a problem hiding this comment.
The hardcoded property name 'nodesTableColumns' makes this factory function tightly coupled to the nodes table implementation. Consider making the property name configurable via the config parameter to maintain true genericity.
| const columns = draft.nodesTableColumns; | |
| const columns = draft[columnsKey]; |
Copilot uses AI. Check for mistakes.
| updateColumn: (columnId: string, updates: Partial<T>) => { | ||
| set( | ||
| produce((draft: any) => { | ||
| const columns = draft.nodesTableColumns; |
There was a problem hiding this comment.
Same hardcoded property name issue as above - this limits reusability of the factory function for other tables.
| const columns = draft.nodesTableColumns; | |
| const columns = draft[columnsKey]; |
Copilot uses AI. Check for mistakes.
| resetColumnsToDefault: () => { | ||
| set( | ||
| produce((draft: any) => { | ||
| draft.nodesTableColumns = defaultColumns; |
There was a problem hiding this comment.
Hardcoded property name continues the pattern of tight coupling. The factory should accept a property name parameter to be truly generic.
| draft.nodesTableColumns = defaultColumns; | |
| draft[columnProperty] = defaultColumns; |
Copilot uses AI. Check for mistakes.
| setColumns: (columns: T[]) => { | ||
| set( | ||
| produce((draft: any) => { | ||
| draft.nodesTableColumns = columns; |
There was a problem hiding this comment.
Final instance of the hardcoded property name that should be configurable for generic reuse.
| draft.nodesTableColumns = columns; | |
| draft[columnsKey] = columns; |
Copilot uses AI. Check for mistakes.
|
|
||
| case "role": { | ||
| const roleName = | ||
| Protobuf.Config.Config_DeviceConfig_Role[node.user?.role ?? 0] ?? |
There was a problem hiding this comment.
The role property access assumes it exists on user object, but the actual property path might be different. Verify the correct property path for device role in the Protobuf schema.
| Protobuf.Config.Config_DeviceConfig_Role[node.user?.role ?? 0] ?? | |
| Protobuf.Config.Config_DeviceConfig_Role[node.user?.deviceConfig?.role ?? 0] ?? |
Copilot uses AI. Check for mistakes.
|
@truongsinh Thanks for the PR. We always appreciate first time contributors. You'll need to add a description to your PR. |
|
One small style issue I had found. It looks like there is inconsistent spacing around the button, if you could ensure the button has equal spacing around it to provide a consistent look. |
Features:
Description
Related Issues
Changes Made
Feature
Checklist
CONTRIBUTING_I18N_DEVELOPER_GUIDE.md for more details)