diff --git a/.github/workflows/loki.yml b/.github/workflows/loki.yml
index b7aa7e6bc6c720e0525065a23ac343d585216b53..698e3efb74ec8455a4df2e2025714670cb0e8f50 100644
--- a/.github/workflows/loki.yml
+++ b/.github/workflows/loki.yml
@@ -69,7 +69,7 @@ jobs:
- name: Run Loki Visual Tests
run: |
- yarn loki test
+ yarn test-visual:loki
- name: Generate Visual Report on Failure
if: failure()
diff --git a/.loki/reference/chrome_laptop_viz_PivotTable_Default.png b/.loki/reference/chrome_laptop_viz_PivotTable_Default.png
index 0c12038c00fd941820b9c134f0d9ce2e11a50aa3..96beed7eea72cd540521fb9bc9c83fdd6ba4795a 100644
Binary files a/.loki/reference/chrome_laptop_viz_PivotTable_Default.png and b/.loki/reference/chrome_laptop_viz_PivotTable_Default.png differ
diff --git a/.loki/reference/chrome_laptop_viz_PivotTable_Horizontal_Scroll_43215.png b/.loki/reference/chrome_laptop_viz_PivotTable_Horizontal_Scroll_43215.png
new file mode 100644
index 0000000000000000000000000000000000000000..ae615be4341aab0dd5a780619ef9aa66086ed790
Binary files /dev/null and b/.loki/reference/chrome_laptop_viz_PivotTable_Horizontal_Scroll_43215.png differ
diff --git a/.loki/reference/chrome_laptop_viz_TableInteractive_Default.png b/.loki/reference/chrome_laptop_viz_TableInteractive_Default.png
index 861b847ff479faaf15ca38bd9a68d7c0558e8cde..3ff24f5b2c75039e9b02756351c6fb57cfbb24f7 100644
Binary files a/.loki/reference/chrome_laptop_viz_TableInteractive_Default.png and b/.loki/reference/chrome_laptop_viz_TableInteractive_Default.png differ
diff --git a/e2e/test/scenarios/visualizations-tabular/pivot_tables.cy.spec.js b/e2e/test/scenarios/visualizations-tabular/pivot_tables.cy.spec.js
index cd370f2a40551443ef3d7d7cdf6b39b58e0c0b1e..b76b8375d0917ef354c3645a195b45cc7c3dc18b 100644
--- a/e2e/test/scenarios/visualizations-tabular/pivot_tables.cy.spec.js
+++ b/e2e/test/scenarios/visualizations-tabular/pivot_tables.cy.spec.js
@@ -16,7 +16,10 @@ import {
openStaticEmbeddingModal,
modal,
createQuestion,
+ dashboardCards,
+ queryBuilderMain,
} from "e2e/support/helpers";
+import { PIVOT_TABLE_BODY_LABEL } from "metabase/visualizations/visualizations/PivotTable/constants";
const {
ORDERS,
@@ -1016,6 +1019,69 @@ describe("scenarios > visualizations > pivot tables", { tags: "@slow" }, () => {
}
});
+ it("should be horizontally scrollable when columns overflow", () => {
+ const createdAtField = [
+ "field",
+ REVIEWS.CREATED_AT,
+ {
+ "temporal-unit": "month",
+ "base-type": "type/DateTimeWithLocalTZ",
+ },
+ ];
+ const ratingField = [
+ "field",
+ REVIEWS.RATING,
+ { "base-type": "type/Integer" },
+ ];
+
+ const query = {
+ "source-table": REVIEWS_ID,
+ aggregation: [["count"]],
+ breakout: [createdAtField, ratingField],
+ };
+
+ const vizSettings = {
+ rows: ratingField,
+ columns: createdAtField,
+ "pivot_table.column_split": {
+ rows: [ratingField],
+ columns: [createdAtField],
+ values: [["aggregation", 0]],
+ },
+ };
+
+ cy.createQuestionAndDashboard({
+ questionDetails: {
+ name: QUESTION_NAME,
+ query,
+ display: "pivot",
+ visualization_settings: vizSettings,
+ },
+ dashboardDetails: {
+ name: DASHBOARD_NAME,
+ },
+ cardDetails: {
+ size_x: 16,
+ size_y: 8,
+ },
+ }).then(({ body: { dashboard_id }, questionId }) => {
+ cy.wrap(questionId).as("questionId");
+ visitDashboard(dashboard_id);
+ });
+
+ dashboardCards().within(() => {
+ cy.findByLabelText(PIVOT_TABLE_BODY_LABEL).scrollTo(10000, 0);
+ cy.findByText("Row totals").should("be.visible");
+ });
+
+ cy.get("@questionId").then(id => visitQuestion(id));
+
+ queryBuilderMain().within(() => {
+ cy.findByLabelText(PIVOT_TABLE_BODY_LABEL).scrollTo(10000, 0);
+ cy.findByText("Row totals").should("be.visible");
+ });
+ });
+
describe("column resizing", () => {
const getCellWidth = textEl =>
textEl.closest("[data-testid=pivot-table-cell]").width();
diff --git a/frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTable.stories.tsx b/frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTable.stories.tsx
index 90a95ec09792164457b0fbba3860013343b30e94..019062c849d843f04bd50e5bdd540cd6df1a52bb 100644
--- a/frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTable.stories.tsx
+++ b/frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTable.stories.tsx
@@ -7,6 +7,7 @@ import {
import { PivotTable } from "./PivotTable";
import { PivotTableTestWrapper } from "./pivot-table-test-mocks";
+import { HORIZONTAL_SCROLL_43215 } from "./stories-data";
export default {
title: "viz/PivotTable",
@@ -50,3 +51,14 @@ export const EmbeddingTheme: Story = () => {
</SdkVisualizationWrapper>
);
};
+
+export const HorizontalScroll43215: Story = () => {
+ return (
+ <VisualizationWrapper>
+ <PivotTableTestWrapper
+ data={HORIZONTAL_SCROLL_43215.data}
+ initialSettings={HORIZONTAL_SCROLL_43215.initialSettings}
+ />
+ </VisualizationWrapper>
+ );
+};
diff --git a/frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTable.tsx b/frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTable.tsx
index d01e01da2c0b50bbbbd4b818688420bce80e700b..526df24aa19a61bd4482e7ed35cb8f9fbe4b4993 100644
--- a/frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTable.tsx
+++ b/frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTable.tsx
@@ -9,6 +9,7 @@ import { Grid, Collection, ScrollSync, AutoSizer } from "react-virtualized";
import { t } from "ttag";
import _ from "underscore";
+import ExplicitSize from "metabase/components/ExplicitSize";
import CS from "metabase/css/core/index.css";
import { sumArray } from "metabase/lib/arrays";
import {
@@ -42,6 +43,7 @@ import {
CELL_HEIGHT,
LEFT_HEADER_LEFT_SPACING,
MIN_HEADER_CELL_WIDTH,
+ PIVOT_TABLE_BODY_LABEL,
} from "./constants";
import {
settings,
@@ -66,6 +68,7 @@ interface PivotTableProps {
data: DatasetData;
settings: VisualizationSettings;
width: number;
+ height: number;
onUpdateVisualizationSettings: (settings: VisualizationSettings) => void;
isNightMode: boolean;
isDashboard: boolean;
@@ -73,10 +76,11 @@ interface PivotTableProps {
onVisualizationClick: (options: any) => void;
}
-function PivotTable({
+function _PivotTable({
data,
settings,
width,
+ height,
onUpdateVisualizationSettings,
isNightMode,
isDashboard,
@@ -296,6 +300,7 @@ function PivotTable({
columnIndexes.length + (valueIndexes.length > 1 ? 1 : 0) || 1;
const topHeaderHeight = topHeaderRows * CELL_HEIGHT;
+ const bodyHeight = height - topHeaderHeight;
const leftHeaderWidth =
rowIndexes.length > 0
@@ -411,7 +416,7 @@ function PivotTable({
{/* left header */}
<div style={{ width: leftHeaderWidth }}>
<AutoSizer disableWidth>
- {({ height }) => (
+ {() => (
<Collection
ref={leftHeaderRef}
className={CS.scrollHideAll}
@@ -438,7 +443,7 @@ function PivotTable({
)
}
width={leftHeaderWidth}
- height={height - scrollBarOffsetSize()}
+ height={bodyHeight - scrollBarOffsetSize()}
scrollTop={scrollTop}
onScroll={({ scrollTop }) =>
onScroll({ scrollTop } as OnScrollParams)
@@ -450,10 +455,11 @@ function PivotTable({
{/* pivot table body */}
<div>
<AutoSizer disableWidth>
- {({ height }) => (
+ {() => (
<Grid
+ aria-label={PIVOT_TABLE_BODY_LABEL}
width={width - leftHeaderWidth}
- height={height}
+ height={bodyHeight}
className={CS.textDark}
rowCount={rowCount}
columnCount={columnCount}
@@ -506,6 +512,15 @@ function PivotTable({
);
}
+const PivotTable = ExplicitSize<
+ PivotTableProps & {
+ className?: string;
+ }
+>({
+ wrapped: false,
+ refreshMode: "debounceLeading",
+})(_PivotTable);
+
// eslint-disable-next-line import/no-default-export -- deprecated usage
export default Object.assign(connect(mapStateToProps)(PivotTable), {
uiName: t`Pivot Table`,
diff --git a/frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTable.unit.spec.tsx b/frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTable.unit.spec.tsx
index 8c6d4519567cd4f6d45a3047193f13392ed3fa45..b94f9ee8a3cfcc0fbf0cea233c0c7109ddea7504 100644
--- a/frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTable.unit.spec.tsx
+++ b/frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTable.unit.spec.tsx
@@ -14,6 +14,11 @@ const { rows, cols, settings } = PIVOT_TABLE_MOCK_DATA;
// 3 isn't a real column, it's a pivot-grouping
const columnIndexes = [0, 1, 2, 4, 5];
+const TEST_CASES = [
+ { name: "dashboard", isDashboard: true },
+ { name: "query builder", isDashboard: false },
+];
+
function setup(options?: any) {
renderWithProviders(<PivotTableTestWrapper {...options} />);
}
@@ -54,95 +59,107 @@ describe("Visualizations > PivotTable > PivotTable", () => {
);
});
- it("should render pivot table wrapper", async () => {
- setup();
- expect(await screen.findByTestId("pivot-table")).toBeInTheDocument();
- });
+ TEST_CASES.forEach(testCase => {
+ describe(` > ${testCase.name}`, () => {
+ it("should render pivot table wrapper", async () => {
+ setup({ isDashboard: testCase.isDashboard });
+ expect(await screen.findByTestId("pivot-table")).toBeInTheDocument();
+ });
- it("should render column names", () => {
- setup();
+ it("should render column names", () => {
+ setup({ isDashboard: testCase.isDashboard });
- // all column names except 3, the pivot grouping, should be in the document
- columnIndexes.forEach(colIndex => {
- expect(screen.getByText(cols[colIndex].display_name)).toBeInTheDocument();
- });
- });
+ // all column names except 3, the pivot grouping, should be in the document
+ columnIndexes.forEach(colIndex => {
+ expect(
+ screen.getByText(cols[colIndex].display_name),
+ ).toBeInTheDocument();
+ });
+ });
- it("should render column values", () => {
- setup();
+ it("should render column values", () => {
+ setup({ isDashboard: testCase.isDashboard });
- rows.forEach(rowData => {
- columnIndexes.forEach(colIndex => {
- expect(screen.getByTestId("pivot-table")).toHaveTextContent(
- rowData[colIndex].toString(),
- );
+ rows.forEach(rowData => {
+ columnIndexes.forEach(colIndex => {
+ expect(screen.getByTestId("pivot-table")).toHaveTextContent(
+ rowData[colIndex].toString(),
+ );
+ });
+ });
});
- });
- });
- it("should collapse columns", () => {
- const hiddenSettings = {
- ...settings,
- "pivot_table.collapsed_rows": {
- rows: [cols[0].field_ref, cols[1].field_ref, cols[2].field_ref],
- value: ["2"],
- },
- } as unknown as VisualizationSettings;
-
- setup({ initialSettings: hiddenSettings });
-
- const COLLAPSED_COLUMN_INDEX = 1;
-
- rows.forEach(row => {
- const totalsElement = screen.getByText(
- `Totals for ${row[COLLAPSED_COLUMN_INDEX]}`,
- );
- expect(totalsElement).toBeInTheDocument();
-
- const totalsContainer = screen.getByTestId(
- `${row[COLLAPSED_COLUMN_INDEX]}-toggle-button`,
- );
-
- expect(
- within(totalsContainer).getByRole("img", {
- name: /add/i,
- }),
- ).toBeInTheDocument();
- });
- });
+ it("should collapse columns", () => {
+ const hiddenSettings = {
+ ...settings,
+ "pivot_table.collapsed_rows": {
+ rows: [cols[0].field_ref, cols[1].field_ref, cols[2].field_ref],
+ value: ["2"],
+ },
+ } as unknown as VisualizationSettings;
+
+ setup({
+ initialSettings: hiddenSettings,
+ isDashboard: testCase.isDashboard,
+ });
+
+ const COLLAPSED_COLUMN_INDEX = 1;
+
+ rows.forEach(row => {
+ const totalsElement = screen.getByText(
+ `Totals for ${row[COLLAPSED_COLUMN_INDEX]}`,
+ );
+ expect(totalsElement).toBeInTheDocument();
+
+ const totalsContainer = screen.getByTestId(
+ `${row[COLLAPSED_COLUMN_INDEX]}-toggle-button`,
+ );
+
+ expect(
+ within(totalsContainer).getByRole("img", {
+ name: /add/i,
+ }),
+ ).toBeInTheDocument();
+ });
+ });
- it("expanding collapsed columns", async () => {
- const hiddenSettings = {
- ...settings,
- "pivot_table.collapsed_rows": {
- rows: [cols[0].field_ref, cols[1].field_ref, cols[2].field_ref],
- value: ["2"],
- },
- } as unknown as VisualizationSettings;
+ it("expanding collapsed columns", async () => {
+ const hiddenSettings = {
+ ...settings,
+ "pivot_table.collapsed_rows": {
+ rows: [cols[0].field_ref, cols[1].field_ref, cols[2].field_ref],
+ value: ["2"],
+ },
+ } as unknown as VisualizationSettings;
- setup({ initialSettings: hiddenSettings });
+ setup({
+ initialSettings: hiddenSettings,
+ isDashboard: testCase.isDashboard,
+ });
- const COLLAPSED_COLUMN_INDEX = 1;
+ const COLLAPSED_COLUMN_INDEX = 1;
- const LAST_ROW = rows[3];
+ const LAST_ROW = rows[3];
- // Find and click the toggle button to expand the last row
- // as it's the easiest to make assertions on
- const toggleButton = screen.getByTestId(
- `${LAST_ROW[COLLAPSED_COLUMN_INDEX]}-toggle-button`,
- );
+ // Find and click the toggle button to expand the last row
+ // as it's the easiest to make assertions on
+ const toggleButton = screen.getByTestId(
+ `${LAST_ROW[COLLAPSED_COLUMN_INDEX]}-toggle-button`,
+ );
- expect(
- within(toggleButton).getByRole("img", { name: /add/i }),
- ).toBeInTheDocument();
+ expect(
+ within(toggleButton).getByRole("img", { name: /add/i }),
+ ).toBeInTheDocument();
- await userEvent.click(toggleButton);
+ await userEvent.click(toggleButton);
- //Ensure that collapsed data is now visible
- columnIndexes.forEach(columnIndex => {
- expect(
- screen.getByText(LAST_ROW[columnIndex].toString()),
- ).toBeInTheDocument();
+ //Ensure that collapsed data is now visible
+ columnIndexes.forEach(columnIndex => {
+ expect(
+ screen.getByText(LAST_ROW[columnIndex].toString()),
+ ).toBeInTheDocument();
+ });
+ });
});
});
});
diff --git a/frontend/src/metabase/visualizations/visualizations/PivotTable/constants.ts b/frontend/src/metabase/visualizations/visualizations/PivotTable/constants.ts
index a5bcbf34cfb2f3259c862d26edbf916637337527..09925eabd555e54f453618551f7d91e9cfe8a149 100644
--- a/frontend/src/metabase/visualizations/visualizations/PivotTable/constants.ts
+++ b/frontend/src/metabase/visualizations/visualizations/PivotTable/constants.ts
@@ -17,3 +17,5 @@ export const LEFT_HEADER_LEFT_SPACING = 24;
export const MAX_ROWS_TO_MEASURE = 100;
export const RESIZE_HANDLE_WIDTH = 5;
+
+export const PIVOT_TABLE_BODY_LABEL = "pivot-table-body-grid";
diff --git a/frontend/src/metabase/visualizations/visualizations/PivotTable/pivot-table-test-mocks.tsx b/frontend/src/metabase/visualizations/visualizations/PivotTable/pivot-table-test-mocks.tsx
index a5bbc4e7595887240fabde88ee004b20ea966659..7d616d3f58e18410d91e5f018a9bcedf64fbb0c7 100644
--- a/frontend/src/metabase/visualizations/visualizations/PivotTable/pivot-table-test-mocks.tsx
+++ b/frontend/src/metabase/visualizations/visualizations/PivotTable/pivot-table-test-mocks.tsx
@@ -85,11 +85,10 @@ export function PivotTableTestWrapper(props?: any) {
);
return (
- <Box h="400px">
+ <Box h="400px" w="600px">
<PivotTable
settings={vizSettings}
data={{ rows, cols }}
- width={600}
onVisualizationClick={() => {}}
onUpdateVisualizationSettings={newSettings =>
setVizSettings({ ...vizSettings, ...newSettings })
diff --git a/frontend/src/metabase/visualizations/visualizations/PivotTable/stories-data.ts b/frontend/src/metabase/visualizations/visualizations/PivotTable/stories-data.ts
new file mode 100644
index 0000000000000000000000000000000000000000..91048425db1419ca1422116d86516eb1981ae919
--- /dev/null
+++ b/frontend/src/metabase/visualizations/visualizations/PivotTable/stories-data.ts
@@ -0,0 +1,101 @@
+const cols = [
+ {
+ source: "breakout",
+ field_ref: ["field", 123, null],
+ display_name: "field-123",
+ name: "field-123",
+ },
+ {
+ source: "breakout",
+ field_ref: ["field", 456, null],
+ display_name: "field-456",
+ name: "field-456",
+ },
+ {
+ source: "breakout",
+ field_ref: ["field", 789, null],
+ display_name: "field-789",
+ name: "field-789",
+ },
+ {
+ source: "breakout",
+ field_ref: ["expression", "pivot-grouping"],
+ name: "pivot-grouping",
+ display_name: "pivot-grouping",
+ },
+ {
+ source: "aggregation",
+ field_ref: ["aggregation", 1],
+ display_name: "aggregation-1",
+ },
+ {
+ source: "aggregation",
+ field_ref: ["aggregation", 2],
+ display_name: "aggregation-2",
+ },
+ {
+ source: "aggregation",
+ field_ref: ["aggregation", 3],
+ display_name: "aggregation-3",
+ },
+ {
+ source: "aggregation",
+ field_ref: ["aggregation", 4],
+ display_name: "aggregation-4",
+ },
+ {
+ source: "aggregation",
+ field_ref: ["aggregation", 5],
+ display_name: "aggregation-5",
+ },
+ {
+ source: "aggregation",
+ field_ref: ["aggregation", 6],
+ display_name: "aggregation-6",
+ },
+];
+
+const rows = [
+ ["foo1", "bar1", "baz1", 0, 111, 222, 100, 100, 100, 100],
+ ["foo1", "bar1", "baz2", 0, 777, 888, 2000, 2000, 2000, 2000],
+ ["foo2", "bar2", "baz2", 0, 333, 444, 30000, 30000, 30000, 30000],
+ ["foo3", "bar3", "baz3", 0, 555, 666, 400000, 400000, 400000, 400000],
+];
+
+const pivotSettings = {
+ "pivot.show_column_totals": true,
+ "pivot.show_row_totals": true,
+ "pivot_table.collapsed_rows": {
+ rows: [cols[0].field_ref, cols[1].field_ref, cols[2].field_ref],
+ value: [],
+ },
+ "pivot_table.column_split": {
+ columns: [],
+ rows: [cols[0].field_ref, cols[1].field_ref, cols[2].field_ref],
+ values: [
+ cols[4].field_ref,
+ cols[5].field_ref,
+ cols[6].field_ref,
+ cols[7].field_ref,
+ cols[8].field_ref,
+ cols[9].field_ref,
+ ],
+ },
+ "table.column_formatting": [],
+ column_settings: {},
+};
+
+export const HORIZONTAL_SCROLL_43215 = {
+ data: {
+ cols,
+ rows,
+ },
+ initialSettings: {
+ ...pivotSettings,
+ column: (c: any) => ({
+ ...pivotSettings,
+ column: c,
+ column_title: c.display_name,
+ }),
+ },
+};
diff --git a/package.json b/package.json
index 11f129deddabfd7b3a844da58946c708a0090774..b3aca2b977594d357a834cb93ad7f31a2ff83a56 100644
--- a/package.json
+++ b/package.json
@@ -390,8 +390,8 @@
"test-unit-watch": "yarn test-unit --watch",
"test-unit-watch:cljs": "yarn concurrently -n 'cljs,tests' 'yarn build-watch:cljs' 'yarn test-unit-keep-cljs --watch'",
"test-visual": "yarn build && ./bin/build-for-test && yarn test-visual-run",
- "test-visual:loki": "yarn loki test",
- "test-visual:loki-update": "yarn loki update",
+ "test-visual:loki": "yarn loki test --chromeFlags='--headless --disable-gpu'",
+ "test-visual:loki-update": "yarn loki update --chromeFlags='--headless --disable-gpu'",
"test-visual:loki-report": "node frontend/test/generate-loki-report-json.js && reg-cli --from .loki/report.json --report .loki/report.html",
"test-visual:loki-report-open": "yarn test-visual:loki || (echo 'Visual test failed, opening report...' && yarn test-visual:loki-report && open-cli .loki/report.html)",
"type-check": "yarn clean:cljs && yarn build:cljs && yarn type-check-pure",