feat: Implement useTable state hook#24329
Conversation
📊 Bundle size reportUnchanged fixtures
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 1b3f64a:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: a0cfab0e5f74e3a3bfa9c269fff574295042d7f2 (build) |
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 1275 | 1267 | 5000 | |
| Button | mount | 978 | 970 | 5000 | |
| FluentProvider | mount | 1593 | 1586 | 5000 | |
| FluentProviderWithTheme | mount | 636 | 633 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 584 | 600 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 624 | 621 | 10 | |
| MakeStyles | mount | 1912 | 1933 | 50000 | |
| SpinButton | mount | 2513 | 2507 | 5000 |
|
55kB of JS... wow. good call |
| setSelected(s => { | ||
| if (s.size === items.length) { | ||
| return new Set<RowId>(); | ||
| } | ||
|
|
||
| return new Set<RowId>(items.map((item, i) => getRowId(item, i))); | ||
| }); |
There was a problem hiding this comment.
| setSelected(s => { | |
| if (s.size === items.length) { | |
| return new Set<RowId>(); | |
| } | |
| return new Set<RowId>(items.map((item, i) => getRowId(item, i))); | |
| }); | |
| setSelected(current => ( | |
| current.size === items.length ? new Set<RowId>() : new Set<RowId>(items.map((item, i) => getRowId(item, i))) | |
| )); |
bsunderhus
left a comment
There was a problem hiding this comment.
Looks amazing to me! I'm totally in favour of delegating complex behaviours to hooks!
| import { useMultipleSelection } from './useMultipleSelection'; | ||
| import { useSingleSelection } from './useSingleSelection'; | ||
|
|
||
| export function useSelection<TItem>( |
There was a problem hiding this comment.
IMHO, wouldn't this hook be a bad idea? I mean, ideally it would be better if the user directly called useMultipleSelection or useSingleSelection.
There was a problem hiding this comment.
yeah, I don't like it now either, it's a part of this issue to decompose these
| <TableHeaderCell {...headerSortProps('file')}>File</TableHeaderCell> | ||
| <TableHeaderCell {...headerSortProps('author')}>Author</TableHeaderCell> | ||
| <TableHeaderCell {...headerSortProps('lastUpdated')}>Last updated</TableHeaderCell> | ||
| <TableHeaderCell {...headerSortProps('lastUpdate')}>Last update</TableHeaderCell> |
There was a problem hiding this comment.
Now that looks nice 😎 👏🏻
| toggleColumnSort, | ||
| headerSortProps: (columnId: ColumnId) => ({ | ||
| sortDirection: sortColumn === columnId ? sortDirection : undefined, | ||
| onClick: () => toggleColumnSort(columnId), |
There was a problem hiding this comment.
This looks wrong to me, as this callback should be somehow memoized
There was a problem hiding this comment.
This is impossible, it's similar to creating factory functions for radio onChange handlers
Implments a
useTablestate hook to support selection and sort.Initially I tried this with @tanstack/react-table, but that library is incredibly generic and relies heavily on render functions which I would like to avoid (for now). That library is also enormous because it supports features that are currently not asked for in Fluent UI
Addresses #24226