-
Notifications
You must be signed in to change notification settings - Fork 9
Description
In #73, the new useTableSortState relies on columnIndex to identify which column is being sorted on. This is fine for now because it's what the PatternFly table expects, but there is an upcoming feature in PF to support dragging/reordering table columns. When that lands, it will be annoying for the consumer to have to shift around their numeric indexes to keep the sort state accurate. If we instead track the active column by a string id (which will still need to be mapped to a numeric order to support the legacy table) the hook will be more future-proof.
I made a few attempts at this in 44f47c6 and 04eeb24 but I couldn't get the TS types quite right. It's a little awkward because we already support alternate signatures requiring either getSortValues or compareFn, and this will require a columnKeys param that when present changes the signature of setSortColumnIndex to take a key, getSortValues to return a key/value pair instead of an array, and compareFn to take a key instead of an index for the active column.
Incidentally, in this attempt I learned something interesting about TypeScript: an array is assignable to Record<number, T>! So if we have a generic type param ColumnKey that can either be a union of strings or a number, we can have getSortValues require a return of Record<ColumnKey, number | string | boolean> and it will accept either an object or an array happily depending on what ColumnKey resolves to. Pretty neat, although maybe overkill.
An alternate solution to this crazy hybrid approach would be to always require columnKeys even if we are using numeric column indexes everywhere else, and then the consumer just needs to translate indexes to keys whenever updating the sort. We could do this automatically in PF usage with tableSortProps though.
I may be overthinking all of this.