Copilot commented on code in PR #39453:
URL: https://github.com/apache/superset/pull/39453#discussion_r3190585606
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.tsx:
##########
@@ -714,727 +825,707 @@ export class TableRenderer extends Component<
};
newSortingOrder[columnIndex] = newDirection;
- const cacheKey =
`${columnIndex}-${visibleColKeys.length}-${rowEnabled}-${rowPartialOnTop}-${newDirection}`;
+ // Include the target column's flat key so different visible-column sets
+ // with the same length don't collide in the cache.
+ const sortTargetKey = flatKey(visColKeys[columnIndex] ?? []);
+ const cacheKey =
`${columnIndex}-${visColKeys.length}-${sortTargetKey}-${rowEnabled}-${rowPartialOnTop}-${newDirection}`;
let newRowKeys;
- if (this.sortCache.has(cacheKey)) {
- const cachedRowKeys = this.sortCache.get(cacheKey);
+ if (sortCacheRef.current.has(cacheKey)) {
+ const cachedRowKeys = sortCacheRef.current.get(cacheKey);
newRowKeys = cachedRowKeys;
} else {
- const groups = this.getAggregatedData(
+ const groups = getAggregatedData(
pivotData,
- visibleColKeys[columnIndex],
+ visColKeys[columnIndex],
rowPartialOnTop,
);
- const sortedRowKeys = this.sortAndCacheData(
+ const computedSortedRowKeys = sortAndCacheData(
groups,
newDirection,
rowEnabled,
rowPartialOnTop,
maxRowIndex,
);
- this.sortCache.set(cacheKey, sortedRowKeys);
- newRowKeys = sortedRowKeys;
+ sortCacheRef.current.set(cacheKey, computedSortedRowKeys);
+ newRowKeys = computedSortedRowKeys;
}
- this.cachedBasePivotSettings = {
- ...this.cachedBasePivotSettings!,
- rowKeys: newRowKeys!,
- };
- return {
- sortingOrder: newSortingOrder,
- activeSortColumn: columnIndex,
- };
- });
- }
+ setSortedRowKeys(newRowKeys!);
+ setSortingOrder(newSortingOrder);
+ setActiveSortColumn(columnIndex);
+ },
+ [activeSortColumn, sortingOrder, getAggregatedData, sortAndCacheData],
+ );
- renderColHeaderRow(
- attrName: string,
- attrIdx: number,
- pivotSettings: PivotSettings,
- ) {
- // Render a single row in the column header at the top of the pivot table.
+ const renderColHeaderRow = useCallback(
+ (attrName: string, attrIdx: number, settings: PivotSettings) => {
+ // Render a single row in the column header at the top of the pivot
table.
+
+ const {
+ rowAttrs: settingsRowAttrs,
+ colAttrs: settingsColAttrs,
+ colKeys: settingsColKeys,
+ visibleColKeys: settingsVisibleColKeys,
+ colAttrSpans,
+ rowTotals,
+ arrowExpanded,
+ arrowCollapsed,
+ colSubtotalDisplay: settingsColSubtotalDisplay,
+ maxColVisible,
+ pivotData,
+ namesMapping,
+ allowRenderHtml: settingsAllowRenderHtml,
+ } = settings;
+ const {
+ highlightHeaderCellsOnHover,
+ omittedHighlightHeaderGroups = [],
+ highlightedHeaderCells,
+ cellColorFormatters,
+ dateFormatters,
+ cellBackgroundColor = supersetTheme.colorBgBase,
+ activeHeaderBackgroundColor = supersetTheme.colorPrimaryBg,
+ } = tableOptions;
- const {
- rowAttrs,
- colAttrs,
- colKeys,
- visibleColKeys,
- colAttrSpans,
- rowTotals,
- arrowExpanded,
- arrowCollapsed,
- colSubtotalDisplay,
- maxColVisible,
- pivotData,
- namesMapping,
- allowRenderHtml,
- } = pivotSettings;
- const {
- highlightHeaderCellsOnHover,
- omittedHighlightHeaderGroups = [],
- highlightedHeaderCells,
- cellColorFormatters,
- dateFormatters,
- cellBackgroundColor = supersetTheme.colorBgBase,
- activeHeaderBackgroundColor = supersetTheme.colorPrimaryBg,
- } = this.props.tableOptions;
-
- if (!visibleColKeys || !colAttrSpans) {
- return null;
- }
+ if (!settingsVisibleColKeys || !colAttrSpans) {
+ return null;
+ }
- const spaceCell =
- attrIdx === 0 && rowAttrs.length !== 0 ? (
- <th
- key="padding"
- colSpan={rowAttrs.length}
- rowSpan={colAttrs.length}
- aria-hidden="true"
- />
- ) : null;
-
- const needToggle =
- colSubtotalDisplay.enabled === true && attrIdx !== colAttrs.length - 1;
- let arrowClickHandle = null;
- let subArrow = null;
- if (needToggle) {
- arrowClickHandle =
- attrIdx + 1 < maxColVisible!
- ? this.collapseAttr(false, attrIdx, colKeys)
- : this.expandAttr(false, attrIdx, colKeys);
- subArrow = attrIdx + 1 < maxColVisible! ? arrowExpanded : arrowCollapsed;
- }
- const attrNameCell = (
- <th key="label" className="pvtAxisLabel">
- {displayHeaderCell(
- needToggle,
- subArrow,
- arrowClickHandle,
- attrName,
- namesMapping,
- allowRenderHtml,
- )}
- </th>
- );
-
- const attrValueCells = [];
- const rowIncrSpan = rowAttrs.length !== 0 ? 1 : 0;
- // Iterate through columns. Jump over duplicate values.
- let i = 0;
- while (i < visibleColKeys.length) {
- let handleContextMenu: ((e: MouseEvent) => void) | undefined;
- const colKey = visibleColKeys[i];
- const colSpan = attrIdx < colKey.length ? colAttrSpans[i][attrIdx] : 1;
- let colLabelClass = 'pvtColLabel';
- if (attrIdx < colKey.length) {
- if (!omittedHighlightHeaderGroups.includes(colAttrs[attrIdx])) {
- if (highlightHeaderCellsOnHover) {
- colLabelClass += ' hoverable';
+ const spaceCell =
+ attrIdx === 0 && settingsRowAttrs.length !== 0 ? (
+ <th
+ key="padding"
+ colSpan={settingsRowAttrs.length}
+ rowSpan={settingsColAttrs.length}
+ aria-hidden="true"
+ />
+ ) : null;
+
+ const needToggle =
+ settingsColSubtotalDisplay.enabled === true &&
+ attrIdx !== settingsColAttrs.length - 1;
+ let arrowClickHandle = null;
+ let subArrow = null;
+ if (needToggle) {
+ arrowClickHandle =
+ attrIdx + 1 < maxColVisible!
+ ? collapseAttr(false, attrIdx, settingsColKeys)
+ : expandAttr(false, attrIdx, settingsColKeys);
+ subArrow =
+ attrIdx + 1 < maxColVisible! ? arrowExpanded : arrowCollapsed;
+ }
+ const attrNameCell = (
+ <th key="label" className="pvtAxisLabel">
+ {displayHeaderCell(
+ needToggle,
+ subArrow,
+ arrowClickHandle,
+ attrName,
+ namesMapping,
+ settingsAllowRenderHtml,
+ )}
+ </th>
+ );
+
+ const attrValueCells = [];
+ const rowIncrSpan = settingsRowAttrs.length !== 0 ? 1 : 0;
+ // Iterate through columns. Jump over duplicate values.
+ let i = 0;
+ while (i < settingsVisibleColKeys.length) {
+ let handleContextMenu: ((e: MouseEvent) => void) | undefined;
+ const colKey = settingsVisibleColKeys[i];
+ const colSpan = attrIdx < colKey.length ? colAttrSpans[i][attrIdx] : 1;
+ let colLabelClass = 'pvtColLabel';
+ if (attrIdx < colKey.length) {
+ if (
+ !omittedHighlightHeaderGroups.includes(settingsColAttrs[attrIdx])
+ ) {
+ if (highlightHeaderCellsOnHover) {
+ colLabelClass += ' hoverable';
+ }
+ handleContextMenu = (e: MouseEvent) =>
+ onContextMenu(e, colKey, undefined, {
+ [attrName]: colKey[attrIdx],
+ });
}
- handleContextMenu = (e: MouseEvent) =>
- this.props.onContextMenu(e, colKey, undefined, {
- [attrName]: colKey[attrIdx],
- });
- }
- if (
- highlightedHeaderCells &&
- Array.isArray(highlightedHeaderCells[colAttrs[attrIdx]]) &&
- highlightedHeaderCells[colAttrs[attrIdx]].includes(colKey[attrIdx])
- ) {
- colLabelClass += ' active';
- }
- const isActiveHeader = colLabelClass.includes('active');
- const maxRowIndex = pivotSettings.maxRowVisible!;
- const mColVisible = pivotSettings.maxColVisible!;
- const visibleSortIcon = mColVisible - 1 === attrIdx;
- const columnName = colKey[mColVisible - 1];
-
- const rowSpan = 1 + (attrIdx === colAttrs.length - 1 ? rowIncrSpan :
0);
- const flatColKey = flatKey(colKey.slice(0, attrIdx + 1));
- const onArrowClick = needToggle ? this.toggleColKey(flatColKey) : null;
- const getSortIcon = (key: number) => {
- const { activeSortColumn, sortingOrder } = this.state;
-
- if (activeSortColumn !== key) {
+ if (
+ highlightedHeaderCells &&
+ Array.isArray(highlightedHeaderCells[settingsColAttrs[attrIdx]]) &&
+ highlightedHeaderCells[settingsColAttrs[attrIdx]].includes(
+ colKey[attrIdx],
+ )
+ ) {
+ colLabelClass += ' active';
+ }
+ const maxRowIndex = settings.maxRowVisible!;
+ const mColVisible = settings.maxColVisible!;
+ const visibleSortIcon = mColVisible - 1 === attrIdx;
+ const columnName = colKey[mColVisible - 1];
+
+ const rowSpan =
+ 1 + (attrIdx === settingsColAttrs.length - 1 ? rowIncrSpan : 0);
+ const flatColKey = flatKey(colKey.slice(0, attrIdx + 1));
+ const onArrowClick = needToggle ? toggleColKey(flatColKey) : null;
+ const getSortIcon = (key: number) => {
+ if (activeSortColumn !== key) {
+ return (
+ <ColumnHeightOutlined
+ onClick={() =>
+ sortData(
+ key,
+ settingsVisibleColKeys,
+ pivotData,
+ maxRowIndex,
+ )
+ }
+ />
+ );
+ }
+
+ const SortIcon =
+ sortingOrder[key] === 'asc' ? CaretUpOutlined :
CaretDownOutlined;
return (
- <ColumnHeightOutlined
+ <SortIcon
onClick={() =>
- this.sortData(key, visibleColKeys, pivotData, maxRowIndex)
+ sortData(key, settingsVisibleColKeys, pivotData, maxRowIndex)
}
/>
);
- }
-
- const SortIcon =
- sortingOrder[key] === 'asc' ? CaretUpOutlined : CaretDownOutlined;
- return (
- <SortIcon
- onClick={() =>
- this.sortData(key, visibleColKeys, pivotData, maxRowIndex)
- }
- />
+ };
+ // Coerce numeric timestamp strings to numbers so temporal formatters
+ // (which typically expect an epoch) render correctly.
+ const rawHeaderCellValue = colKey[attrIdx];
+ const headerCellFormatterValue =
+ typeof rawHeaderCellValue === 'string' &&
+ rawHeaderCellValue.trim() !== '' &&
+ Number.isFinite(Number(rawHeaderCellValue))
+ ? Number(rawHeaderCellValue)
+ : rawHeaderCellValue;
+ const headerCellFormattedValue =
+ dateFormatters?.[attrName]?.(headerCellFormatterValue) ??
+ rawHeaderCellValue;
Review Comment:
Mandatory: this refactor keeps separate coercion paths for column and row
header date formatters, but the rewritten tests removed the previous cases that
verified numeric timestamp strings versus plain strings. Because this logic is
easy to break during refactors, both paths should keep explicit coverage.
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.tsx:
##########
@@ -714,727 +825,707 @@ export class TableRenderer extends Component<
};
newSortingOrder[columnIndex] = newDirection;
- const cacheKey =
`${columnIndex}-${visibleColKeys.length}-${rowEnabled}-${rowPartialOnTop}-${newDirection}`;
+ // Include the target column's flat key so different visible-column sets
+ // with the same length don't collide in the cache.
+ const sortTargetKey = flatKey(visColKeys[columnIndex] ?? []);
+ const cacheKey =
`${columnIndex}-${visColKeys.length}-${sortTargetKey}-${rowEnabled}-${rowPartialOnTop}-${newDirection}`;
let newRowKeys;
- if (this.sortCache.has(cacheKey)) {
- const cachedRowKeys = this.sortCache.get(cacheKey);
+ if (sortCacheRef.current.has(cacheKey)) {
+ const cachedRowKeys = sortCacheRef.current.get(cacheKey);
newRowKeys = cachedRowKeys;
} else {
- const groups = this.getAggregatedData(
+ const groups = getAggregatedData(
pivotData,
- visibleColKeys[columnIndex],
+ visColKeys[columnIndex],
rowPartialOnTop,
);
- const sortedRowKeys = this.sortAndCacheData(
+ const computedSortedRowKeys = sortAndCacheData(
groups,
newDirection,
rowEnabled,
rowPartialOnTop,
maxRowIndex,
);
- this.sortCache.set(cacheKey, sortedRowKeys);
- newRowKeys = sortedRowKeys;
+ sortCacheRef.current.set(cacheKey, computedSortedRowKeys);
+ newRowKeys = computedSortedRowKeys;
}
- this.cachedBasePivotSettings = {
- ...this.cachedBasePivotSettings!,
- rowKeys: newRowKeys!,
- };
- return {
- sortingOrder: newSortingOrder,
- activeSortColumn: columnIndex,
- };
- });
- }
+ setSortedRowKeys(newRowKeys!);
+ setSortingOrder(newSortingOrder);
+ setActiveSortColumn(columnIndex);
+ },
+ [activeSortColumn, sortingOrder, getAggregatedData, sortAndCacheData],
+ );
- renderColHeaderRow(
- attrName: string,
- attrIdx: number,
- pivotSettings: PivotSettings,
- ) {
- // Render a single row in the column header at the top of the pivot table.
+ const renderColHeaderRow = useCallback(
+ (attrName: string, attrIdx: number, settings: PivotSettings) => {
+ // Render a single row in the column header at the top of the pivot
table.
+
+ const {
+ rowAttrs: settingsRowAttrs,
+ colAttrs: settingsColAttrs,
+ colKeys: settingsColKeys,
+ visibleColKeys: settingsVisibleColKeys,
+ colAttrSpans,
+ rowTotals,
+ arrowExpanded,
+ arrowCollapsed,
+ colSubtotalDisplay: settingsColSubtotalDisplay,
+ maxColVisible,
+ pivotData,
+ namesMapping,
+ allowRenderHtml: settingsAllowRenderHtml,
+ } = settings;
+ const {
+ highlightHeaderCellsOnHover,
+ omittedHighlightHeaderGroups = [],
+ highlightedHeaderCells,
+ cellColorFormatters,
+ dateFormatters,
+ cellBackgroundColor = supersetTheme.colorBgBase,
+ activeHeaderBackgroundColor = supersetTheme.colorPrimaryBg,
+ } = tableOptions;
- const {
- rowAttrs,
- colAttrs,
- colKeys,
- visibleColKeys,
- colAttrSpans,
- rowTotals,
- arrowExpanded,
- arrowCollapsed,
- colSubtotalDisplay,
- maxColVisible,
- pivotData,
- namesMapping,
- allowRenderHtml,
- } = pivotSettings;
- const {
- highlightHeaderCellsOnHover,
- omittedHighlightHeaderGroups = [],
- highlightedHeaderCells,
- cellColorFormatters,
- dateFormatters,
- cellBackgroundColor = supersetTheme.colorBgBase,
- activeHeaderBackgroundColor = supersetTheme.colorPrimaryBg,
- } = this.props.tableOptions;
-
- if (!visibleColKeys || !colAttrSpans) {
- return null;
- }
+ if (!settingsVisibleColKeys || !colAttrSpans) {
+ return null;
+ }
- const spaceCell =
- attrIdx === 0 && rowAttrs.length !== 0 ? (
- <th
- key="padding"
- colSpan={rowAttrs.length}
- rowSpan={colAttrs.length}
- aria-hidden="true"
- />
- ) : null;
-
- const needToggle =
- colSubtotalDisplay.enabled === true && attrIdx !== colAttrs.length - 1;
- let arrowClickHandle = null;
- let subArrow = null;
- if (needToggle) {
- arrowClickHandle =
- attrIdx + 1 < maxColVisible!
- ? this.collapseAttr(false, attrIdx, colKeys)
- : this.expandAttr(false, attrIdx, colKeys);
- subArrow = attrIdx + 1 < maxColVisible! ? arrowExpanded : arrowCollapsed;
- }
- const attrNameCell = (
- <th key="label" className="pvtAxisLabel">
- {displayHeaderCell(
- needToggle,
- subArrow,
- arrowClickHandle,
- attrName,
- namesMapping,
- allowRenderHtml,
- )}
- </th>
- );
-
- const attrValueCells = [];
- const rowIncrSpan = rowAttrs.length !== 0 ? 1 : 0;
- // Iterate through columns. Jump over duplicate values.
- let i = 0;
- while (i < visibleColKeys.length) {
- let handleContextMenu: ((e: MouseEvent) => void) | undefined;
- const colKey = visibleColKeys[i];
- const colSpan = attrIdx < colKey.length ? colAttrSpans[i][attrIdx] : 1;
- let colLabelClass = 'pvtColLabel';
- if (attrIdx < colKey.length) {
- if (!omittedHighlightHeaderGroups.includes(colAttrs[attrIdx])) {
- if (highlightHeaderCellsOnHover) {
- colLabelClass += ' hoverable';
+ const spaceCell =
+ attrIdx === 0 && settingsRowAttrs.length !== 0 ? (
+ <th
+ key="padding"
+ colSpan={settingsRowAttrs.length}
+ rowSpan={settingsColAttrs.length}
+ aria-hidden="true"
+ />
+ ) : null;
+
+ const needToggle =
+ settingsColSubtotalDisplay.enabled === true &&
+ attrIdx !== settingsColAttrs.length - 1;
+ let arrowClickHandle = null;
+ let subArrow = null;
+ if (needToggle) {
+ arrowClickHandle =
+ attrIdx + 1 < maxColVisible!
+ ? collapseAttr(false, attrIdx, settingsColKeys)
+ : expandAttr(false, attrIdx, settingsColKeys);
+ subArrow =
+ attrIdx + 1 < maxColVisible! ? arrowExpanded : arrowCollapsed;
+ }
+ const attrNameCell = (
+ <th key="label" className="pvtAxisLabel">
+ {displayHeaderCell(
+ needToggle,
+ subArrow,
+ arrowClickHandle,
+ attrName,
+ namesMapping,
+ settingsAllowRenderHtml,
+ )}
+ </th>
+ );
+
+ const attrValueCells = [];
+ const rowIncrSpan = settingsRowAttrs.length !== 0 ? 1 : 0;
+ // Iterate through columns. Jump over duplicate values.
+ let i = 0;
+ while (i < settingsVisibleColKeys.length) {
+ let handleContextMenu: ((e: MouseEvent) => void) | undefined;
+ const colKey = settingsVisibleColKeys[i];
+ const colSpan = attrIdx < colKey.length ? colAttrSpans[i][attrIdx] : 1;
+ let colLabelClass = 'pvtColLabel';
+ if (attrIdx < colKey.length) {
+ if (
+ !omittedHighlightHeaderGroups.includes(settingsColAttrs[attrIdx])
+ ) {
+ if (highlightHeaderCellsOnHover) {
+ colLabelClass += ' hoverable';
+ }
+ handleContextMenu = (e: MouseEvent) =>
+ onContextMenu(e, colKey, undefined, {
+ [attrName]: colKey[attrIdx],
+ });
}
- handleContextMenu = (e: MouseEvent) =>
- this.props.onContextMenu(e, colKey, undefined, {
- [attrName]: colKey[attrIdx],
- });
- }
- if (
- highlightedHeaderCells &&
- Array.isArray(highlightedHeaderCells[colAttrs[attrIdx]]) &&
- highlightedHeaderCells[colAttrs[attrIdx]].includes(colKey[attrIdx])
- ) {
- colLabelClass += ' active';
- }
- const isActiveHeader = colLabelClass.includes('active');
- const maxRowIndex = pivotSettings.maxRowVisible!;
- const mColVisible = pivotSettings.maxColVisible!;
- const visibleSortIcon = mColVisible - 1 === attrIdx;
- const columnName = colKey[mColVisible - 1];
-
- const rowSpan = 1 + (attrIdx === colAttrs.length - 1 ? rowIncrSpan :
0);
- const flatColKey = flatKey(colKey.slice(0, attrIdx + 1));
- const onArrowClick = needToggle ? this.toggleColKey(flatColKey) : null;
- const getSortIcon = (key: number) => {
- const { activeSortColumn, sortingOrder } = this.state;
-
- if (activeSortColumn !== key) {
+ if (
+ highlightedHeaderCells &&
+ Array.isArray(highlightedHeaderCells[settingsColAttrs[attrIdx]]) &&
+ highlightedHeaderCells[settingsColAttrs[attrIdx]].includes(
+ colKey[attrIdx],
+ )
+ ) {
+ colLabelClass += ' active';
+ }
+ const maxRowIndex = settings.maxRowVisible!;
+ const mColVisible = settings.maxColVisible!;
+ const visibleSortIcon = mColVisible - 1 === attrIdx;
+ const columnName = colKey[mColVisible - 1];
+
+ const rowSpan =
+ 1 + (attrIdx === settingsColAttrs.length - 1 ? rowIncrSpan : 0);
+ const flatColKey = flatKey(colKey.slice(0, attrIdx + 1));
+ const onArrowClick = needToggle ? toggleColKey(flatColKey) : null;
+ const getSortIcon = (key: number) => {
+ if (activeSortColumn !== key) {
+ return (
+ <ColumnHeightOutlined
+ onClick={() =>
+ sortData(
+ key,
+ settingsVisibleColKeys,
+ pivotData,
+ maxRowIndex,
+ )
+ }
+ />
+ );
+ }
+
+ const SortIcon =
+ sortingOrder[key] === 'asc' ? CaretUpOutlined :
CaretDownOutlined;
return (
- <ColumnHeightOutlined
+ <SortIcon
onClick={() =>
- this.sortData(key, visibleColKeys, pivotData, maxRowIndex)
+ sortData(key, settingsVisibleColKeys, pivotData, maxRowIndex)
}
/>
);
- }
-
- const SortIcon =
- sortingOrder[key] === 'asc' ? CaretUpOutlined : CaretDownOutlined;
- return (
- <SortIcon
- onClick={() =>
- this.sortData(key, visibleColKeys, pivotData, maxRowIndex)
- }
- />
+ };
+ // Coerce numeric timestamp strings to numbers so temporal formatters
+ // (which typically expect an epoch) render correctly.
+ const rawHeaderCellValue = colKey[attrIdx];
+ const headerCellFormatterValue =
+ typeof rawHeaderCellValue === 'string' &&
+ rawHeaderCellValue.trim() !== '' &&
+ Number.isFinite(Number(rawHeaderCellValue))
+ ? Number(rawHeaderCellValue)
+ : rawHeaderCellValue;
+ const headerCellFormattedValue =
+ dateFormatters?.[attrName]?.(headerCellFormatterValue) ??
+ rawHeaderCellValue;
+ const isActiveHeader = colLabelClass.includes('active');
+ const { backgroundColor, color } = getCellColor(
+ [attrName],
+ headerCellFormattedValue,
+ cellColorFormatters,
+ isActiveHeader ? activeHeaderBackgroundColor : cellBackgroundColor,
);
- };
- const headerCellFormattedValue =
- dateFormatters?.[attrName]?.(
- convertToNumberIfNumeric(colKey[attrIdx]),
- ) ?? colKey[attrIdx];
- const { backgroundColor, color } = getCellColor(
- [attrName],
- headerCellFormattedValue,
- cellColorFormatters,
- isActiveHeader ? activeHeaderBackgroundColor : cellBackgroundColor,
- );
- const style = {
- backgroundColor,
- ...(color ? { color } : {}),
- };
- attrValueCells.push(
+ const colHeaderStyle = {
+ backgroundColor,
+ ...(color ? { color } : {}),
Review Comment:
Optional: the style/contrast logic for formatted headers and subtotal cells
is still present after the hook conversion, but the rewritten tests dropped the
earlier assertions that protected these paths. Since regressions here make
headers and subtotal cells unreadable in production, it would be worth keeping
at least one focused style assertion for a formatted header and one
subtotal/value cell.
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.tsx:
##########
@@ -714,727 +825,707 @@ export class TableRenderer extends Component<
};
newSortingOrder[columnIndex] = newDirection;
- const cacheKey =
`${columnIndex}-${visibleColKeys.length}-${rowEnabled}-${rowPartialOnTop}-${newDirection}`;
+ // Include the target column's flat key so different visible-column sets
+ // with the same length don't collide in the cache.
+ const sortTargetKey = flatKey(visColKeys[columnIndex] ?? []);
+ const cacheKey =
`${columnIndex}-${visColKeys.length}-${sortTargetKey}-${rowEnabled}-${rowPartialOnTop}-${newDirection}`;
let newRowKeys;
- if (this.sortCache.has(cacheKey)) {
- const cachedRowKeys = this.sortCache.get(cacheKey);
+ if (sortCacheRef.current.has(cacheKey)) {
+ const cachedRowKeys = sortCacheRef.current.get(cacheKey);
newRowKeys = cachedRowKeys;
} else {
- const groups = this.getAggregatedData(
+ const groups = getAggregatedData(
pivotData,
- visibleColKeys[columnIndex],
+ visColKeys[columnIndex],
rowPartialOnTop,
);
- const sortedRowKeys = this.sortAndCacheData(
+ const computedSortedRowKeys = sortAndCacheData(
groups,
newDirection,
rowEnabled,
rowPartialOnTop,
maxRowIndex,
);
- this.sortCache.set(cacheKey, sortedRowKeys);
- newRowKeys = sortedRowKeys;
+ sortCacheRef.current.set(cacheKey, computedSortedRowKeys);
+ newRowKeys = computedSortedRowKeys;
}
- this.cachedBasePivotSettings = {
- ...this.cachedBasePivotSettings!,
- rowKeys: newRowKeys!,
- };
- return {
- sortingOrder: newSortingOrder,
- activeSortColumn: columnIndex,
- };
- });
- }
+ setSortedRowKeys(newRowKeys!);
+ setSortingOrder(newSortingOrder);
+ setActiveSortColumn(columnIndex);
+ },
+ [activeSortColumn, sortingOrder, getAggregatedData, sortAndCacheData],
+ );
- renderColHeaderRow(
- attrName: string,
- attrIdx: number,
- pivotSettings: PivotSettings,
- ) {
- // Render a single row in the column header at the top of the pivot table.
+ const renderColHeaderRow = useCallback(
+ (attrName: string, attrIdx: number, settings: PivotSettings) => {
+ // Render a single row in the column header at the top of the pivot
table.
+
+ const {
+ rowAttrs: settingsRowAttrs,
+ colAttrs: settingsColAttrs,
+ colKeys: settingsColKeys,
+ visibleColKeys: settingsVisibleColKeys,
+ colAttrSpans,
+ rowTotals,
+ arrowExpanded,
+ arrowCollapsed,
+ colSubtotalDisplay: settingsColSubtotalDisplay,
+ maxColVisible,
+ pivotData,
+ namesMapping,
+ allowRenderHtml: settingsAllowRenderHtml,
+ } = settings;
+ const {
+ highlightHeaderCellsOnHover,
+ omittedHighlightHeaderGroups = [],
+ highlightedHeaderCells,
+ cellColorFormatters,
+ dateFormatters,
+ cellBackgroundColor = supersetTheme.colorBgBase,
+ activeHeaderBackgroundColor = supersetTheme.colorPrimaryBg,
+ } = tableOptions;
- const {
- rowAttrs,
- colAttrs,
- colKeys,
- visibleColKeys,
- colAttrSpans,
- rowTotals,
- arrowExpanded,
- arrowCollapsed,
- colSubtotalDisplay,
- maxColVisible,
- pivotData,
- namesMapping,
- allowRenderHtml,
- } = pivotSettings;
- const {
- highlightHeaderCellsOnHover,
- omittedHighlightHeaderGroups = [],
- highlightedHeaderCells,
- cellColorFormatters,
- dateFormatters,
- cellBackgroundColor = supersetTheme.colorBgBase,
- activeHeaderBackgroundColor = supersetTheme.colorPrimaryBg,
- } = this.props.tableOptions;
-
- if (!visibleColKeys || !colAttrSpans) {
- return null;
- }
+ if (!settingsVisibleColKeys || !colAttrSpans) {
+ return null;
+ }
- const spaceCell =
- attrIdx === 0 && rowAttrs.length !== 0 ? (
- <th
- key="padding"
- colSpan={rowAttrs.length}
- rowSpan={colAttrs.length}
- aria-hidden="true"
- />
- ) : null;
-
- const needToggle =
- colSubtotalDisplay.enabled === true && attrIdx !== colAttrs.length - 1;
- let arrowClickHandle = null;
- let subArrow = null;
- if (needToggle) {
- arrowClickHandle =
- attrIdx + 1 < maxColVisible!
- ? this.collapseAttr(false, attrIdx, colKeys)
- : this.expandAttr(false, attrIdx, colKeys);
- subArrow = attrIdx + 1 < maxColVisible! ? arrowExpanded : arrowCollapsed;
- }
- const attrNameCell = (
- <th key="label" className="pvtAxisLabel">
- {displayHeaderCell(
- needToggle,
- subArrow,
- arrowClickHandle,
- attrName,
- namesMapping,
- allowRenderHtml,
- )}
- </th>
- );
-
- const attrValueCells = [];
- const rowIncrSpan = rowAttrs.length !== 0 ? 1 : 0;
- // Iterate through columns. Jump over duplicate values.
- let i = 0;
- while (i < visibleColKeys.length) {
- let handleContextMenu: ((e: MouseEvent) => void) | undefined;
- const colKey = visibleColKeys[i];
- const colSpan = attrIdx < colKey.length ? colAttrSpans[i][attrIdx] : 1;
- let colLabelClass = 'pvtColLabel';
- if (attrIdx < colKey.length) {
- if (!omittedHighlightHeaderGroups.includes(colAttrs[attrIdx])) {
- if (highlightHeaderCellsOnHover) {
- colLabelClass += ' hoverable';
+ const spaceCell =
+ attrIdx === 0 && settingsRowAttrs.length !== 0 ? (
+ <th
+ key="padding"
+ colSpan={settingsRowAttrs.length}
+ rowSpan={settingsColAttrs.length}
+ aria-hidden="true"
+ />
+ ) : null;
+
+ const needToggle =
+ settingsColSubtotalDisplay.enabled === true &&
+ attrIdx !== settingsColAttrs.length - 1;
+ let arrowClickHandle = null;
+ let subArrow = null;
+ if (needToggle) {
+ arrowClickHandle =
+ attrIdx + 1 < maxColVisible!
+ ? collapseAttr(false, attrIdx, settingsColKeys)
+ : expandAttr(false, attrIdx, settingsColKeys);
+ subArrow =
+ attrIdx + 1 < maxColVisible! ? arrowExpanded : arrowCollapsed;
+ }
+ const attrNameCell = (
+ <th key="label" className="pvtAxisLabel">
+ {displayHeaderCell(
+ needToggle,
+ subArrow,
+ arrowClickHandle,
+ attrName,
+ namesMapping,
+ settingsAllowRenderHtml,
+ )}
+ </th>
+ );
+
+ const attrValueCells = [];
+ const rowIncrSpan = settingsRowAttrs.length !== 0 ? 1 : 0;
+ // Iterate through columns. Jump over duplicate values.
+ let i = 0;
+ while (i < settingsVisibleColKeys.length) {
+ let handleContextMenu: ((e: MouseEvent) => void) | undefined;
+ const colKey = settingsVisibleColKeys[i];
+ const colSpan = attrIdx < colKey.length ? colAttrSpans[i][attrIdx] : 1;
+ let colLabelClass = 'pvtColLabel';
+ if (attrIdx < colKey.length) {
+ if (
+ !omittedHighlightHeaderGroups.includes(settingsColAttrs[attrIdx])
+ ) {
+ if (highlightHeaderCellsOnHover) {
+ colLabelClass += ' hoverable';
+ }
+ handleContextMenu = (e: MouseEvent) =>
+ onContextMenu(e, colKey, undefined, {
+ [attrName]: colKey[attrIdx],
+ });
}
- handleContextMenu = (e: MouseEvent) =>
- this.props.onContextMenu(e, colKey, undefined, {
- [attrName]: colKey[attrIdx],
- });
- }
- if (
- highlightedHeaderCells &&
- Array.isArray(highlightedHeaderCells[colAttrs[attrIdx]]) &&
- highlightedHeaderCells[colAttrs[attrIdx]].includes(colKey[attrIdx])
- ) {
- colLabelClass += ' active';
- }
- const isActiveHeader = colLabelClass.includes('active');
- const maxRowIndex = pivotSettings.maxRowVisible!;
- const mColVisible = pivotSettings.maxColVisible!;
- const visibleSortIcon = mColVisible - 1 === attrIdx;
- const columnName = colKey[mColVisible - 1];
-
- const rowSpan = 1 + (attrIdx === colAttrs.length - 1 ? rowIncrSpan :
0);
- const flatColKey = flatKey(colKey.slice(0, attrIdx + 1));
- const onArrowClick = needToggle ? this.toggleColKey(flatColKey) : null;
- const getSortIcon = (key: number) => {
- const { activeSortColumn, sortingOrder } = this.state;
-
- if (activeSortColumn !== key) {
+ if (
+ highlightedHeaderCells &&
+ Array.isArray(highlightedHeaderCells[settingsColAttrs[attrIdx]]) &&
+ highlightedHeaderCells[settingsColAttrs[attrIdx]].includes(
+ colKey[attrIdx],
+ )
+ ) {
+ colLabelClass += ' active';
+ }
+ const maxRowIndex = settings.maxRowVisible!;
+ const mColVisible = settings.maxColVisible!;
+ const visibleSortIcon = mColVisible - 1 === attrIdx;
+ const columnName = colKey[mColVisible - 1];
+
+ const rowSpan =
+ 1 + (attrIdx === settingsColAttrs.length - 1 ? rowIncrSpan : 0);
+ const flatColKey = flatKey(colKey.slice(0, attrIdx + 1));
+ const onArrowClick = needToggle ? toggleColKey(flatColKey) : null;
+ const getSortIcon = (key: number) => {
+ if (activeSortColumn !== key) {
+ return (
+ <ColumnHeightOutlined
+ onClick={() =>
+ sortData(
+ key,
+ settingsVisibleColKeys,
+ pivotData,
+ maxRowIndex,
+ )
+ }
+ />
+ );
+ }
+
+ const SortIcon =
+ sortingOrder[key] === 'asc' ? CaretUpOutlined :
CaretDownOutlined;
return (
- <ColumnHeightOutlined
+ <SortIcon
onClick={() =>
- this.sortData(key, visibleColKeys, pivotData, maxRowIndex)
+ sortData(key, settingsVisibleColKeys, pivotData, maxRowIndex)
}
/>
);
- }
-
- const SortIcon =
- sortingOrder[key] === 'asc' ? CaretUpOutlined : CaretDownOutlined;
- return (
- <SortIcon
- onClick={() =>
- this.sortData(key, visibleColKeys, pivotData, maxRowIndex)
- }
- />
+ };
+ // Coerce numeric timestamp strings to numbers so temporal formatters
+ // (which typically expect an epoch) render correctly.
+ const rawHeaderCellValue = colKey[attrIdx];
+ const headerCellFormatterValue =
+ typeof rawHeaderCellValue === 'string' &&
+ rawHeaderCellValue.trim() !== '' &&
+ Number.isFinite(Number(rawHeaderCellValue))
+ ? Number(rawHeaderCellValue)
+ : rawHeaderCellValue;
+ const headerCellFormattedValue =
+ dateFormatters?.[attrName]?.(headerCellFormatterValue) ??
+ rawHeaderCellValue;
+ const isActiveHeader = colLabelClass.includes('active');
+ const { backgroundColor, color } = getCellColor(
+ [attrName],
+ headerCellFormattedValue,
+ cellColorFormatters,
+ isActiveHeader ? activeHeaderBackgroundColor : cellBackgroundColor,
);
- };
- const headerCellFormattedValue =
- dateFormatters?.[attrName]?.(
- convertToNumberIfNumeric(colKey[attrIdx]),
- ) ?? colKey[attrIdx];
- const { backgroundColor, color } = getCellColor(
- [attrName],
- headerCellFormattedValue,
- cellColorFormatters,
- isActiveHeader ? activeHeaderBackgroundColor : cellBackgroundColor,
- );
- const style = {
- backgroundColor,
- ...(color ? { color } : {}),
- };
- attrValueCells.push(
+ const colHeaderStyle = {
+ backgroundColor,
+ ...(color ? { color } : {}),
+ };
+ attrValueCells.push(
+ <th
+ className={colLabelClass}
+ key={`colKey-${flatColKey}`}
+ style={colHeaderStyle}
+ colSpan={colSpan}
+ rowSpan={rowSpan}
+ onClick={clickHeaderHandler(
+ pivotData,
+ colKey,
+ cols,
+ attrIdx,
+ tableOptions.clickColumnHeaderCallback,
+ )}
+ onContextMenu={handleContextMenu}
+ >
+ {displayHeaderCell(
+ needToggle,
+ collapsedCols[flatColKey] ? arrowCollapsed : arrowExpanded,
+ onArrowClick,
+ headerCellFormattedValue,
+ namesMapping,
+ settingsAllowRenderHtml,
+ )}
+ <span
+ role="button"
+ tabIndex={0}
+ // Prevents event bubbling to avoid conflict with column
header click handlers
+ // Ensures sort operation executes without triggering
cross-filtration
+ onClick={e => {
+ e.stopPropagation();
+ }}
+ aria-label={
+ activeSortColumn === i
+ ? `Sorted by ${columnName} ${sortingOrder[i] === 'asc' ?
'ascending' : 'descending'}`
+ : undefined
+ }
+ >
+ {visibleSortIcon && getSortIcon(i)}
+ </span>
+ </th>,
+ );
+ } else if (attrIdx === colKey.length) {
+ const rowSpan = settingsColAttrs.length - colKey.length +
rowIncrSpan;
+ attrValueCells.push(
+ <th
+ className={`${colLabelClass} pvtSubtotalLabel`}
+ key={`colKeyBuffer-${flatKey(colKey)}`}
+ colSpan={colSpan}
+ rowSpan={rowSpan}
+ onClick={clickHeaderHandler(
+ pivotData,
+ colKey,
+ cols,
+ attrIdx,
+ tableOptions.clickColumnHeaderCallback,
+ true,
+ )}
+ >
+ {t('Subtotal')}
+ </th>,
+ );
+ }
+ // The next colSpan columns will have the same value anyway...
+ i += colSpan;
+ }
+
+ const totalCell =
+ attrIdx === 0 && rowTotals ? (
<th
- className={colLabelClass}
- key={`colKey-${flatColKey}`}
- style={style}
- colSpan={colSpan}
- rowSpan={rowSpan}
- role="columnheader button"
- onClick={this.clickHeaderHandler(
+ key="total"
+ className="pvtTotalLabel"
+ rowSpan={
+ settingsColAttrs.length + Math.min(settingsRowAttrs.length, 1)
+ }
+ onClick={clickHeaderHandler(
pivotData,
- colKey,
- this.props.cols,
+ [],
+ cols,
attrIdx,
- this.props.tableOptions.clickColumnHeaderCallback,
+ tableOptions.clickColumnHeaderCallback,
+ false,
+ true,
)}
- onContextMenu={handleContextMenu}
>
- {displayHeaderCell(
- needToggle,
- this.state.collapsedCols[flatColKey]
- ? arrowCollapsed
- : arrowExpanded,
- onArrowClick,
- headerCellFormattedValue,
- namesMapping,
- allowRenderHtml,
- )}
- <span
- role="button"
- tabIndex={0}
- // Prevents event bubbling to avoid conflict with column header
click handlers
- // Ensures sort operation executes without triggering
cross-filtration
- onClick={e => {
- e.stopPropagation();
- }}
- aria-label={
- this.state.activeSortColumn === i
- ? `Sorted by ${columnName} ${this.state.sortingOrder[i] ===
'asc' ? 'ascending' : 'descending'}`
- : undefined
- }
- >
- {visibleSortIcon && getSortIcon(i)}
- </span>
- </th>,
- );
- } else if (attrIdx === colKey.length) {
- const rowSpan = colAttrs.length - colKey.length + rowIncrSpan;
- attrValueCells.push(
+ {t('Total (%(aggregatorName)s)', {
+ aggregatorName: t(aggregatorName),
+ })}
+ </th>
+ ) : null;
+
+ const cells = [spaceCell, attrNameCell, ...attrValueCells, totalCell];
+ return <tr key={`colAttr-${attrIdx}`}>{cells}</tr>;
+ },
+ [
+ tableOptions,
+ onContextMenu,
+ collapseAttr,
+ expandAttr,
+ toggleColKey,
+ clickHeaderHandler,
+ cols,
+ aggregatorName,
+ activeSortColumn,
+ sortingOrder,
+ collapsedCols,
+ sortData,
+ ],
+ );
+
+ const renderRowHeaderRow = useCallback(
+ (settings: PivotSettings) => {
+ // Render just the attribute names of the rows (the actual attribute
values
+ // will show up in the individual rows).
+
+ const {
+ rowAttrs: settingsRowAttrs,
+ colAttrs: settingsColAttrs,
+ rowKeys: settingsRowKeys,
+ arrowCollapsed,
+ arrowExpanded,
+ rowSubtotalDisplay: settingsRowSubtotalDisplay,
+ maxRowVisible,
+ pivotData,
+ namesMapping,
+ allowRenderHtml: settingsAllowRenderHtml,
+ } = settings;
+ return (
+ <tr key="rowHdr">
+ {settingsRowAttrs.map((r, i) => {
+ const needLabelToggle =
+ settingsRowSubtotalDisplay.enabled === true &&
+ i !== settingsRowAttrs.length - 1;
+ let arrowClickHandle = null;
+ let subArrow = null;
+ if (needLabelToggle) {
+ arrowClickHandle =
+ i + 1 < maxRowVisible!
+ ? collapseAttr(true, i, settingsRowKeys)
+ : expandAttr(true, i, settingsRowKeys);
+ subArrow =
+ i + 1 < maxRowVisible! ? arrowExpanded : arrowCollapsed;
+ }
+ return (
+ <th className="pvtAxisLabel" key={`rowAttr-${i}`}>
+ {displayHeaderCell(
+ needLabelToggle,
+ subArrow,
+ arrowClickHandle,
+ r,
+ namesMapping,
+ settingsAllowRenderHtml,
+ )}
+ </th>
+ );
+ })}
<th
- className={`${colLabelClass} pvtSubtotalLabel`}
- key={`colKeyBuffer-${flatKey(colKey)}`}
- colSpan={colSpan}
- rowSpan={rowSpan}
- role="columnheader button"
- onClick={this.clickHeaderHandler(
+ className="pvtTotalLabel"
+ key="padding"
+ onClick={clickHeaderHandler(
pivotData,
- colKey,
- this.props.cols,
- attrIdx,
- this.props.tableOptions.clickColumnHeaderCallback,
+ [],
+ rows,
+ 0,
+ tableOptions.clickRowHeaderCallback,
+ false,
true,
)}
>
- {t('Subtotal')}
- </th>,
- );
- }
- // The next colSpan columns will have the same value anyway...
- i += colSpan;
- }
-
- const totalCell =
- attrIdx === 0 && rowTotals ? (
- <th
- key="total"
- className="pvtTotalLabel"
- rowSpan={colAttrs.length + Math.min(rowAttrs.length, 1)}
- role="columnheader button"
- onClick={this.clickHeaderHandler(
- pivotData,
- [],
- this.props.cols,
- attrIdx,
- this.props.tableOptions.clickColumnHeaderCallback,
- false,
- true,
- )}
- >
- {t('Total (%(aggregatorName)s)', {
- aggregatorName: t(this.props.aggregatorName),
- })}
- </th>
- ) : null;
-
- const cells = [spaceCell, attrNameCell, ...attrValueCells, totalCell];
- return <tr key={`colAttr-${attrIdx}`}>{cells}</tr>;
- }
+ {settingsColAttrs.length === 0
+ ? t('Total (%(aggregatorName)s)', {
+ aggregatorName: t(aggregatorName),
+ })
+ : null}
+ </th>
+ </tr>
+ );
+ },
+ [
+ collapseAttr,
+ expandAttr,
+ clickHeaderHandler,
+ rows,
+ tableOptions.clickRowHeaderCallback,
+ aggregatorName,
+ ],
+ );
- renderRowHeaderRow(pivotSettings: PivotSettings) {
- // Render just the attribute names of the rows (the actual attribute values
- // will show up in the individual rows).
+ const renderTableRow = useCallback(
+ (rowKey: string[], rowIdx: number, settings: PivotSettings) => {
+ // Render a single row in the pivot table.
- const {
- rowAttrs,
- colAttrs,
- rowKeys,
- arrowCollapsed,
- arrowExpanded,
- rowSubtotalDisplay,
- maxRowVisible,
- pivotData,
- namesMapping,
- allowRenderHtml,
- } = pivotSettings;
- return (
- <tr key="rowHdr">
- {rowAttrs.map((r, i) => {
- const needLabelToggle =
- rowSubtotalDisplay.enabled === true && i !== rowAttrs.length - 1;
- let arrowClickHandle = null;
- let subArrow = null;
- if (needLabelToggle) {
- arrowClickHandle =
- i + 1 < maxRowVisible!
- ? this.collapseAttr(true, i, rowKeys)
- : this.expandAttr(true, i, rowKeys);
- subArrow = i + 1 < maxRowVisible! ? arrowExpanded : arrowCollapsed;
+ const {
+ rowAttrs: settingsRowAttrs,
+ colAttrs: settingsColAttrs,
+ rowAttrSpans,
+ visibleColKeys: settingsVisibleColKeys,
+ pivotData,
+ rowTotals,
+ rowSubtotalDisplay: settingsRowSubtotalDisplay,
+ arrowExpanded,
+ arrowCollapsed,
+ cellCallbacks,
+ rowTotalCallbacks,
+ namesMapping,
+ allowRenderHtml: settingsAllowRenderHtml,
+ } = settings;
+
+ const {
+ highlightHeaderCellsOnHover,
+ omittedHighlightHeaderGroups = [],
+ highlightedHeaderCells,
+ cellColorFormatters,
+ dateFormatters,
+ cellBackgroundColor = supersetTheme.colorBgBase,
+ cellTextColor = supersetTheme.colorPrimaryText,
+ activeHeaderBackgroundColor = supersetTheme.colorPrimaryBg,
+ } = tableOptions;
+ const flatRowKey = flatKey(rowKey);
+
+ const colIncrSpan = settingsColAttrs.length !== 0 ? 1 : 0;
+ const attrValueCells = rowKey.map((r: string, i: number) => {
+ let handleContextMenu: ((e: MouseEvent) => void) | undefined;
+ let valueCellClassName = 'pvtRowLabel';
+ if (!omittedHighlightHeaderGroups.includes(settingsRowAttrs[i])) {
+ if (highlightHeaderCellsOnHover) {
+ valueCellClassName += ' hoverable';
}
+ handleContextMenu = (e: MouseEvent) =>
+ onContextMenu(e, undefined, rowKey, {
+ [settingsRowAttrs[i]]: r,
+ });
+ }
+ if (
+ highlightedHeaderCells &&
+ Array.isArray(highlightedHeaderCells[settingsRowAttrs[i]]) &&
+ highlightedHeaderCells[settingsRowAttrs[i]].includes(r)
+ ) {
+ valueCellClassName += ' active';
+ }
+ const rowSpan = rowAttrSpans![rowIdx][i];
+ if (rowSpan > 0) {
+ const flatRowKeySlice = flatKey(rowKey.slice(0, i + 1));
+ const colSpan =
+ 1 + (i === settingsRowAttrs.length - 1 ? colIncrSpan : 0);
+ const needRowToggle =
+ settingsRowSubtotalDisplay.enabled === true &&
+ i !== settingsRowAttrs.length - 1;
+ const onArrowClick = needRowToggle
+ ? toggleRowKey(flatRowKeySlice)
+ : null;
+
+ // Coerce numeric timestamp strings to numbers so temporal formatters
+ // (which typically expect an epoch) render correctly.
+ const headerFormatterValue =
+ typeof r === 'string' &&
+ r.trim() !== '' &&
+ Number.isFinite(Number(r))
+ ? Number(r)
+ : r;
+ const headerCellFormattedValue =
+ dateFormatters?.[settingsRowAttrs[i]]?.(headerFormatterValue) ?? r;
+ const isActiveHeader = valueCellClassName.includes('active');
+ const { backgroundColor, color } = getCellColor(
+ [settingsRowAttrs[i]],
+ headerCellFormattedValue,
+ cellColorFormatters,
+ isActiveHeader ? activeHeaderBackgroundColor : cellBackgroundColor,
+ );
+ const rowHeaderStyle = {
+ backgroundColor,
+ ...(color ? { color } : {}),
+ };
return (
- <th className="pvtAxisLabel" key={`rowAttr-${i}`}>
+ <th
+ key={`rowKeyLabel-${i}`}
+ className={valueCellClassName}
+ style={rowHeaderStyle}
+ rowSpan={rowSpan}
+ colSpan={colSpan}
+ onClick={clickHeaderHandler(
+ pivotData,
+ rowKey,
+ rows,
+ i,
+ tableOptions.clickRowHeaderCallback,
+ )}
+ onContextMenu={handleContextMenu}
+ >
{displayHeaderCell(
- needLabelToggle,
- subArrow,
- arrowClickHandle,
- r,
+ needRowToggle,
+ collapsedRows[flatRowKeySlice] ? arrowCollapsed :
arrowExpanded,
+ onArrowClick,
+ headerCellFormattedValue,
namesMapping,
- allowRenderHtml,
+ settingsAllowRenderHtml,
)}
</th>
);
- })}
- <th
- className="pvtTotalLabel"
- key="padding"
- role="columnheader button"
- onClick={this.clickHeaderHandler(
- pivotData,
- [],
- this.props.rows,
- 0,
- this.props.tableOptions.clickRowHeaderCallback,
- false,
- true,
- )}
- >
- {colAttrs.length === 0
- ? t('Total (%(aggregatorName)s)', {
- aggregatorName: t(this.props.aggregatorName),
- })
- : null}
- </th>
- </tr>
- );
- }
+ }
+ return null;
+ });
- renderTableRow(
- rowKey: string[],
- rowIdx: number,
- pivotSettings: PivotSettings,
- ) {
- // Render a single row in the pivot table.
+ const attrValuePaddingCell =
+ rowKey.length < settingsRowAttrs.length ? (
+ <th
+ className="pvtRowLabel pvtSubtotalLabel"
+ key="rowKeyBuffer"
+ colSpan={settingsRowAttrs.length - rowKey.length + colIncrSpan}
+ rowSpan={1}
+ onClick={clickHeaderHandler(
+ pivotData,
+ rowKey,
+ rows,
+ rowKey.length,
+ tableOptions.clickRowHeaderCallback,
+ true,
+ )}
+ >
+ {t('Subtotal')}
+ </th>
+ ) : null;
- const {
- rowAttrs,
- colAttrs,
- rowAttrSpans,
- visibleColKeys,
- pivotData,
- rowTotals,
- rowSubtotalDisplay,
- arrowExpanded,
- arrowCollapsed,
- cellCallbacks,
- rowTotalCallbacks,
- namesMapping,
- allowRenderHtml,
- } = pivotSettings;
-
- const {
- highlightHeaderCellsOnHover,
- omittedHighlightHeaderGroups = [],
- highlightedHeaderCells,
- cellColorFormatters,
- dateFormatters,
- cellBackgroundColor = supersetTheme.colorBgBase,
- cellTextColor = supersetTheme.colorPrimaryText,
- activeHeaderBackgroundColor = supersetTheme.colorPrimaryBg,
- } = this.props.tableOptions;
- const flatRowKey = flatKey(rowKey);
-
- const colIncrSpan = colAttrs.length !== 0 ? 1 : 0;
- const attrValueCells = rowKey.map((r: string, i: number) => {
- let handleContextMenu: ((e: MouseEvent) => void) | undefined;
- let valueCellClassName = 'pvtRowLabel';
- if (!omittedHighlightHeaderGroups.includes(rowAttrs[i])) {
- if (highlightHeaderCellsOnHover) {
- valueCellClassName += ' hoverable';
- }
- handleContextMenu = (e: MouseEvent) =>
- this.props.onContextMenu(e, undefined, rowKey, {
- [rowAttrs[i]]: r,
- });
- }
- if (
- highlightedHeaderCells &&
- Array.isArray(highlightedHeaderCells[rowAttrs[i]]) &&
- highlightedHeaderCells[rowAttrs[i]].includes(r)
- ) {
- valueCellClassName += ' active';
+ if (!settingsVisibleColKeys) {
+ return null;
}
- const isActiveHeader = valueCellClassName.includes('active');
- const rowSpan = rowAttrSpans![rowIdx][i];
- if (rowSpan > 0) {
- const flatRowKey = flatKey(rowKey.slice(0, i + 1));
- const colSpan = 1 + (i === rowAttrs.length - 1 ? colIncrSpan : 0);
- const needRowToggle =
- rowSubtotalDisplay.enabled === true && i !== rowAttrs.length - 1;
- const onArrowClick = needRowToggle
- ? this.toggleRowKey(flatRowKey)
- : null;
-
- const headerCellFormattedValue =
- dateFormatters?.[rowAttrs[i]]?.(convertToNumberIfNumeric(r)) ?? r;
+ const rowClickHandlers = cellCallbacks[flatRowKey] || {};
+ const valueCells = settingsVisibleColKeys.map((colKey: string[]) => {
+ const flatColKey = flatKey(colKey);
+ const agg = pivotData.getAggregator(rowKey, colKey);
+ const aggValue = agg.value();
+
+ const keys = [...rowKey, ...colKey];
const { backgroundColor, color } = getCellColor(
- [rowAttrs[i]],
- headerCellFormattedValue,
+ keys,
+ aggValue,
cellColorFormatters,
- isActiveHeader ? activeHeaderBackgroundColor : cellBackgroundColor,
+ cellBackgroundColor,
);
- const style = {
- backgroundColor,
- ...(color ? { color } : {}),
- };
+
+ const style = agg.isSubtotal
+ ? {
+ backgroundColor,
+ fontWeight: 'bold',
+ color: color ?? cellTextColor,
+ }
+ : { backgroundColor, color: color ?? cellTextColor };
Review Comment:
Optional: the style/contrast logic for formatted headers and subtotal cells
is still present after the hook conversion, but the rewritten tests dropped the
earlier assertions that protected these paths. Since regressions here make
headers and subtotal cells unreadable in production, it would be worth keeping
at least one focused style assertion for a formatted header and one
subtotal/value cell.
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.tsx:
##########
@@ -714,727 +825,707 @@ export class TableRenderer extends Component<
};
newSortingOrder[columnIndex] = newDirection;
- const cacheKey =
`${columnIndex}-${visibleColKeys.length}-${rowEnabled}-${rowPartialOnTop}-${newDirection}`;
+ // Include the target column's flat key so different visible-column sets
+ // with the same length don't collide in the cache.
+ const sortTargetKey = flatKey(visColKeys[columnIndex] ?? []);
+ const cacheKey =
`${columnIndex}-${visColKeys.length}-${sortTargetKey}-${rowEnabled}-${rowPartialOnTop}-${newDirection}`;
let newRowKeys;
- if (this.sortCache.has(cacheKey)) {
- const cachedRowKeys = this.sortCache.get(cacheKey);
+ if (sortCacheRef.current.has(cacheKey)) {
+ const cachedRowKeys = sortCacheRef.current.get(cacheKey);
newRowKeys = cachedRowKeys;
} else {
- const groups = this.getAggregatedData(
+ const groups = getAggregatedData(
pivotData,
- visibleColKeys[columnIndex],
+ visColKeys[columnIndex],
rowPartialOnTop,
);
- const sortedRowKeys = this.sortAndCacheData(
+ const computedSortedRowKeys = sortAndCacheData(
groups,
newDirection,
rowEnabled,
rowPartialOnTop,
maxRowIndex,
);
- this.sortCache.set(cacheKey, sortedRowKeys);
- newRowKeys = sortedRowKeys;
+ sortCacheRef.current.set(cacheKey, computedSortedRowKeys);
+ newRowKeys = computedSortedRowKeys;
}
- this.cachedBasePivotSettings = {
- ...this.cachedBasePivotSettings!,
- rowKeys: newRowKeys!,
- };
- return {
- sortingOrder: newSortingOrder,
- activeSortColumn: columnIndex,
- };
- });
- }
+ setSortedRowKeys(newRowKeys!);
+ setSortingOrder(newSortingOrder);
+ setActiveSortColumn(columnIndex);
+ },
+ [activeSortColumn, sortingOrder, getAggregatedData, sortAndCacheData],
+ );
- renderColHeaderRow(
- attrName: string,
- attrIdx: number,
- pivotSettings: PivotSettings,
- ) {
- // Render a single row in the column header at the top of the pivot table.
+ const renderColHeaderRow = useCallback(
+ (attrName: string, attrIdx: number, settings: PivotSettings) => {
+ // Render a single row in the column header at the top of the pivot
table.
+
+ const {
+ rowAttrs: settingsRowAttrs,
+ colAttrs: settingsColAttrs,
+ colKeys: settingsColKeys,
+ visibleColKeys: settingsVisibleColKeys,
+ colAttrSpans,
+ rowTotals,
+ arrowExpanded,
+ arrowCollapsed,
+ colSubtotalDisplay: settingsColSubtotalDisplay,
+ maxColVisible,
+ pivotData,
+ namesMapping,
+ allowRenderHtml: settingsAllowRenderHtml,
+ } = settings;
+ const {
+ highlightHeaderCellsOnHover,
+ omittedHighlightHeaderGroups = [],
+ highlightedHeaderCells,
+ cellColorFormatters,
+ dateFormatters,
+ cellBackgroundColor = supersetTheme.colorBgBase,
+ activeHeaderBackgroundColor = supersetTheme.colorPrimaryBg,
+ } = tableOptions;
- const {
- rowAttrs,
- colAttrs,
- colKeys,
- visibleColKeys,
- colAttrSpans,
- rowTotals,
- arrowExpanded,
- arrowCollapsed,
- colSubtotalDisplay,
- maxColVisible,
- pivotData,
- namesMapping,
- allowRenderHtml,
- } = pivotSettings;
- const {
- highlightHeaderCellsOnHover,
- omittedHighlightHeaderGroups = [],
- highlightedHeaderCells,
- cellColorFormatters,
- dateFormatters,
- cellBackgroundColor = supersetTheme.colorBgBase,
- activeHeaderBackgroundColor = supersetTheme.colorPrimaryBg,
- } = this.props.tableOptions;
-
- if (!visibleColKeys || !colAttrSpans) {
- return null;
- }
+ if (!settingsVisibleColKeys || !colAttrSpans) {
+ return null;
+ }
- const spaceCell =
- attrIdx === 0 && rowAttrs.length !== 0 ? (
- <th
- key="padding"
- colSpan={rowAttrs.length}
- rowSpan={colAttrs.length}
- aria-hidden="true"
- />
- ) : null;
-
- const needToggle =
- colSubtotalDisplay.enabled === true && attrIdx !== colAttrs.length - 1;
- let arrowClickHandle = null;
- let subArrow = null;
- if (needToggle) {
- arrowClickHandle =
- attrIdx + 1 < maxColVisible!
- ? this.collapseAttr(false, attrIdx, colKeys)
- : this.expandAttr(false, attrIdx, colKeys);
- subArrow = attrIdx + 1 < maxColVisible! ? arrowExpanded : arrowCollapsed;
- }
- const attrNameCell = (
- <th key="label" className="pvtAxisLabel">
- {displayHeaderCell(
- needToggle,
- subArrow,
- arrowClickHandle,
- attrName,
- namesMapping,
- allowRenderHtml,
- )}
- </th>
- );
-
- const attrValueCells = [];
- const rowIncrSpan = rowAttrs.length !== 0 ? 1 : 0;
- // Iterate through columns. Jump over duplicate values.
- let i = 0;
- while (i < visibleColKeys.length) {
- let handleContextMenu: ((e: MouseEvent) => void) | undefined;
- const colKey = visibleColKeys[i];
- const colSpan = attrIdx < colKey.length ? colAttrSpans[i][attrIdx] : 1;
- let colLabelClass = 'pvtColLabel';
- if (attrIdx < colKey.length) {
- if (!omittedHighlightHeaderGroups.includes(colAttrs[attrIdx])) {
- if (highlightHeaderCellsOnHover) {
- colLabelClass += ' hoverable';
+ const spaceCell =
+ attrIdx === 0 && settingsRowAttrs.length !== 0 ? (
+ <th
+ key="padding"
+ colSpan={settingsRowAttrs.length}
+ rowSpan={settingsColAttrs.length}
+ aria-hidden="true"
+ />
+ ) : null;
+
+ const needToggle =
+ settingsColSubtotalDisplay.enabled === true &&
+ attrIdx !== settingsColAttrs.length - 1;
+ let arrowClickHandle = null;
+ let subArrow = null;
+ if (needToggle) {
+ arrowClickHandle =
+ attrIdx + 1 < maxColVisible!
+ ? collapseAttr(false, attrIdx, settingsColKeys)
+ : expandAttr(false, attrIdx, settingsColKeys);
+ subArrow =
+ attrIdx + 1 < maxColVisible! ? arrowExpanded : arrowCollapsed;
+ }
+ const attrNameCell = (
+ <th key="label" className="pvtAxisLabel">
+ {displayHeaderCell(
+ needToggle,
+ subArrow,
+ arrowClickHandle,
+ attrName,
+ namesMapping,
+ settingsAllowRenderHtml,
+ )}
+ </th>
+ );
+
+ const attrValueCells = [];
+ const rowIncrSpan = settingsRowAttrs.length !== 0 ? 1 : 0;
+ // Iterate through columns. Jump over duplicate values.
+ let i = 0;
+ while (i < settingsVisibleColKeys.length) {
+ let handleContextMenu: ((e: MouseEvent) => void) | undefined;
+ const colKey = settingsVisibleColKeys[i];
+ const colSpan = attrIdx < colKey.length ? colAttrSpans[i][attrIdx] : 1;
+ let colLabelClass = 'pvtColLabel';
+ if (attrIdx < colKey.length) {
+ if (
+ !omittedHighlightHeaderGroups.includes(settingsColAttrs[attrIdx])
+ ) {
+ if (highlightHeaderCellsOnHover) {
+ colLabelClass += ' hoverable';
+ }
+ handleContextMenu = (e: MouseEvent) =>
+ onContextMenu(e, colKey, undefined, {
+ [attrName]: colKey[attrIdx],
+ });
}
- handleContextMenu = (e: MouseEvent) =>
- this.props.onContextMenu(e, colKey, undefined, {
- [attrName]: colKey[attrIdx],
- });
- }
- if (
- highlightedHeaderCells &&
- Array.isArray(highlightedHeaderCells[colAttrs[attrIdx]]) &&
- highlightedHeaderCells[colAttrs[attrIdx]].includes(colKey[attrIdx])
- ) {
- colLabelClass += ' active';
- }
- const isActiveHeader = colLabelClass.includes('active');
- const maxRowIndex = pivotSettings.maxRowVisible!;
- const mColVisible = pivotSettings.maxColVisible!;
- const visibleSortIcon = mColVisible - 1 === attrIdx;
- const columnName = colKey[mColVisible - 1];
-
- const rowSpan = 1 + (attrIdx === colAttrs.length - 1 ? rowIncrSpan :
0);
- const flatColKey = flatKey(colKey.slice(0, attrIdx + 1));
- const onArrowClick = needToggle ? this.toggleColKey(flatColKey) : null;
- const getSortIcon = (key: number) => {
- const { activeSortColumn, sortingOrder } = this.state;
-
- if (activeSortColumn !== key) {
+ if (
+ highlightedHeaderCells &&
+ Array.isArray(highlightedHeaderCells[settingsColAttrs[attrIdx]]) &&
+ highlightedHeaderCells[settingsColAttrs[attrIdx]].includes(
+ colKey[attrIdx],
+ )
+ ) {
+ colLabelClass += ' active';
+ }
+ const maxRowIndex = settings.maxRowVisible!;
+ const mColVisible = settings.maxColVisible!;
+ const visibleSortIcon = mColVisible - 1 === attrIdx;
+ const columnName = colKey[mColVisible - 1];
+
+ const rowSpan =
+ 1 + (attrIdx === settingsColAttrs.length - 1 ? rowIncrSpan : 0);
+ const flatColKey = flatKey(colKey.slice(0, attrIdx + 1));
+ const onArrowClick = needToggle ? toggleColKey(flatColKey) : null;
+ const getSortIcon = (key: number) => {
+ if (activeSortColumn !== key) {
+ return (
+ <ColumnHeightOutlined
+ onClick={() =>
+ sortData(
+ key,
+ settingsVisibleColKeys,
+ pivotData,
+ maxRowIndex,
+ )
+ }
+ />
+ );
+ }
+
+ const SortIcon =
+ sortingOrder[key] === 'asc' ? CaretUpOutlined :
CaretDownOutlined;
return (
- <ColumnHeightOutlined
+ <SortIcon
onClick={() =>
- this.sortData(key, visibleColKeys, pivotData, maxRowIndex)
+ sortData(key, settingsVisibleColKeys, pivotData, maxRowIndex)
}
/>
);
- }
-
- const SortIcon =
- sortingOrder[key] === 'asc' ? CaretUpOutlined : CaretDownOutlined;
- return (
- <SortIcon
- onClick={() =>
- this.sortData(key, visibleColKeys, pivotData, maxRowIndex)
- }
- />
+ };
+ // Coerce numeric timestamp strings to numbers so temporal formatters
+ // (which typically expect an epoch) render correctly.
+ const rawHeaderCellValue = colKey[attrIdx];
+ const headerCellFormatterValue =
+ typeof rawHeaderCellValue === 'string' &&
+ rawHeaderCellValue.trim() !== '' &&
+ Number.isFinite(Number(rawHeaderCellValue))
+ ? Number(rawHeaderCellValue)
+ : rawHeaderCellValue;
+ const headerCellFormattedValue =
+ dateFormatters?.[attrName]?.(headerCellFormatterValue) ??
+ rawHeaderCellValue;
+ const isActiveHeader = colLabelClass.includes('active');
+ const { backgroundColor, color } = getCellColor(
+ [attrName],
+ headerCellFormattedValue,
+ cellColorFormatters,
+ isActiveHeader ? activeHeaderBackgroundColor : cellBackgroundColor,
);
- };
- const headerCellFormattedValue =
- dateFormatters?.[attrName]?.(
- convertToNumberIfNumeric(colKey[attrIdx]),
- ) ?? colKey[attrIdx];
- const { backgroundColor, color } = getCellColor(
- [attrName],
- headerCellFormattedValue,
- cellColorFormatters,
- isActiveHeader ? activeHeaderBackgroundColor : cellBackgroundColor,
- );
- const style = {
- backgroundColor,
- ...(color ? { color } : {}),
- };
- attrValueCells.push(
+ const colHeaderStyle = {
+ backgroundColor,
+ ...(color ? { color } : {}),
+ };
+ attrValueCells.push(
+ <th
+ className={colLabelClass}
+ key={`colKey-${flatColKey}`}
+ style={colHeaderStyle}
+ colSpan={colSpan}
+ rowSpan={rowSpan}
+ onClick={clickHeaderHandler(
+ pivotData,
+ colKey,
+ cols,
+ attrIdx,
+ tableOptions.clickColumnHeaderCallback,
+ )}
+ onContextMenu={handleContextMenu}
+ >
+ {displayHeaderCell(
+ needToggle,
+ collapsedCols[flatColKey] ? arrowCollapsed : arrowExpanded,
+ onArrowClick,
+ headerCellFormattedValue,
+ namesMapping,
+ settingsAllowRenderHtml,
+ )}
+ <span
+ role="button"
+ tabIndex={0}
+ // Prevents event bubbling to avoid conflict with column
header click handlers
+ // Ensures sort operation executes without triggering
cross-filtration
+ onClick={e => {
+ e.stopPropagation();
+ }}
+ aria-label={
+ activeSortColumn === i
+ ? `Sorted by ${columnName} ${sortingOrder[i] === 'asc' ?
'ascending' : 'descending'}`
+ : undefined
+ }
+ >
+ {visibleSortIcon && getSortIcon(i)}
+ </span>
+ </th>,
+ );
+ } else if (attrIdx === colKey.length) {
+ const rowSpan = settingsColAttrs.length - colKey.length +
rowIncrSpan;
+ attrValueCells.push(
+ <th
+ className={`${colLabelClass} pvtSubtotalLabel`}
+ key={`colKeyBuffer-${flatKey(colKey)}`}
+ colSpan={colSpan}
+ rowSpan={rowSpan}
+ onClick={clickHeaderHandler(
+ pivotData,
+ colKey,
+ cols,
+ attrIdx,
+ tableOptions.clickColumnHeaderCallback,
+ true,
+ )}
+ >
+ {t('Subtotal')}
+ </th>,
+ );
+ }
+ // The next colSpan columns will have the same value anyway...
+ i += colSpan;
+ }
+
+ const totalCell =
+ attrIdx === 0 && rowTotals ? (
<th
- className={colLabelClass}
- key={`colKey-${flatColKey}`}
- style={style}
- colSpan={colSpan}
- rowSpan={rowSpan}
- role="columnheader button"
- onClick={this.clickHeaderHandler(
+ key="total"
+ className="pvtTotalLabel"
+ rowSpan={
+ settingsColAttrs.length + Math.min(settingsRowAttrs.length, 1)
+ }
+ onClick={clickHeaderHandler(
pivotData,
- colKey,
- this.props.cols,
+ [],
+ cols,
attrIdx,
- this.props.tableOptions.clickColumnHeaderCallback,
+ tableOptions.clickColumnHeaderCallback,
+ false,
+ true,
)}
- onContextMenu={handleContextMenu}
>
- {displayHeaderCell(
- needToggle,
- this.state.collapsedCols[flatColKey]
- ? arrowCollapsed
- : arrowExpanded,
- onArrowClick,
- headerCellFormattedValue,
- namesMapping,
- allowRenderHtml,
- )}
- <span
- role="button"
- tabIndex={0}
- // Prevents event bubbling to avoid conflict with column header
click handlers
- // Ensures sort operation executes without triggering
cross-filtration
- onClick={e => {
- e.stopPropagation();
- }}
- aria-label={
- this.state.activeSortColumn === i
- ? `Sorted by ${columnName} ${this.state.sortingOrder[i] ===
'asc' ? 'ascending' : 'descending'}`
- : undefined
- }
- >
- {visibleSortIcon && getSortIcon(i)}
- </span>
- </th>,
- );
- } else if (attrIdx === colKey.length) {
- const rowSpan = colAttrs.length - colKey.length + rowIncrSpan;
- attrValueCells.push(
+ {t('Total (%(aggregatorName)s)', {
+ aggregatorName: t(aggregatorName),
+ })}
+ </th>
+ ) : null;
+
+ const cells = [spaceCell, attrNameCell, ...attrValueCells, totalCell];
+ return <tr key={`colAttr-${attrIdx}`}>{cells}</tr>;
+ },
+ [
+ tableOptions,
+ onContextMenu,
+ collapseAttr,
+ expandAttr,
+ toggleColKey,
+ clickHeaderHandler,
+ cols,
+ aggregatorName,
+ activeSortColumn,
+ sortingOrder,
+ collapsedCols,
+ sortData,
+ ],
+ );
+
+ const renderRowHeaderRow = useCallback(
+ (settings: PivotSettings) => {
+ // Render just the attribute names of the rows (the actual attribute
values
+ // will show up in the individual rows).
+
+ const {
+ rowAttrs: settingsRowAttrs,
+ colAttrs: settingsColAttrs,
+ rowKeys: settingsRowKeys,
+ arrowCollapsed,
+ arrowExpanded,
+ rowSubtotalDisplay: settingsRowSubtotalDisplay,
+ maxRowVisible,
+ pivotData,
+ namesMapping,
+ allowRenderHtml: settingsAllowRenderHtml,
+ } = settings;
+ return (
+ <tr key="rowHdr">
+ {settingsRowAttrs.map((r, i) => {
+ const needLabelToggle =
+ settingsRowSubtotalDisplay.enabled === true &&
+ i !== settingsRowAttrs.length - 1;
+ let arrowClickHandle = null;
+ let subArrow = null;
+ if (needLabelToggle) {
+ arrowClickHandle =
+ i + 1 < maxRowVisible!
+ ? collapseAttr(true, i, settingsRowKeys)
+ : expandAttr(true, i, settingsRowKeys);
+ subArrow =
+ i + 1 < maxRowVisible! ? arrowExpanded : arrowCollapsed;
+ }
+ return (
+ <th className="pvtAxisLabel" key={`rowAttr-${i}`}>
+ {displayHeaderCell(
+ needLabelToggle,
+ subArrow,
+ arrowClickHandle,
+ r,
+ namesMapping,
+ settingsAllowRenderHtml,
+ )}
+ </th>
+ );
+ })}
<th
- className={`${colLabelClass} pvtSubtotalLabel`}
- key={`colKeyBuffer-${flatKey(colKey)}`}
- colSpan={colSpan}
- rowSpan={rowSpan}
- role="columnheader button"
- onClick={this.clickHeaderHandler(
+ className="pvtTotalLabel"
+ key="padding"
+ onClick={clickHeaderHandler(
pivotData,
- colKey,
- this.props.cols,
- attrIdx,
- this.props.tableOptions.clickColumnHeaderCallback,
+ [],
+ rows,
+ 0,
+ tableOptions.clickRowHeaderCallback,
+ false,
true,
)}
>
- {t('Subtotal')}
- </th>,
- );
- }
- // The next colSpan columns will have the same value anyway...
- i += colSpan;
- }
-
- const totalCell =
- attrIdx === 0 && rowTotals ? (
- <th
- key="total"
- className="pvtTotalLabel"
- rowSpan={colAttrs.length + Math.min(rowAttrs.length, 1)}
- role="columnheader button"
- onClick={this.clickHeaderHandler(
- pivotData,
- [],
- this.props.cols,
- attrIdx,
- this.props.tableOptions.clickColumnHeaderCallback,
- false,
- true,
- )}
- >
- {t('Total (%(aggregatorName)s)', {
- aggregatorName: t(this.props.aggregatorName),
- })}
- </th>
- ) : null;
-
- const cells = [spaceCell, attrNameCell, ...attrValueCells, totalCell];
- return <tr key={`colAttr-${attrIdx}`}>{cells}</tr>;
- }
+ {settingsColAttrs.length === 0
+ ? t('Total (%(aggregatorName)s)', {
+ aggregatorName: t(aggregatorName),
+ })
+ : null}
+ </th>
+ </tr>
+ );
+ },
+ [
+ collapseAttr,
+ expandAttr,
+ clickHeaderHandler,
+ rows,
+ tableOptions.clickRowHeaderCallback,
+ aggregatorName,
+ ],
+ );
- renderRowHeaderRow(pivotSettings: PivotSettings) {
- // Render just the attribute names of the rows (the actual attribute values
- // will show up in the individual rows).
+ const renderTableRow = useCallback(
+ (rowKey: string[], rowIdx: number, settings: PivotSettings) => {
+ // Render a single row in the pivot table.
- const {
- rowAttrs,
- colAttrs,
- rowKeys,
- arrowCollapsed,
- arrowExpanded,
- rowSubtotalDisplay,
- maxRowVisible,
- pivotData,
- namesMapping,
- allowRenderHtml,
- } = pivotSettings;
- return (
- <tr key="rowHdr">
- {rowAttrs.map((r, i) => {
- const needLabelToggle =
- rowSubtotalDisplay.enabled === true && i !== rowAttrs.length - 1;
- let arrowClickHandle = null;
- let subArrow = null;
- if (needLabelToggle) {
- arrowClickHandle =
- i + 1 < maxRowVisible!
- ? this.collapseAttr(true, i, rowKeys)
- : this.expandAttr(true, i, rowKeys);
- subArrow = i + 1 < maxRowVisible! ? arrowExpanded : arrowCollapsed;
+ const {
+ rowAttrs: settingsRowAttrs,
+ colAttrs: settingsColAttrs,
+ rowAttrSpans,
+ visibleColKeys: settingsVisibleColKeys,
+ pivotData,
+ rowTotals,
+ rowSubtotalDisplay: settingsRowSubtotalDisplay,
+ arrowExpanded,
+ arrowCollapsed,
+ cellCallbacks,
+ rowTotalCallbacks,
+ namesMapping,
+ allowRenderHtml: settingsAllowRenderHtml,
+ } = settings;
+
+ const {
+ highlightHeaderCellsOnHover,
+ omittedHighlightHeaderGroups = [],
+ highlightedHeaderCells,
+ cellColorFormatters,
+ dateFormatters,
+ cellBackgroundColor = supersetTheme.colorBgBase,
+ cellTextColor = supersetTheme.colorPrimaryText,
+ activeHeaderBackgroundColor = supersetTheme.colorPrimaryBg,
+ } = tableOptions;
+ const flatRowKey = flatKey(rowKey);
+
+ const colIncrSpan = settingsColAttrs.length !== 0 ? 1 : 0;
+ const attrValueCells = rowKey.map((r: string, i: number) => {
+ let handleContextMenu: ((e: MouseEvent) => void) | undefined;
+ let valueCellClassName = 'pvtRowLabel';
+ if (!omittedHighlightHeaderGroups.includes(settingsRowAttrs[i])) {
+ if (highlightHeaderCellsOnHover) {
+ valueCellClassName += ' hoverable';
}
+ handleContextMenu = (e: MouseEvent) =>
+ onContextMenu(e, undefined, rowKey, {
+ [settingsRowAttrs[i]]: r,
+ });
+ }
+ if (
+ highlightedHeaderCells &&
+ Array.isArray(highlightedHeaderCells[settingsRowAttrs[i]]) &&
+ highlightedHeaderCells[settingsRowAttrs[i]].includes(r)
+ ) {
+ valueCellClassName += ' active';
+ }
+ const rowSpan = rowAttrSpans![rowIdx][i];
+ if (rowSpan > 0) {
+ const flatRowKeySlice = flatKey(rowKey.slice(0, i + 1));
+ const colSpan =
+ 1 + (i === settingsRowAttrs.length - 1 ? colIncrSpan : 0);
+ const needRowToggle =
+ settingsRowSubtotalDisplay.enabled === true &&
+ i !== settingsRowAttrs.length - 1;
+ const onArrowClick = needRowToggle
+ ? toggleRowKey(flatRowKeySlice)
+ : null;
+
+ // Coerce numeric timestamp strings to numbers so temporal formatters
+ // (which typically expect an epoch) render correctly.
+ const headerFormatterValue =
+ typeof r === 'string' &&
+ r.trim() !== '' &&
+ Number.isFinite(Number(r))
+ ? Number(r)
+ : r;
+ const headerCellFormattedValue =
+ dateFormatters?.[settingsRowAttrs[i]]?.(headerFormatterValue) ?? r;
Review Comment:
Mandatory: this refactor keeps separate coercion paths for column and row
header date formatters, but the rewritten tests removed the previous cases that
verified numeric timestamp strings versus plain strings. Because this logic is
easy to break during refactors, both paths should keep explicit coverage.
--
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]
