virtualize select content (metabase#6255)

* virtualize select content * clean up example * show examples
flyforfreedom · Oct 25, 2017 · 7d178fd · 7d178fd
1 parent df0ae90
commit 7d178fd
Show file tree

Hide file tree

Showing 7 changed files with 169 additions and 24 deletions.
diff --git a/frontend/src/metabase/components/Select.info.js b/frontend/src/metabase/components/Select.info.js
@@ -0,0 +1,33 @@
+import React from 'react'
+
+import Select, { Option } from 'metabase/components/Select'
+
+export const component = Select
+
+const fixture = [
+    { name: 'Blue' },
+    { name: 'Green' },
+    { name: 'Red' },
+    { name: 'Yellow' },
+]
+
+export const description = `
+    A component used to make a selection
+`
+
+export const examples = {
+    'Default': (
+        <Select onChange={() => alert('Selected')}>
+            { fixture.map(f => <Option name={f.name}>{f.name}</Option>)}
+        </Select>
+    ),
+    'With search': (
+        <Select
+            searchProp='name'
+            onChange={() => alert('Selected')}
+        >
+            { fixture.map(f => <Option name={f.name}>{f.name}</Option>)}
+        </Select>
+    ),
+}
+
diff --git a/frontend/src/metabase/components/Select.jsx b/frontend/src/metabase/components/Select.jsx
@@ -2,6 +2,9 @@
 import React, { Component } from "react";
 import PropTypes from "prop-types";
 
+import { List } from 'react-virtualized'
+import "react-virtualized/styles.css";
+
 import ColumnarSelector from "metabase/components/ColumnarSelector.jsx";
 import Icon from "metabase/components/Icon.jsx";
 import PopoverWithTrigger from "metabase/components/PopoverWithTrigger.jsx";
@@ -23,25 +26,34 @@ export default class Select extends Component {
 }
 
 class BrowserSelect extends Component {
-    constructor(props, context) {
-        super(props, context);
-        this.state = {
-            inputValue: ""
-        };
+
+    state = {
+        inputValue: ""
     }
+
     static propTypes = {
         children: PropTypes.array.isRequired,
-        className: PropTypes.string,
-        value: PropTypes.any,
         onChange: PropTypes.func.isRequired,
+        value: PropTypes.any,
         searchProp: PropTypes.string,
         searchCaseInsensitive: PropTypes.bool,
         isInitiallyOpen: PropTypes.bool,
         placeholder: PropTypes.string,
-        triggerElement: PropTypes.any
+        // NOTE - @kdoh
+        // seems too generic for us?
+        triggerElement: PropTypes.any,
+        height: PropTypes.number,
+        width: PropTypes.number,
+        rowHeight: PropTypes.number,
+        // TODO - @kdoh
+        // we should not allow this
+        className: PropTypes.string,
     }
     static defaultProps = {
         className: "",
+        width: 320,
+        height: 320,
+        rowHeight: 40,
     }
 
     isSelected(otherValue) {
@@ -52,16 +64,20 @@ class BrowserSelect extends Component {
     render() {
         const {
             className,
-            children,
             value,
             onChange,
             searchProp,
             searchCaseInsensitive,
             isInitiallyOpen,
             placeholder,
-            triggerElement
+            triggerElement,
+            width,
+            height,
+            rowHeight
         } = this.props;
 
+        let children = this.props.children
+
         let selectedName;
         for (const child of children) {
             if (this.isSelected(child.props.value)) {
@@ -87,6 +103,9 @@ class BrowserSelect extends Component {
             }
         }
 
+        // make sure we filter by the search query
+        children = children.filter(filter)
+
         return (
             <PopoverWithTrigger
                 ref="popover"
@@ -105,19 +124,34 @@ class BrowserSelect extends Component {
                             autoFocus
                         />
                     }
-                    <div className="ColumnarSelector-column scroll-y" onClick={(e) => e.stopPropagation()}>
-                        {children.filter(filter).map(child =>
-                            React.cloneElement(child, {
-                                selected: this.isSelected(child.props.value),
-                                onClick: () => {
-                                    if (!child.props.disabled) {
-                                        onChange({ target: { value: child.props.value }});
-                                    }
-                                    this.refs.popover.close()
-                                }
-                            })
-                        )}
-                    </div>
+                    <List
+                        width={width}
+                        height={height}
+                        rowHeight={rowHeight}
+                        rowCount={children.length}
+                        rowRenderer={({index, key, style}) => {
+                            const child = children[index]
+
+                            /*
+                             * for each child we need to add props based on
+                             * the parent's onClick and the current selection
+                             * status, so we use cloneElement here
+                            * */
+                            return (
+                                <div key={key} style={style} onClick={e => e.stopPropagation()}>
+                                    {React.cloneElement(children[index], {
+                                        selected: this.isSelected(child.props.value),
+                                        onClick: () => {
+                                            if (!child.props.disabled) {
+                                                onChange({ target: { value: child.props.value }});
+                                            }
+                                            this.refs.popover.close()
+                                        }
+                                    })}
+                                </div>
+                            )
+                        }}
+                    />
                 </div>
             </PopoverWithTrigger>
         );

diff --git a/frontend/src/metabase/internal/lib/components-webpack.js b/frontend/src/metabase/internal/lib/components-webpack.js
@@ -5,4 +5,4 @@ const req = require.context(
     /^(.*\.info\.(js$))[^.]*$/im
 );
 
-export default req.keys().map(key => req(key));
+export default req.keys().map(key => Object.assign({}, req(key), { showExample: true }));
diff --git a/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.jsx b/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.jsx
@@ -116,6 +116,7 @@ export default class TagEditorParam extends Component {
                         onChange={(e) => this.setType(e.target.value)}
                         isInitiallyOpen={!tag.type}
                         placeholder="Select…"
+                        height={300}
                     >
                         <Option value="text">Text</Option>
                         <Option value="number">Number</Option>
@@ -135,6 +136,8 @@ export default class TagEditorParam extends Component {
                             searchCaseInsensitive
                             isInitiallyOpen={!tag.dimension}
                             placeholder="Select…"
+                            rowHeight={60}
+                            width={280}
                         >
                             {databaseFields && databaseFields.map(field =>
                                 <Option key={field.id} value={field.id} name={field.name}>

diff --git a/frontend/test/internal/__snapshots__/components.unit.spec.js.snap b/frontend/test/internal/__snapshots__/components.unit.spec.js.snap
@@ -436,6 +436,76 @@ exports[`EntityMenu should render "Share menu" correctly 1`] = `
 </div>
 `;
 
+exports[`Select should render "Default" correctly 1`] = `
+<a
+  className="no-decoration"
+  id={undefined}
+  onClick={[Function]}
+  style={undefined}
+>
+  <div
+    className="AdminSelect flex align-center  text-grey-3"
+  >
+    <span
+      className="AdminSelect-content mr1"
+    >
+      Yellow
+    </span>
+    <svg
+      className="Icon Icon-chevrondown AdminSelect-chevron flex-align-right"
+      fill="currentcolor"
+      height={12}
+      name="chevrondown"
+      size={12}
+      viewBox="0 0 32 32"
+      width={12}
+    >
+      <path
+        d="M1 12 L16 26 L31 12 L27 8 L16 18 L5 8 z "
+      />
+    </svg>
+  </div>
+  <span
+    className="hide"
+  />
+</a>
+`;
+
+exports[`Select should render "With search" correctly 1`] = `
+<a
+  className="no-decoration"
+  id={undefined}
+  onClick={[Function]}
+  style={undefined}
+>
+  <div
+    className="AdminSelect flex align-center  text-grey-3"
+  >
+    <span
+      className="AdminSelect-content mr1"
+    >
+      Yellow
+    </span>
+    <svg
+      className="Icon Icon-chevrondown AdminSelect-chevron flex-align-right"
+      fill="currentcolor"
+      height={12}
+      name="chevrondown"
+      size={12}
+      viewBox="0 0 32 32"
+      width={12}
+    >
+      <path
+        d="M1 12 L16 26 L31 12 L27 8 L16 18 L5 8 z "
+      />
+    </svg>
+  </div>
+  <span
+    className="hide"
+  />
+</a>
+`;
+
 exports[`StackedCheckBox should render "Checked with color" correctly 1`] = `
 <div
   className="relative"

diff --git a/package.json b/package.json
@@ -115,6 +115,7 @@
     "html-webpack-plugin": "^2.30.1",
     "image-diff": "^1.6.3",
     "imports-loader": "^0.7.0",
+    "insightful": "^1.1.0",
     "jasmine": "^2.4.1",
     "jasmine-core": "^2.4.1",
     "jasmine-promises": "^0.4.1",

diff --git a/yarn.lock b/yarn.lock
@@ -4600,6 +4600,10 @@ insert-css@^0.2.0:
   version "0.2.0"
   resolved "https://registry.yarnpkg.com/insert-css/-/insert-css-0.2.0.tgz#d15789971662d9899c28977fb6220d5381d2451a"
 
+insightful@^1.1.0:
+  version "1.1.0"
+  resolved "https://registry.yarnpkg.com/insightful/-/insightful-1.1.0.tgz#01afb2a62b49a6324bd5279618cd108e445037eb"
+
 [email protected]:
   version "1.2.0"
   resolved "https://registry.yarnpkg.com/internal-ip/-/internal-ip-1.2.0.tgz#ae9fbf93b984878785d50a8de1b356956058cf5c"