rusackas commented on code in PR #44208:
URL: https://github.com/apache/superset/pull/44208#discussion_r4151607717
##########
superset-frontend/packages/superset-ui-core/src/components/TableCollection/utils.tsx:
##########
@@ -49,23 +40,38 @@ const COLUMN_SIZE_MAP: Record<TableSize, number> = {
xxl: 200,
};
-type EnhancedColumnInstance<T extends object = any> = RTColumnInstance<T> &
- Partial<UseSortByColumnOptions<T>> &
- Partial<UseSortByColumnProps<T>> &
- Partial<UseResizeColumnsColumnOptions<T>> &
- Partial<UseResizeColumnsColumnProps<T>> & {
- hidden?: boolean;
- size?: keyof typeof COLUMN_SIZE_MAP;
- className?: string;
- };
+// Mirrors react-table's `Renderer<Props>` (a component, a render function,
+// or a static node) loosely enough that both a hand-authored column config
+// and a `ColumnInstance<T>` produced by `useTable()` satisfy it, so callers
+// holding either shape can pass it without casting.
+type ColumnRenderer =
+ | ReactNode
+ | ComponentType<any>
+ | ((props: any) => ReactNode);
-type EnhancedHeaderGroup<T extends object = any> = RTHeaderGroup<T> & {
- isSorted?: boolean;
- isSortedDesc?: boolean;
-};
+// The `columns` prop is typically the raw column config a caller authors,
+// though the `ColumnInstance<T>` objects react-table builds from that config
+// are structurally accepted too. This is intentionally its own interface
+// rather than react-table's `Column<T>`: that type requires each column's
+// `accessor` to be either a specific `keyof T` literal or an accessor
+// function, but every column config in this codebase writes `accessor` as
+// a plain (TypeScript-widened) string, which satisfies neither — matching
+// react-table's stricter modeling here would mean annotating every column
+// array across ~20 call sites, not a change this shim should make
+// unilaterally.
+export interface ListViewColumn<T extends object = any> {
+ id?: string;
+ Header?: ColumnRenderer;
+ accessor?: keyof T | string | ((row: T) => unknown);
Review Comment:
Good catch, widened `accessor` to react-table's own `Accessor<T>` signature
so index/sub-row accessors type-check too now.
##########
superset-frontend/packages/superset-ui-core/src/components/TableCollection/utils.tsx:
##########
@@ -84,35 +90,39 @@ function getSortingInfo<T extends object>(
}
export function mapColumns<T extends object>(
- columns: EnhancedColumnInstance<T>[],
- headerGroups: EnhancedHeaderGroup<T>[],
+ columns: ListViewColumn<T>[],
+ headerGroups: HeaderGroup<T>[],
columnsForWrapText?: string[],
) {
return columns.map(column => {
- const { isSorted, isSortedDesc } = getSortingInfo(headerGroups, column.id);
+ const id = column.id ?? '';
Review Comment:
Added that case, two id-less columns, click one header, and only that one
picks up the sorted `aria-sort` state.
##########
superset-frontend/packages/superset-ui-core/src/components/TableCollection/utils.tsx:
##########
@@ -121,7 +131,8 @@ export function mapColumns<T extends object>(
column,
});
}
- return val as ReactNode;
+ // A static `Cell` node renders as-is, as react-table itself would.
+ return Cell ?? (val as ReactNode);
Review Comment:
Added that one too, a static `Cell` node next to a plain accessor column,
both render correctly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
