fix deselect/select on multiple select menus, optimize perf, remove b…

…roken context (#1256)

Co-authored-by: Allen Kleiner <akleiner24@gmail.com>

src/select-menu/src/OptionsList.js

@@ -1,4 +1,4 @@
-import React, { memo, useCallback, useEffect, useRef, useState } from 'react'
+import React, { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react'
 import fuzzaldrin from 'fuzzaldrin-plus'
 import PropTypes from 'prop-types'
 import VirtualList from 'react-tiny-virtual-list'
@@ -55,53 +55,43 @@ const OptionsList = memo(function OptionsList(props) {
   } = props
 
   const [searchValue, setSearchValue] = useState(defaultSearchValue)
-  const [selectedOptions, setSelectedOptions] = useState(selected)
   const [searchRef, setSearchRef] = useState(null)
   const requestId = useRef()
   const theme = useTheme()
   const { tokens } = theme
 
   const isSelected = useCallback(
     item => {
-      return Boolean(selectedOptions.find(selectedItem => selectedItem === item.value))
+      return Boolean(selected.find(selectedItem => selectedItem === item.value))
     },
-    [selectedOptions]
+    [selected]
   )
 
-  const search = useCallback(
-    options => {
-      if (searchValue.trim() === '') {
-        return options
-      }
-
-      // Preserve backwards compatibility with allowing custom filters, which accept array of strings
-      if (typeof optionsFilter === 'function') {
-        return optionsFilter(
-          options.map(item => item.label),
-          searchValue
-        ).map(name => options.find(item => item.label === name))
-      }
+  const optionLabels = useMemo(() => {
+    return originalOptions.map(item => item.label)
+  }, [originalOptions])
 
-      return fuzzyFilter(options, searchValue, { key: 'label' })
-    },
-    [optionsFilter, searchValue]
-  )
+  // Gets filtered options any time the filter fn, value, or options change
+  const options = useMemo(() => {
+    if (searchValue.trim() === '') {
+      return originalOptions
+    }
 
-  const options = search(originalOptions)
+    // Preserve backwards compatibility with allowing custom filters, which accept array of strings
+    if (typeof optionsFilter === 'function') {
+      return optionsFilter(optionLabels, searchValue).map(name => {
+        return originalOptions.find(item => item.label === name)
+      })
+    }
 
-  const getFilteredOptions = useCallback(() => {
-    return search(options)
-  }, [options])
+    return fuzzyFilter(originalOptions, searchValue, { key: 'label' })
+  }, [originalOptions, optionLabels, optionsFilter, searchValue])
 
   const getCurrentIndex = useCallback(() => {
-    const options = getFilteredOptions()
-
     return options.findIndex(option => option.value === selected[selected.length - 1])
-  }, [selected])
+  }, [selected, options])
 
   const handleArrowUp = useCallback(() => {
-    const options = getFilteredOptions()
-
     let nextIndex = getCurrentIndex() - 1
 
     if (nextIndex < 0) {
@@ -113,11 +103,9 @@ const OptionsList = memo(function OptionsList(props) {
     }
 
     onSelect(options[nextIndex])
-  }, [onSelect])
+  }, [onSelect, options, getCurrentIndex, isSelected])
 
   const handleArrowDown = useCallback(() => {
-    const options = getFilteredOptions()
-
     let nextIndex = getCurrentIndex() + 1
 
     if (nextIndex === options.length) {
@@ -127,7 +115,7 @@ const OptionsList = memo(function OptionsList(props) {
     if (!isSelected(options[nextIndex])) {
       onSelect(options[nextIndex])
     }
-  }, [onSelect])
+  }, [onSelect, options, getCurrentIndex, isSelected])
 
   const handleChange = useCallback(
     searchValue => {
@@ -149,7 +137,7 @@ const OptionsList = memo(function OptionsList(props) {
         close()
       }
     },
-    [onDeselect, isMultiSelect, closeOnSelect]
+    [onDeselect, isMultiSelect, closeOnSelect, onSelect, isSelected, close]
   )
 
   const handleEnter = useCallback(() => {
@@ -160,7 +148,7 @@ const OptionsList = memo(function OptionsList(props) {
         close()
       }
     }
-  }, [isMultiSelect, close, closeOnSelect])
+  }, [isMultiSelect, close, closeOnSelect, getCurrentIndex])
 
   const handleDeselect = useCallback(
     item => {
@@ -187,7 +175,7 @@ const OptionsList = memo(function OptionsList(props) {
         close()
       }
     },
-    [close]
+    [close, handleArrowUp, handleArrowDown, handleEnter]
   )
 
   useEffect(() => {
@@ -206,12 +194,6 @@ const OptionsList = memo(function OptionsList(props) {
     }
   }, [hasFilter, searchRef, handleKeyDown])
 
-  useEffect(() => {
-    if (selected !== selectedOptions) {
-      setSelectedOptions(selected)
-    }
-  }, [selected])
-
   const listHeight = height - (hasFilter ? 32 : 0)
   const currentIndex = getCurrentIndex()
   const scrollToIndex = currentIndex === -1 ? 0 : currentIndex

src/table/src/TableCell.js

@@ -6,7 +6,6 @@ import { Pane } from '../../layers'
 import safeInvoke from '../../lib/safe-invoke'
 import { toaster } from '../../toaster'
 import manageTableCellFocusInteraction from './manageTableCellFocusInteraction'
-import { TableRowConsumer } from './TableRowContext'
 
 function executeArrowKeyOverride(override) {
   if (!override) {
@@ -93,26 +92,19 @@ const TableCell = memo(
     )
 
     return (
-      <TableRowConsumer>
-        {height => {
-          return (
-            <Pane
-              ref={handleRef}
-              height={height}
-              className={cx(themedClassName, className)}
-              tabIndex={isSelectable ? tabIndex : undefined}
-              data-isselectable={isSelectable}
-              onClick={onClick}
-              onKeyDown={handleKeyDown}
-              {...boxProps}
-              {...rest}
-            >
-              {children}
-              {rightView || null}
-            </Pane>
-          )
-        }}
-      </TableRowConsumer>
+      <Pane
+        ref={handleRef}
+        className={cx(themedClassName, className)}
+        tabIndex={isSelectable ? tabIndex : undefined}
+        data-isselectable={isSelectable}
+        onClick={onClick}
+        onKeyDown={handleKeyDown}
+        {...boxProps}
+        {...rest}
+      >
+        {children}
+        {rightView || null}
+      </Pane>
     )
   })
 )

src/table/src/TableRow.js

@@ -5,7 +5,6 @@ import { useClickable, useLatest, useMergedRef, useStyleConfig } from '../../hoo
 import { Pane } from '../../layers'
 import safeInvoke from '../../lib/safe-invoke'
 import manageTableRowFocusInteraction from './manageTableRowFocusInteraction'
-import { TableRowProvider } from './TableRowContext'
 
 const noop = () => {}
 
@@ -99,24 +98,22 @@ const TableRow = memo(
     const height = rest.height || themeHeight
 
     return (
-      <TableRowProvider height={height}>
-        <Pane
-          ref={onRef}
-          className={cx(themedClassName, className)}
-          aria-selected={isHighlighted}
-          aria-current={isSelected}
-          data-isselectable={isSelectable}
-          tabIndex={isSelectable ? clickable.tabIndex : undefined}
-          onClick={handleClick}
-          onKeyDown={clickable.onKeyDown}
-          borderBottom="muted"
-          height={height}
-          {...boxProps}
-          {...rest}
-        >
-          {children}
-        </Pane>
-      </TableRowProvider>
+      <Pane
+        ref={onRef}
+        className={cx(themedClassName, className)}
+        aria-selected={isHighlighted}
+        aria-current={isSelected}
+        data-isselectable={isSelectable}
+        tabIndex={isSelectable ? clickable.tabIndex : undefined}
+        onClick={handleClick}
+        onKeyDown={clickable.onKeyDown}
+        borderBottom="muted"
+        height={height}
+        {...boxProps}
+        {...rest}
+      >
+        {children}
+      </Pane>
     )
   })
 )