fix data picker another double scroll and add a visual test (#20524)

* fix data picker another double scroll and add a visual test

* review

frontend/src/metabase/core/components/AccordionList/AccordionList.jsx

 import React, { Component } from "react";
+import ReactDOM from "react-dom";
 import PropTypes from "prop-types";
 import { List, CellMeasurer, CellMeasurerCache } from "react-virtualized";
 
@@ -45,8 +46,6 @@ export default class AccordionList extends Component {
       fixedWidth: true,
       minHeight: 10,
     });
-
-    this.containerRef = React.createRef();
   }
 
   static propTypes = {
@@ -127,6 +126,8 @@ export default class AccordionList extends Component {
   };
 
   componentDidMount() {
+    this.container = ReactDOM.findDOMNode(this);
+
     // NOTE: for some reason the row heights aren't computed correctly when
     // first rendering, so force the list to update
     this._forceUpdateList();
@@ -134,11 +135,11 @@ export default class AccordionList extends Component {
     // Use list.scrollToRow instead of the scrollToIndex prop since the
     // causes the list's scrolling to be pinned to the selected row
     setTimeout(() => {
-      const hasFocusedChildren = this.containerRef?.current?.contains(
+      const hasFocusedChildren = this.container.contains(
         document.activeElement,
       );
       if (!hasFocusedChildren && this.props.hasInitialFocus) {
-        this.containerRef?.current?.focus();
+        this.container.focus();
       }
 
       const index = this._initialSelectedRowIndex;
@@ -272,7 +273,7 @@ export default class AccordionList extends Component {
 
     for (let sectionIndex = 0; sectionIndex < sections.length; sectionIndex++) {
       const section = sections[sectionIndex];
-      for (let itemIndex = 0; itemIndex < section.items.length; itemIndex++) {
+      for (let itemIndex = 0; itemIndex < section.items?.length; itemIndex++) {
         const item = section.items[itemIndex];
         if (itemIsSelected(item)) {
           return {
@@ -512,6 +513,8 @@ export default class AccordionList extends Component {
     );
   }
 
+  isVirtualized = () => this.props.maxHeight !== Infinity;
+
   canToggleSections = () => {
     const { alwaysTogglable, sections } = this.props;
     return alwaysTogglable || sections.length > 1;
@@ -539,7 +542,7 @@ export default class AccordionList extends Component {
   // Because of virtualization, focused search input can be removed which does not trigger blur event.
   // We need to restore focus on the component root container to make keyboard navigation working
   handleSearchRemoval = () => {
-    this.containerRef?.current?.focus();
+    this.container?.focus();
   };
 
   render() {
@@ -553,11 +556,10 @@ export default class AccordionList extends Component {
 
     const searchRowIndex = rows.findIndex(row => row.type === "search");
 
-    if (this.props.maxHeight === Infinity) {
+    if (!this.isVirtualized()) {
       return (
         <AccordionListRoot
           role="tree"
-          ref={this.containerRef}
           onKeyDown={this.handleKeyDown}
           tabIndex={-1}
           className={className}
@@ -603,68 +605,65 @@ export default class AccordionList extends Component {
       // HACK - Ensure the component can scroll
       // This is a temporary fix to handle cases where the parent component doesn’t pass in the correct `maxHeight`
       overflowY: "auto",
+      outline: "none",
     };
 
     return (
-      <AccordionListRoot
-        role="tree"
-        ref={this.containerRef}
-        onKeyDown={this.handleKeyDown}
-        tabIndex={-1}
-      >
-        <List
-          id={id}
-          ref={list => (this._list = list)}
-          className={className}
-          style={{ ...defaultListStyle, ...style }}
-          containerStyle={{ pointerEvents: "auto" }}
-          width={width}
-          height={height}
-          rowCount={rows.length}
-          deferredMeasurementCache={this._cache}
-          rowHeight={this._cache.rowHeight}
-          // HACK: needs to be large enough to render enough rows to fill the screen since we used
-          // the CellMeasurerCache to calculate the height
-          overscanRowCount={100}
-          scrollToIndex={scrollToIndex}
-          scrollToAlignment={scrollToAlignment}
-          rowRenderer={({ key, index, parent, style }) => {
-            return (
-              <CellMeasurer
-                cache={this._cache}
-                columnIndex={0}
-                key={key}
-                rowIndex={index}
-                parent={parent}
-              >
-                {({ measure }) => (
-                  <AccordionListCell
-                    hasCursor={this.isRowSelected(rows[index])}
-                    {...this.props}
-                    style={style}
-                    row={rows[index]}
-                    sections={sections}
-                    onChange={this.handleChange}
-                    searchText={this.state.searchText}
-                    onChangeSearchText={this.handleChangeSearchText}
-                    sectionIsExpanded={this.isSectionExpanded}
-                    canToggleSections={this.canToggleSections()}
-                    toggleSection={this.toggleSection}
-                  />
-                )}
-              </CellMeasurer>
-            );
-          }}
-          onRowsRendered={({ startIndex, stopIndex }) => {
-            this._startIndex = startIndex;
-            this._stopIndex = stopIndex;
-
-            if (searchRowIndex < startIndex || searchRowIndex > stopIndex) {
-              this.handleSearchRemoval();
-            }
-          }}
-        />
-      </AccordionListRoot>
+      <List
+        id={id}
+        ref={list => (this._list = list)}
+        className={className}
+        style={{ ...defaultListStyle, ...style }}
+        containerStyle={{ pointerEvents: "auto" }}
+        width={width}
+        height={height}
+        rowCount={rows.length}
+        deferredMeasurementCache={this._cache}
+        rowHeight={this._cache.rowHeight}
+        // HACK: needs to be large enough to render enough rows to fill the screen since we used
+        // the CellMeasurerCache to calculate the height
+        overscanRowCount={100}
+        scrollToIndex={scrollToIndex}
+        scrollToAlignment={scrollToAlignment}
+        containerProps={{
+          onKeyDown: this.handleKeyDown,
+        }}
+        rowRenderer={({ key, index, parent, style }) => {
+          return (
+            <CellMeasurer
+              cache={this._cache}
+              columnIndex={0}
+              key={key}
+              rowIndex={index}
+              parent={parent}
+            >
+              {({ measure }) => (
+                <AccordionListCell
+                  hasCursor={this.isRowSelected(rows[index])}
+                  {...this.props}
+                  style={style}
+                  row={rows[index]}
+                  sections={sections}
+                  onChange={this.handleChange}
+                  searchText={this.state.searchText}
+                  onChangeSearchText={this.handleChangeSearchText}
+                  sectionIsExpanded={this.isSectionExpanded}
+                  canToggleSections={this.canToggleSections()}
+                  toggleSection={this.toggleSection}
+                />
+              )}
+            </CellMeasurer>
+          );
+        }}
+        onRowsRendered={({ startIndex, stopIndex }) => {
+          this._startIndex = startIndex;
+          this._stopIndex = stopIndex;
+
+          if (searchRowIndex < startIndex || searchRowIndex > stopIndex) {
+            this.handleSearchRemoval();
+          }
+        }}
+      />
     );
   }
 }

frontend/test/metabase-visual/notebook/notebook.cy.spec.js

-import { restore, popover } from "__support__/e2e/cypress";
+import _ from "underscore";
+import { restore, popover, openNotebookEditor } from "__support__/e2e/cypress";
 
 describe("visual tests > notebook > major UI elements", () => {
   const VIEWPORT_WIDTH = 2500;
@@ -103,6 +104,28 @@ describe("visual tests > notebook > Run buttons", () => {
   });
 });
 
+describe("visual tests > notebook", () => {
+  const VIEWPORT_WIDTH = 1200;
+  const VIEWPORT_HEIGHT = 600;
+
+  beforeEach(() => {
+    restore();
+    cy.signInAsAdmin();
+    cy.viewport(VIEWPORT_WIDTH, VIEWPORT_HEIGHT);
+
+    _.range(10).forEach(index => {
+      const name = `Sample Database ${index + 1}`;
+      cy.addH2SampleDatabase({ name });
+    });
+  });
+
+  it("data picker", () => {
+    openNotebookEditor();
+    cy.findByText("Sample Database");
+    cy.percySnapshot();
+  });
+});
+
 function selectFromDropdown(itemName) {
   return popover().findByText(itemName);
 }