dangdtr commented on code in PR #42502:
URL: https://github.com/apache/superset/pull/42502#discussion_r3663631824
##########
superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts:
##########
@@ -24,65 +24,86 @@ interface ExtendedNativeFilterScope extends
NativeFilterScope {
selectedLayers?: string[];
}
+export type ChartLayoutItemMap = ReadonlyMap<number, LayoutItem>;
+export type ChartLayoutItems = readonly LayoutItem[] | ChartLayoutItemMap;
+
+const isChartLayoutItemArray = (
+ chartLayoutItems: ChartLayoutItems,
+): chartLayoutItems is readonly LayoutItem[] =>
Array.isArray(chartLayoutItems);
+
+export function createChartLayoutItemMap(
+ layoutItems: readonly (LayoutItem | null | undefined)[],
+): ChartLayoutItemMap {
+ const chartLayoutItemMap = new Map<number, LayoutItem>();
+
+ layoutItems.forEach(layoutItem => {
+ const chartId = layoutItem?.meta?.chartId;
+ if (
+ layoutItem?.type === CHART_TYPE &&
+ typeof chartId === 'number' &&
+ !chartLayoutItemMap.has(chartId)
+ ) {
+ chartLayoutItemMap.set(chartId, layoutItem);
Review Comment:
Thanks for catching this. Fixed in b41bbb8a8e: the index now preserves all
layout holders for each chart ID, scope membership checks any holder, and tab
discovery unions parent tabs across every holder. I also added regression tests
covering a chart whose second holder is the one in scope and tab discovery
across duplicate placements. The targeted suites pass (29 tests), and the full
pre-commit suite passes.
--
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]
