diff --git a/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_Dark_Theme_No_Background_Default.png b/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_Dark_Theme_No_Background_Default.png index 5583b8869580cc51549b93621b08e369bc5f6cda..2a95a2a19ddd1db108b05b2c89a1119aa190fb0c 100644 Binary files a/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_Dark_Theme_No_Background_Default.png and b/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_Dark_Theme_No_Background_Default.png differ diff --git a/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Dark_Theme_Date_Filter_Quarter_Year_Dropdown.png b/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Dark_Theme_Date_Filter_Quarter_Year_Dropdown.png index 3883f48de71ca8298d53d96d0b34f9a5124dd040..4c6abc129e36302394a21da3ea1907b545b1e39b 100644 Binary files a/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Dark_Theme_Date_Filter_Quarter_Year_Dropdown.png and b/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Dark_Theme_Date_Filter_Quarter_Year_Dropdown.png differ diff --git a/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Dark_Theme_Number.png b/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Dark_Theme_Number.png index 1fead004731791636f92c22276cd6e63ffb9ffca..1214fa5e39165f256383040ca50bd119ff1c1601 100644 Binary files a/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Dark_Theme_Number.png and b/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Dark_Theme_Number.png differ diff --git a/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Light_Theme_Date_Filter_Quarter_Year_Dropdown.png b/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Light_Theme_Date_Filter_Quarter_Year_Dropdown.png index bb6659b6e95d139d1e2aeb429418944a67b55264..4ae0fa3108ca52e068ffbc4c8a0e53861035d890 100644 Binary files a/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Light_Theme_Date_Filter_Quarter_Year_Dropdown.png and b/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Light_Theme_Date_Filter_Quarter_Year_Dropdown.png differ diff --git a/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Light_Theme_Parameter_List_With_Value.png b/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Light_Theme_Parameter_List_With_Value.png index f5901f6cab61d51be0a0c5ccb9c0885349421643..923a976c1e85039d5ae0be1faf504e22194605a0 100644 Binary files a/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Light_Theme_Parameter_List_With_Value.png and b/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Light_Theme_Parameter_List_With_Value.png differ diff --git a/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Light_Theme_Parameter_Search_With_Value.png b/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Light_Theme_Parameter_Search_With_Value.png index d73f3f82406320c4472b67cef05353a121907097..43ab8fae5dc6a91883f9ce85105fbc3dfbefe63c 100644 Binary files a/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Light_Theme_Parameter_Search_With_Value.png and b/.loki/reference/chrome_laptop_embed_PublicOrEmbeddedDashboardView_filters_Light_Theme_Parameter_Search_With_Value.png differ diff --git a/e2e/test/scenarios/visualizations-charts/visualizations-charts-reproductions.cy.spec.js b/e2e/test/scenarios/visualizations-charts/visualizations-charts-reproductions.cy.spec.js index 18dac970eab2c716c782a25f76cdcd6152fdd1c8..410c17c41b18eb039e0d873057147197f77a849b 100644 --- a/e2e/test/scenarios/visualizations-charts/visualizations-charts-reproductions.cy.spec.js +++ b/e2e/test/scenarios/visualizations-charts/visualizations-charts-reproductions.cy.spec.js @@ -7,6 +7,7 @@ import { cartesianChartCircle, chartPathWithFillColor, createQuestion, + describeEE, echartsContainer, editDashboard, filter, @@ -24,9 +25,11 @@ import { saveDashboard, saveSavedQuestion, selectFilterOperator, + setTokenFeatures, sidebar, summarize, testPairedTooltipValues, + updateSetting, visitAlias, visitDashboard, visitQuestionAdhoc, @@ -1207,3 +1210,55 @@ describe("issue 43077", () => { cy.wait(100).then(() => expect(cardRequestSpy).not.to.have.been.called); }); }); + +describeEE("issue 49160", () => { + beforeEach(() => { + restore(); + cy.signInAsAdmin(); + setTokenFeatures("all"); + }); + + it("pie chart should have a placeholder", () => { + visitQuestionAdhoc({ + dataset_query: { + type: "query", + query: { + "source-table": PRODUCTS_ID, + }, + database: SAMPLE_DB_ID, + }, + display: "pie", + }); + + // Shows pie placeholder + echartsContainer().findByText("18,760").should("be.visible"); + cy.findByTestId("qb-header-action-panel").findByText("Summarize").click(); + + cy.findByLabelText("Rating").click(); + echartsContainer().findByText("200").should("be.visible"); + echartsContainer().findByText("TOTAL").should("be.visible"); + }); + + it("pie chart should work when instance colors have overrides", () => { + updateSetting("application-colors", { "accent0-light": "#98b4ce" }); + + visitQuestionAdhoc({ + dataset_query: { + type: "query", + query: { + "source-table": PRODUCTS_ID, + aggregation: [["count"]], + breakout: [["field", PRODUCTS.CATEGORY, null]], + }, + database: SAMPLE_DB_ID, + }, + display: "pie", + }); + + echartsContainer().findByText("200").should("be.visible"); + + cy.findByTestId("viz-settings-button").click(); + + leftSidebar().findByText("Gizmo"); + }); +}); diff --git a/frontend/src/metabase/visualizations/echarts/pie/model/index.ts b/frontend/src/metabase/visualizations/echarts/pie/model/index.ts index c6f20af21d3704efecccc1219a239d863c0c4df4..ef2cf2ed26b17d57b97958d8b2c1752a290b5b2e 100644 --- a/frontend/src/metabase/visualizations/echarts/pie/model/index.ts +++ b/frontend/src/metabase/visualizations/echarts/pie/model/index.ts @@ -330,11 +330,11 @@ export function getPieChartModel( if (!hasMultipleRings) { return hexColor; } - const accentNumber = hexToAccentColorMap.get(hexColor); - if (accentNumber == null) { + const accentKey = hexToAccentColorMap.get(hexColor); + if (accentKey == null) { return hexColor; } - return renderingContext.getColor(getRingColorAlias(accentNumber, ring)); + return renderingContext.getColor(getRingColorAlias(accentKey, ring)); } // Create sliceTree, fill out the innermost slice ring diff --git a/frontend/src/metabase/visualizations/echarts/pie/util/colors.ts b/frontend/src/metabase/visualizations/echarts/pie/util/colors.ts index ebd017d5c7bc5b6c70ee9a30a2358b4458b10407..d8f6d5593d1f852d4eb4fe3d20ed2ddad17a5bc3 100644 --- a/frontend/src/metabase/visualizations/echarts/pie/util/colors.ts +++ b/frontend/src/metabase/visualizations/echarts/pie/util/colors.ts @@ -1,41 +1,45 @@ import { aliases, colors } from "metabase/lib/colors"; -import { checkNumber } from "metabase/lib/types"; const ACCENT_KEY_PREFIX = "accent"; +type AccentKey = string; + +const isAccentColorKey = (key: string) => key.startsWith(ACCENT_KEY_PREFIX); + +const extractAccentKey = (input: string): AccentKey => { + const withoutPrefix = input.slice(ACCENT_KEY_PREFIX.length); + return withoutPrefix.split("-")[0] ?? null; +}; + export function createHexToAccentNumberMap() { - const hexToAccentNumber = new Map<string, number>(); + const hexToAccentNumber = new Map<string, AccentKey>(); - for (const [key, hex] of Object.entries(colors)) { - if (!key.startsWith(ACCENT_KEY_PREFIX)) { + for (const [colorKey, hex] of Object.entries(colors)) { + if (!isAccentColorKey(colorKey)) { continue; } - - const accentNumber = checkNumber( - Number(key.slice(ACCENT_KEY_PREFIX.length)), - ); - - hexToAccentNumber.set(hex, accentNumber); + const accentKey = extractAccentKey(colorKey); + if (accentKey) { + hexToAccentNumber.set(hex, accentKey); + } } - for (const [key, hexGetter] of Object.entries(aliases)) { - if (!key.startsWith(ACCENT_KEY_PREFIX)) { + for (const [colorKey, hexGetter] of Object.entries(aliases)) { + if (!isAccentColorKey(colorKey)) { continue; } - const accentNumber = checkNumber( - Number(key.slice(ACCENT_KEY_PREFIX.length, ACCENT_KEY_PREFIX.length + 1)), - ); - const hex = hexGetter(colors); - - hexToAccentNumber.set(hex, accentNumber); + const accentKey = extractAccentKey(colorKey); + if (accentKey) { + hexToAccentNumber.set(hexGetter(colors), accentKey); + } } return hexToAccentNumber; } export function getRingColorAlias( - accentColorNumber: number, + accentKey: AccentKey, ring: "inner" | "middle" | "outer", ) { let suffix = ""; @@ -45,9 +49,9 @@ export function getRingColorAlias( suffix = "-light"; } - return `${ACCENT_KEY_PREFIX}${accentColorNumber}${suffix}`; + return `${ACCENT_KEY_PREFIX}${accentKey}${suffix}`; } -export function getPickerColorAlias(accentNumber: number) { - return `${ACCENT_KEY_PREFIX}${accentNumber}-dark`; +export function getPickerColorAlias(accentKey: AccentKey) { + return `${ACCENT_KEY_PREFIX}${accentKey}-dark`; } diff --git a/frontend/src/metabase/visualizations/visualizations/PieChart/PieChart.tsx b/frontend/src/metabase/visualizations/visualizations/PieChart/PieChart.tsx index 694fdace3ddd9f8d11d524befde12358cd85215f..64a81e545074fd1fd912ef338a27b50979969cb5 100644 --- a/frontend/src/metabase/visualizations/visualizations/PieChart/PieChart.tsx +++ b/frontend/src/metabase/visualizations/visualizations/PieChart/PieChart.tsx @@ -31,6 +31,8 @@ export function PieChart(props: VisualizationProps) { onRender, isDashboard, isFullscreen, + isPlaceholder, + series: transformedSeries, } = props; const hoveredIndex = props.hovered?.index; const hoveredSliceKeyPath = props.hovered?.pieSliceKeyPath; @@ -57,22 +59,22 @@ export function PieChart(props: VisualizationProps) { () => extractRemappings(rawSeries), [rawSeries], ); + + const seriesToRender = useMemo( + () => (isPlaceholder ? transformedSeries : rawSeriesWithRemappings), + [isPlaceholder, transformedSeries, rawSeriesWithRemappings], + ); + const chartModel = useMemo( () => getPieChartModel( - rawSeriesWithRemappings, + seriesToRender, settings, Array.from(hiddenSlices), renderingContext, showWarning, ), - [ - rawSeriesWithRemappings, - settings, - hiddenSlices, - renderingContext, - showWarning, - ], + [seriesToRender, settings, hiddenSlices, renderingContext, showWarning], ); const formatters = useMemo( () => getPieChartFormatters(chartModel, settings), diff --git a/frontend/src/metabase/visualizations/visualizations/PieChart/PieRowsPicker.tsx b/frontend/src/metabase/visualizations/visualizations/PieChart/PieRowsPicker.tsx index 70384a345d97c1495ee439118fb788d9b573f01d..5f479d6eab57b4fb3d4daeef7c95002d7dbd3e7b 100644 --- a/frontend/src/metabase/visualizations/visualizations/PieChart/PieRowsPicker.tsx +++ b/frontend/src/metabase/visualizations/visualizations/PieChart/PieRowsPicker.tsx @@ -41,11 +41,11 @@ export function PieRowsPicker({ if (!hasMultipleRings || hexColor == null) { return hexColor; } - const accentNumber = hexToAccentNumberMap.get(hexColor); - if (accentNumber == null) { + const accentKey = hexToAccentNumberMap.get(hexColor); + if (accentKey == null) { return hexColor; } - return color(getPickerColorAlias(accentNumber)); + return color(getPickerColorAlias(accentKey)); }; const onChangeSeriesColor = (sliceKey: string, color: string) => diff --git a/frontend/src/metabase/visualizations/visualizations/PieChart/chart-definition.ts b/frontend/src/metabase/visualizations/visualizations/PieChart/chart-definition.ts index ec4087f9f25b7bf069dedd60af49a815f9d97a4d..a5d33173353c2b7bd1e7874dd55f97842e7dfa94 100644 --- a/frontend/src/metabase/visualizations/visualizations/PieChart/chart-definition.ts +++ b/frontend/src/metabase/visualizations/visualizations/PieChart/chart-definition.ts @@ -77,7 +77,11 @@ export const PIE_CHART_DEFINITION: VisualizationDefinition = { if (rows.length < 1) { throw new MinRowsError(1, 0); } - if (!settings["pie.dimension"] || !settings["pie.metric"]) { + const isDimensionMissing = + !settings["pie.dimension"] || + (Array.isArray(settings["pie.dimension"]) && + settings["pie.dimension"].every(col => col == null)); + if (isDimensionMissing || !settings["pie.metric"]) { throw new ChartSettingsError(t`Which columns do you want to use?`, { section: `Data`, });