From 14854fe3aa11ed15f9c421a83f63dd9d956536e9 Mon Sep 17 00:00:00 2001
From: Anton Kulyk <kuliks.anton@gmail.com>
Date: Tue, 10 Jan 2023 12:43:10 +0000
Subject: [PATCH] Virtualize table and card lists in data picker (#27582)

* Convert `VirtualizedList` to TypeScript

* Add `VirtualizedSelectList` component

* Virtualize raw table list

* Virtualize model and question list

* Fix rendering virtualized lists in tests
---
 ...irtualizedList.jsx => VirtualizedList.tsx} | 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, 95 insertions(+), 33 deletions(-)
 rename frontend/src/metabase/components/{VirtualizedList.jsx => VirtualizedList.tsx} (69%)
 create mode 100644 frontend/src/metabase/containers/DataPicker/VirtualizedSelectList.tsx

diff --git a/frontend/src/metabase/components/VirtualizedList.jsx b/frontend/src/metabase/components/VirtualizedList.tsx
similarity index 69%
rename from frontend/src/metabase/components/VirtualizedList.jsx
rename to frontend/src/metabase/components/VirtualizedList.tsx
index 7f8104e36f0..acd498d2a2e 100644
--- a/frontend/src/metabase/components/VirtualizedList.jsx
+++ b/frontend/src/metabase/components/VirtualizedList.tsx
@@ -1,8 +1,20 @@
-/* eslint-disable react/prop-types */
 import React from "react";
 import { List, WindowScroller, AutoSizer } from "react-virtualized";
 
