From 12f9cd063f0222a542ca7b7dee77dc4c52c470cb Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Wed, 11 Jan 2023 15:06:52 +0200
Subject: [PATCH] Revert "Virtualize table and card lists in data picker
(#27582)" (#27625)
---
...irtualizedList.tsx => VirtualizedList.jsx} | 18 ++----------
.../CardPicker/CardPicker.styled.tsx | 4 ++-
.../DataPicker/CardPicker/CardPickerView.tsx | 22 ++++++---------
.../RawDataPicker/RawDataPicker.styled.tsx | 4 ++-
.../RawDataPicker/RawDataPickerView.tsx | 21 ++++++--------
.../DataPicker/VirtualizedSelectList.tsx | 28 -------------------
.../tests/DataPicker-Common.unit.spec.ts | 6 ++--
.../tests/DataPicker-Models.unit.spec.ts | 3 +-
.../tests/DataPicker-Questions.unit.spec.ts | 3 +-
.../tests/DataPicker-RawData.unit.spec.ts | 4 +--
.../containers/DataPicker/tests/common.tsx | 15 ----------
11 files changed, 33 insertions(+), 95 deletions(-)
rename frontend/src/metabase/components/{VirtualizedList.tsx => VirtualizedList.jsx} (69%)
delete mode 100644 frontend/src/metabase/containers/DataPicker/VirtualizedSelectList.tsx
diff --git a/frontend/src/metabase/components/VirtualizedList.tsx b/frontend/src/metabase/components/VirtualizedList.jsx
similarity index 69%
rename from frontend/src/metabase/components/VirtualizedList.tsx
rename to frontend/src/metabase/components/VirtualizedList.jsx
index acd498d2a2e..7f8104e36f0 100644
--- a/frontend/src/metabase/components/VirtualizedList.tsx
+++ b/frontend/src/metabase/components/VirtualizedList.jsx
@@ -1,20 +1,8 @@
+/* eslint-disable react/prop-types */
import React from "react";
import { List, WindowScroller, AutoSizer } from "react-virtualized";
-export interface VirtualizedListProps<Item = unknown>
- extends React.HTMLProps<HTMLUListElement> {
- items: Item[];
- rowHeight: number;
- renderItem: (props: { item: Item; index: number }) => React.ReactNode;
- scrollElement?: HTMLElement | null;
-}
-
-function VirtualizedList<Item>({
- items,
- rowHeight,
- renderItem,
- scrollElement,
-}: VirtualizedListProps<Item>) {
+function VirtualizedList({ items, rowHeight, renderItem, scrollElement }) {
const rowRenderer = React.useCallback(
({ index, key, style }) => (
<div key={key} style={style}>
@@ -27,7 +15,7 @@ function VirtualizedList<Item>({
const renderScrollComponent = React.useCallback(
({ width }) => {
return (
- <WindowScroller scrollElement={scrollElement || undefined}>
+ <WindowScroller scrollElement={scrollElement}>
{({ height, isScrolling, scrollTop }) => (
<List
autoHeight
diff --git a/frontend/src/metabase/containers/DataPicker/CardPicker/CardPicker.styled.tsx b/frontend/src/metabase/containers/DataPicker/CardPicker/CardPicker.styled.tsx
index 4c5897c86f1..5bb3ffb3db8 100644
--- a/frontend/src/metabase/containers/DataPicker/CardPicker/CardPicker.styled.tsx
+++ b/frontend/src/metabase/containers/DataPicker/CardPicker/CardPicker.styled.tsx
@@ -1,6 +1,8 @@
import styled from "@emotion/styled";
-export const ListContainer = styled.div`
+import SelectList from "metabase/components/SelectList";
+
+export const StyledSelectList = styled(SelectList)`
width: 100%;
padding-left: 1rem;
`;
diff --git a/frontend/src/metabase/containers/DataPicker/CardPicker/CardPickerView.tsx b/frontend/src/metabase/containers/DataPicker/CardPicker/CardPickerView.tsx
index cb7a60925e7..b301fd7e472 100644
--- a/frontend/src/metabase/containers/DataPicker/CardPicker/CardPickerView.tsx
+++ b/frontend/src/metabase/containers/DataPicker/CardPicker/CardPickerView.tsx
@@ -1,6 +1,8 @@
import React, { useCallback, useMemo } from "react";
import _ from "underscore";
+import SelectList from "metabase/components/SelectList";
+
import { canonicalCollectionId } from "metabase/collections/utils";
import type { ITreeNodeItem } from "metabase/components/tree/types";
@@ -11,10 +13,9 @@ import type { DataPickerSelectedItem, VirtualTable } from "../types";
import EmptyState from "../EmptyState";
import LoadingState from "../LoadingState";
-import VirtualizedSelectList from "../VirtualizedSelectList";
import PanePicker from "../PanePicker";
-import { ListContainer } from "./CardPicker.styled";
+import { StyledSelectList } from "./CardPicker.styled";
type TargetModel = "model" | "question";
@@ -54,7 +55,7 @@ function TableSelectListItem({
onSelect: (id: Table["id"]) => void;
}) {
return (
- <VirtualizedSelectList.Item
+ <SelectList.Item
id={table.id}
name={table.display_name}
isSelected={isSelected}
@@ -62,7 +63,7 @@ function TableSelectListItem({
onSelect={onSelect}
>
{table.display_name}
- </VirtualizedSelectList.Item>
+ </SelectList.Item>
);
}
@@ -103,7 +104,7 @@ function CardPickerView({
);
const renderVirtualTable = useCallback(
- ({ item: table }: { item: VirtualTable }) => (
+ (table: VirtualTable) => (
<TableSelectListItem
key={table.id}
table={table}
@@ -129,14 +130,9 @@ function CardPickerView({
) : isEmpty ? (
<EmptyState />
) : (
- <ListContainer>
- {Array.isArray(virtualTables) && (
- <VirtualizedSelectList<VirtualTable>
- items={virtualTables}
- renderItem={renderVirtualTable}
- />
- )}
- </ListContainer>
+ <StyledSelectList>
+ {virtualTables?.map?.(renderVirtualTable)}
+ </StyledSelectList>
)}
</PanePicker>
);
diff --git a/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPicker.styled.tsx b/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPicker.styled.tsx
index fb0f876d871..98464679d16 100644
--- a/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPicker.styled.tsx
+++ b/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPicker.styled.tsx
@@ -1,6 +1,8 @@
import styled from "@emotion/styled";
-export const ListContainer = styled.div`
+import SelectList from "metabase/components/SelectList";
+
+export const StyledSelectList = styled(SelectList)`
width: 100%;
padding: 0 1rem;
`;
diff --git a/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPickerView.tsx b/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPickerView.tsx
index c741e5d92de..cd741fc35e5 100644
--- a/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPickerView.tsx
+++ b/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPickerView.tsx
@@ -1,6 +1,8 @@
import React, { useCallback, useMemo } from "react";
import _ from "underscore";
+import SelectList from "metabase/components/SelectList";
+
import type { ITreeNodeItem } from "metabase/components/tree/types";
import type Database from "metabase-lib/metadata/Database";
@@ -11,9 +13,8 @@ import type { DataPickerSelectedItem } from "../types";
import EmptyState from "../EmptyState";
import LoadingState from "../LoadingState";
-import VirtualizedSelectList from "../VirtualizedSelectList";
import PanePicker from "../PanePicker";
-import { ListContainer } from "./RawDataPicker.styled";
+import { StyledSelectList } from "./RawDataPicker.styled";
interface RawDataPickerViewProps {
databases: Database[];
@@ -60,7 +61,7 @@ function TableSelectListItem({
}) {
const name = table.displayName();
return (
- <VirtualizedSelectList.Item
+ <SelectList.Item
id={table.id}
name={name}
isSelected={isSelected}
@@ -68,7 +69,7 @@ function TableSelectListItem({
onSelect={onSelect}
>
{name}
- </VirtualizedSelectList.Item>
+ </SelectList.Item>
);
}
@@ -134,8 +135,9 @@ function RawDataPickerView({
);
const renderTable = useCallback(
- ({ item: table }: { item: Table }) => (
+ (table: Table) => (
<TableSelectListItem
+ key={table.id}
table={table}
isSelected={selectedTableIds.includes(table.id)}
onSelect={onSelectedTable}
@@ -160,14 +162,7 @@ function RawDataPickerView({
) : isEmpty ? (
<EmptyState />
) : (
- <ListContainer>
- {Array.isArray(tables) && (
- <VirtualizedSelectList<Table>
- items={tables}
- renderItem={renderTable}
- />
- )}
- </ListContainer>
+ <StyledSelectList>{tables?.map?.(renderTable)}</StyledSelectList>
)}
</PanePicker>
);
diff --git a/frontend/src/metabase/containers/DataPicker/VirtualizedSelectList.tsx b/frontend/src/metabase/containers/DataPicker/VirtualizedSelectList.tsx
deleted file mode 100644
index 7ad52248e1d..00000000000
--- a/frontend/src/metabase/containers/DataPicker/VirtualizedSelectList.tsx
+++ /dev/null
@@ -1,28 +0,0 @@
-import React from "react";
-
-import VirtualizedList, {
- VirtualizedListProps,
-} from "metabase/components/VirtualizedList";
-import SelectList from "metabase/components/SelectList";
-
-const SELECT_LIST_ITEM_HEIGHT = 40;
-
-type VirtualizedSelectListProps<Item> = Omit<
- VirtualizedListProps<Item>,
- "rowHeight" | "role"
->;
-
-function VirtualizedSelectList<Item>(props: VirtualizedSelectListProps<Item>) {
- return (
- <VirtualizedList<Item>
- data-testid="select-list"
- {...props}
- role="menu"
- rowHeight={SELECT_LIST_ITEM_HEIGHT}
- />
- );
-}
-
-export default Object.assign(VirtualizedSelectList, {
- Item: SelectList.Item,
-});
diff --git a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Common.unit.spec.ts b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Common.unit.spec.ts
index eb638ade81c..d2c6dc25d66 100644
--- a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Common.unit.spec.ts
+++ b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Common.unit.spec.ts
@@ -1,13 +1,13 @@
import nock from "nock";
-import { screen } from "__support__/ui";
+import { screen, waitFor } from "__support__/ui";
import { SAMPLE_DATABASE } from "__support__/sample_database_fixture";
-import { setup, setupVirtualizedLists } from "./common";
+import { setup } from "./common";
describe("DataPicker", () => {
beforeAll(() => {
- setupVirtualizedLists();
+ window.HTMLElement.prototype.scrollIntoView = jest.fn();
});
afterEach(() => {
diff --git a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Models.unit.spec.ts b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Models.unit.spec.ts
index a1a7c36d4c1..44ac210af3e 100644
--- a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Models.unit.spec.ts
+++ b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Models.unit.spec.ts
@@ -13,7 +13,6 @@ import {
import {
setup,
- setupVirtualizedLists,
EMPTY_COLLECTION,
SAMPLE_COLLECTION,
SAMPLE_MODEL,
@@ -28,7 +27,7 @@ const ROOT_COLLECTION_MODEL_VIRTUAL_SCHEMA_ID = getCollectionVirtualSchemaId(
describe("DataPicker — picking models", () => {
beforeAll(() => {
- setupVirtualizedLists();
+ window.HTMLElement.prototype.scrollIntoView = jest.fn();
});
afterEach(() => {
diff --git a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Questions.unit.spec.ts b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Questions.unit.spec.ts
index 5d38a0b09a8..320c707e4a1 100644
--- a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Questions.unit.spec.ts
+++ b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-Questions.unit.spec.ts
@@ -13,7 +13,6 @@ import {
import {
setup,
- setupVirtualizedLists,
EMPTY_COLLECTION,
SAMPLE_COLLECTION,
SAMPLE_QUESTION,
@@ -26,7 +25,7 @@ const ROOT_COLLECTION_QUESTIONS_VIRTUAL_SCHEMA_ID =
describe("DataPicker — picking questions", () => {
beforeAll(() => {
- setupVirtualizedLists();
+ window.HTMLElement.prototype.scrollIntoView = jest.fn();
});
afterEach(() => {
diff --git a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-RawData.unit.spec.ts b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-RawData.unit.spec.ts
index d43bb01762c..f05e431e73e 100644
--- a/frontend/src/metabase/containers/DataPicker/tests/DataPicker-RawData.unit.spec.ts
+++ b/frontend/src/metabase/containers/DataPicker/tests/DataPicker-RawData.unit.spec.ts
@@ -10,11 +10,11 @@ import {
import { generateSchemaId } from "metabase-lib/metadata/utils/schema";
-import { setup, setupVirtualizedLists } from "./common";
+import { setup } from "./common";
describe("DataPicker — picking raw data", () => {
beforeAll(() => {
- setupVirtualizedLists();
+ window.HTMLElement.prototype.scrollIntoView = jest.fn();
});
afterEach(() => {
diff --git a/frontend/src/metabase/containers/DataPicker/tests/common.tsx b/frontend/src/metabase/containers/DataPicker/tests/common.tsx
index 3bb1fdb15c4..7846c37c9a9 100644
--- a/frontend/src/metabase/containers/DataPicker/tests/common.tsx
+++ b/frontend/src/metabase/containers/DataPicker/tests/common.tsx
@@ -111,21 +111,6 @@ interface SetupOpts {
hasNestedQueriesEnabled?: boolean;
}
-// react-virtualized's AutoSizer uses offsetWidth and offsetHeight.
-// Jest runs in JSDom which doesn't support measurements APIs.
-export function setupVirtualizedLists() {
- Object.defineProperty(HTMLElement.prototype, "offsetHeight", {
- configurable: true,
- value: 100,
- });
- Object.defineProperty(HTMLElement.prototype, "offsetWidth", {
- configurable: true,
- value: 100,
- });
-
- window.HTMLElement.prototype.scrollIntoView = jest.fn();
-}
-
export async function setup({
initialValue = { tableIds: [] },
filters,
--
GitLab