-
Notifications
You must be signed in to change notification settings - Fork 4
feat: custom props to Table and show sort arrow on hover #1226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: custom props to Table and show sort arrow on hover #1226
Conversation
Integrated TanStack Table's internal pagination model (getPaginationRowModel) for more robust state management. Created a standalone, reusable DataTablePagination component located within the DataTable module. Refactored the Structured View implementation: Merged TableStructuredCell logic into TableCell for a cleaner architecture. Moved the expand/collapse click handler to the root cell element to improve hit area. Added W3C ARIA accessibility attributes (aria-expanded, aria-level, role="row"). Enhanced component robustness by adding safe initializers for pageSize and handling empty data edge cases (e.g., "0-0" range display). Updated Storybook stories with comprehensive examples, including combined support for pagination, sorting, and structured views. Resolved linting issues and standardized flex layouts for better cross-browser consistency.
- Forward className and spread props from DataTable to Table component - Add rowProps and cellProps for custom attributes (e.g., data-testid for Cypress) - Hide sort arrow by default on sortable columns, reveal on hover - Keep sort arrow always visible when column is actively sorted
| {'moonstone-tableCellHead_sortActive': isActive}, | ||
| {'moonstone-tableCellHead_sortHidden': !isActive} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of switching two classes, can we set it hidden by default and display it with moonstone-tableCellHead_sortActive
| return ( | ||
| <div className="moonstone-dataTable"> | ||
| <Table> | ||
| <Table className={className} {...tableProps}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we don't use moonstone-dataTable can we get rid of the wrapper div
| rowsPerPageOptions, | ||
| paginationLabel | ||
| paginationLabel, | ||
| ...tableProps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As other components we can name it ...props instead of ...tableProps
| * Function to add custom HTML attributes to the cell element | ||
| * @param row - The row data | ||
| */ | ||
| cellProps?: (row: T) => React.HTMLAttributes<HTMLTableCellElement> & Record<string, unknown>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be an object instead of a function so it doesn't depend on row; this way, our users can add any custom attributes to the cell.
| * Function to add custom HTML attributes to each row element | ||
| * @param row - The row data | ||
| */ | ||
| rowProps?: (row: T) => React.HTMLAttributes<HTMLTableRowElement> & Record<string, unknown>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be an object instead of a function so it doesn't depend on row; this way, our users can add any custom attributes to the row.
- Remove wrapper div around DataTable (moonstone-dataTable unused) - Simplify sort icon CSS: hidden by default, visible on hover or when active - Rename ...tableProps to ...props for consistency with other components - Change rowProps from function to object for static attribute support - Change cellProps from function to object for static attribute support
No description provided.