EnxDev commented on code in PR #45030:
URL: https://github.com/apache/superset/pull/45030#discussion_r4200777546
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.tsx:
##########
@@ -357,13 +388,24 @@ export function TableRenderer(props: TableRendererProps) {
const [collapsedRows, setCollapsedRows] = useState<Record<string, boolean>>(
{},
);
+ // Filled once row keys are known. The toggle reads it at click time so the
+ // default matches the prefixes that have a renderable subtotal row.
+ const collapsibleRowPrefixesRef = useRef<ReadonlySet<string>>(
+ NO_COLLAPSIBLE_PREFIXES,
+ );
const [collapsedCols, setCollapsedCols] = useState<Record<string, boolean>>(
{},
);
const [sortingOrder, setSortingOrder] = useState<string[]>([]);
const [activeSortColumn, setActiveSortColumn] = useState<number |
null>(null);
const [sortedRowKeys, setSortedRowKeys] = useState<string[][] | null>(null);
+ // Default collapse only applies while row subtotals are on. Without them
+ // there is no subtotal row to show and no arrow to expand a hidden group.
+ const collapseRowsByDefault = Boolean(
Review Comment:
Explicit toggles in `collapsedRows` always beat this default, so the Explore
preview can drift from what viewers get. Expand US, untick and re-tick
"Collapse rows by default", and US stays open, while the saved chart opens with
everything collapsed.
Could we clear `collapsedRows` when `collapseRowsByDefault` flips? An effect
keyed on it that resets the state to `{}` would cover it, plus a test that
toggles the prop after an expand.
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.tsx:
##########
@@ -788,19 +844,37 @@ export function TableRenderer(props: TableRendererProps) {
const rowKeys = effectiveRowKeys;
+ // Subtotal rows are the only prefixes default collapse may hide children
+ // under. A shorter key that was never produced (metric-only rollups are
+ // omitted for metrics-on-rows) must stay expanded.
+ const collapsibleRowPrefixes = useMemo(() => {
+ const prefixes = new Set<string>();
+ basePivotSettings.rowKeys.forEach(key => {
+ if (key.length > 0 && key.length < basePivotSettings.rowAttrs.length) {
+ prefixes.add(flatKey(key));
+ }
+ });
+ return prefixes;
+ }, [basePivotSettings]);
+ collapsibleRowPrefixesRef.current = collapsibleRowPrefixes;
Review Comment:
Nit, take it or leave it. Writing the ref during render works, but
`renderTableRow` already has `collapseRowsByDefault` and
`collapsibleRowPrefixes` in hand when it builds the arrow handler.
Passing the default in, e.g. `toggleRowKey(flatRowKeySlice,
collapseRowsByDefault && collapsibleRowPrefixes.has(flatRowKeySlice))`, lets
the updater use it directly and drops both the ref and the extra `useCallback`
dep.
--
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]
