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.
In #73, the new
useTableSortStaterelies oncolumnIndexto 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
getSortValuesorcompareFn, and this will require acolumnKeysparam that when present changes the signature of setSortColumnIndex to take a key,getSortValuesto return a key/value pair instead of an array, andcompareFnto 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 paramColumnKeythat can either be a union of strings or a number, we can havegetSortValuesrequire a return ofRecord<ColumnKey, number | string | boolean>and it will accept either an object or an array happily depending on whatColumnKeyresolves to. Pretty neat, although maybe overkill.An alternate solution to this crazy hybrid approach would be to always require
columnKeyseven 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 withtableSortPropsthough.I may be overthinking all of this.