-function VirtualizedList({ items, rowHeight, renderItem, scrollElement }) {
+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>) {
   const rowRenderer = React.useCallback(
     ({ index, key, style }) => (
       <div key={key} style={style}>
@@ -15,7 +27,7 @@ function VirtualizedList({ items, rowHeight, renderItem, scrollElement }) {
   const renderScrollComponent = React.useCallback(
     ({ width }) => {
       return (
-        <WindowScroller scrollElement={scrollElement}>
+        <WindowScroller scrollElement={scrollElement || undefined}>
           {({ 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 5bb3ffb3db8..4c5897c86f1 100644
--- a/frontend/src/metabase/containers/DataPicker/CardPicker/CardPicker.styled.tsx
+++ b/frontend/src/metabase/containers/DataPicker/CardPicker/CardPicker.styled.tsx
@@ -1,8 +1,6 @@
 import styled from "@emotion/styled";
 
-import SelectList from "metabase/components/SelectList";
-
-export const StyledSelectList = styled(SelectList)`
+export const ListContainer = styled.div`
   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 b301fd7e472..cb7a60925e7 100644
--- a/frontend/src/metabase/containers/DataPicker/CardPicker/CardPickerView.tsx
+++ b/frontend/src/metabase/containers/DataPicker/CardPicker/CardPickerView.tsx
@@ -1,8 +1,6 @@
 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";
@@ -13,9 +11,10 @@ import type { DataPickerSelectedItem, VirtualTable } from "../types";
 
 import EmptyState from "../EmptyState";
 import LoadingState from "../LoadingState";
+import VirtualizedSelectList from "../VirtualizedSelectList";
 import PanePicker from "../PanePicker";
 
-import { StyledSelectList } from "./CardPicker.styled";
+import { ListContainer } from "./CardPicker.styled";
 
 type TargetModel = "model" | "question";
 
@@ -55,7 +54,7 @@ function TableSelectListItem({
   onSelect: (id: Table["id"]) => void;
 }) {
   return (
-    <SelectList.Item
+    <VirtualizedSelectList.Item
       id={table.id}
       name={table.display_name}
       isSelected={isSelected}
@@ -63,7 +62,7 @@ function TableSelectListItem({
       onSelect={onSelect}
     >
       {table.display_name}
-    </SelectList.Item>
+    </VirtualizedSelectList.Item>
   );
 }
 
@@ -104,7 +103,7 @@ function CardPickerView({
   );
 
   const renderVirtualTable = useCallback(
-    (table: VirtualTable) => (
+    ({ item: table }: { item: VirtualTable }) => (
       <TableSelectListItem
         key={table.id}
         table={table}
@@ -130,9 +129,14 @@ function CardPickerView({
       ) : isEmpty ? (
         <EmptyState />
       ) : (
-        <StyledSelectList>
-          {virtualTables?.map?.(renderVirtualTable)}
-        </StyledSelectList>
+        <ListContainer>
+          {Array.isArray(virtualTables) && (
+            <VirtualizedSelectList<VirtualTable>
+              items={virtualTables}
+              renderItem={renderVirtualTable}
+            />
+          )}
+        </ListContainer>
       )}
     </PanePicker>
   );
diff --git a/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPicker.styled.tsx b/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPicker.styled.tsx
index 98464679d16..fb0f876d871 100644
--- a/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPicker.styled.tsx
+++ b/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPicker.styled.tsx
@@ -1,8 +1,6 @@
 import styled from "@emotion/styled";
 
-import SelectList from "metabase/components/SelectList";
-
-export const StyledSelectList = styled(SelectList)`
+export const ListContainer = styled.div`
   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 cd741fc35e5..c741e5d92de 100644
--- a/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPickerView.tsx
+++ b/frontend/src/metabase/containers/DataPicker/RawDataPicker/RawDataPickerView.tsx
@@ -1,8 +1,6 @@
 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";
@@ -13,8 +11,9 @@ import type { DataPickerSelectedItem } from "../types";
 
 import EmptyState from "../EmptyState";
 import LoadingState from "../LoadingState";
+import VirtualizedSelectList from "../VirtualizedSelectList";
 import PanePicker from "../PanePicker";
-import { StyledSelectList } from "./RawDataPicker.styled";
+import { ListContainer } from "./RawDataPicker.styled";
 
 interface RawDataPickerViewProps {
   databases: Database[];
@@ -61,7 +60,7 @@ function TableSelectListItem({
 }) {
   const name = table.displayName();
   return (
-    <SelectList.Item
+    <VirtualizedSelectList.Item
       id={table.id}
       name={name}
       isSelected={isSelected}
@@ -69,7 +68,7 @@ function TableSelectListItem({
       onSelect={onSelect}
     >
       {name}
-    </SelectList.Item>
+    </VirtualizedSelectList.Item>
   );
 }
 
@@ -135,9 +134,8 @@ function RawDataPickerView({
   );
 
   const renderTable = useCallback(
-    (table: Table) => (
+    ({ item: table }: { item: Table }) => (
       <TableSelectListItem
-        key={table.id}
         table={table}
         isSelected={selectedTableIds.includes(table.id)}
         onSelect={onSelectedTable}
@@ -162,7 +160,14 @@ function RawDataPickerView({
       ) : isEmpty ? (
         <EmptyState />
       ) : (
-        <StyledSelectList>{tables?.map?.(renderTable)}</StyledSelectList>
+        <ListContainer>
+          {Array.isArray(tables) && (
+            <VirtualizedSelectList<Table>
+              items={tables}
+              renderItem={renderTable}
+            />
+          )}
+        </ListContainer>
       )}
     </PanePicker>
   );
diff --git a/frontend/src/metabase/containers/DataPicker/VirtualizedSelectList.tsx b/frontend/src/metabase/containers/DataPicker/VirtualizedSelectList.tsx
new file mode 100644
index 00000000000..7ad52248e1d
--- /dev/null
+++ b/frontend/src/metabase/containers/DataPicker/VirtualizedSelectList.tsx
@@ -0,0 +1,28 @@
+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 d2c6dc25d66..eb638ade81c 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, waitFor } from "__support__/ui";
+import { screen } from "__support__/ui";
 import { SAMPLE_DATABASE } from "__support__/sample_database_fixture";
 
-import { setup } from "./common";
+import { setup, setupVirtualizedLists } from "./common";
 
 describe("DataPicker", () => {
   beforeAll(() => {
-    window.HTMLElement.prototype.scrollIntoView = jest.fn();
+    setupVirtualizedLists();
   });
 
   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 44ac210af3e..a1a7c36d4c1 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,6 +13,7 @@ import {
 
 import {
   setup,
+  setupVirtualizedLists,
   EMPTY_COLLECTION,
   SAMPLE_COLLECTION,
   SAMPLE_MODEL,
@@ -27,7 +28,7 @@ const ROOT_COLLECTION_MODEL_VIRTUAL_SCHEMA_ID = getCollectionVirtualSchemaId(
 
 describe("DataPicker — picking models", () => {
   beforeAll(() => {
-    window.HTMLElement.prototype.scrollIntoView = jest.fn();
+    setupVirtualizedLists();
   });
 
   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 320c707e4a1..5d38a0b09a8 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,6 +13,7 @@ import {
 
 import {
   setup,
+  setupVirtualizedLists,
   EMPTY_COLLECTION,
   SAMPLE_COLLECTION,
   SAMPLE_QUESTION,
@@ -25,7 +26,7 @@ const ROOT_COLLECTION_QUESTIONS_VIRTUAL_SCHEMA_ID =
 
 describe("DataPicker — picking questions", () => {
   beforeAll(() => {
-    window.HTMLElement.prototype.scrollIntoView = jest.fn();
+    setupVirtualizedLists();
   });
 
   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 ee9c06872a0..db37ba4d100 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 } from "./common";
+import { setup, setupVirtualizedLists } from "./common";
 
 describe("DataPicker — picking raw data", () => {
   beforeAll(() => {
-    window.HTMLElement.prototype.scrollIntoView = jest.fn();
+    setupVirtualizedLists();
   });
 
   afterEach(() => {
diff --git a/frontend/src/metabase/containers/DataPicker/tests/common.tsx b/frontend/src/metabase/containers/DataPicker/tests/common.tsx
index 7846c37c9a9..3bb1fdb15c4 100644
--- a/frontend/src/metabase/containers/DataPicker/tests/common.tsx
+++ b/frontend/src/metabase/containers/DataPicker/tests/common.tsx
@@ -111,6 +111,21 @@ 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