Browse Source

ref(js): Split out `List` from search component (#34207)

Doing this to help with the Autocomplete performance refactor
Evan Purkhiser 2 years ago
parent
commit
0d552fcc10

+ 13 - 16
static/app/components/helpSearch.tsx

@@ -1,4 +1,4 @@
-import * as React from 'react';
+import {Fragment} from 'react';
 import styled from '@emotion/styled';
 
 import {Search} from 'sentry/components/search';
@@ -9,17 +9,9 @@ import {IconWindow} from 'sentry/icons';
 import {t, tn} from 'sentry/locale';
 import space from 'sentry/styles/space';
 
-type HelpResult = Parameters<
-  React.ComponentProps<typeof HelpSource>['children']
->[0]['results'][0];
+type ItemRenderer = React.ComponentProps<typeof Search>['renderItem'];
 
-type ResultItemProps = HelpResult & {
-  highlighted: boolean;
-  // TODO(ts): Improve types when we've typed more of the search components
-  itemProps: any;
-};
-
-const renderResult = ({item, matches, itemProps, highlighted}: ResultItemProps) => {
+const renderResult: ItemRenderer = ({item, matches, itemProps, highlighted}) => {
   const sectionHeading =
     item.sectionHeading !== undefined ? (
       <SectionHeading>
@@ -31,25 +23,30 @@ const renderResult = ({item, matches, itemProps, highlighted}: ResultItemProps)
 
   if (item.empty) {
     return (
-      <React.Fragment>
+      <Fragment>
         {sectionHeading}
         <Empty>{t('No results from %s', item.sectionHeading)}</Empty>
-      </React.Fragment>
+      </Fragment>
     );
   }
 
   return (
-    <React.Fragment>
+    <Fragment>
       {sectionHeading}
       <SearchResultWrapper {...itemProps} highlighted={highlighted}>
         <SearchResult highlighted={highlighted} item={item} matches={matches} />
       </SearchResultWrapper>
-    </React.Fragment>
+    </Fragment>
   );
 };
 
+type Props = Omit<
+  React.ComponentProps<typeof Search>,
+  'sources' | 'minSearch' | 'closeOnSelect' | 'renderItem'
+>;
+
 // TODO(ts): Type based on Search props once that has types
-const HelpSearch = props => (
+const HelpSearch = (props: Props) => (
   <Search
     {...props}
     sources={[HelpSource]}

+ 1 - 1
static/app/components/modals/commandPalette.tsx

@@ -22,7 +22,7 @@ function CommandPalette({Body}: ModalRenderProps) {
             entryPoint="command_palette"
             minSearch={1}
             maxResults={10}
-            dropdownStyle={injectedCss`
+            dropdownClassName={injectedCss`
                 width: 100%;
                 border: transparent;
                 border-top-left-radius: 0;

+ 2 - 5
static/app/components/modals/helpSearchModal.tsx

@@ -30,7 +30,7 @@ function HelpSearchModal({
           <HelpSearch
             {...props}
             entryPoint="sidebar_help"
-            dropdownStyle={injectedCss`
+            dropdownClassName={injectedCss`
                 width: 100%;
                 border: transparent;
                 border-top-left-radius: 0;
@@ -41,10 +41,7 @@ function HelpSearchModal({
               `}
             renderInput={({getInputProps}) => (
               <InputWrapper>
-                <Input
-                  autoFocus
-                  {...getInputProps({type: 'text', label: placeholder, placeholder})}
-                />
+                <Input autoFocus {...getInputProps({type: 'text', placeholder})} />
               </InputWrapper>
             )}
             resultFooter={

+ 35 - 113
static/app/components/search/index.tsx

@@ -1,4 +1,4 @@
-import * as React from 'react';
+import {useCallback, useEffect, useMemo} from 'react';
 import {withRouter, WithRouterProps} from 'react-router';
 import styled from '@emotion/styled';
 import debounce from 'lodash/debounce';
@@ -6,55 +6,30 @@ import debounce from 'lodash/debounce';
 import {addErrorMessage} from 'sentry/actionCreators/indicator';
 import {navigateTo} from 'sentry/actionCreators/navigation';
 import AutoComplete from 'sentry/components/autoComplete';
-import LoadingIndicator from 'sentry/components/loadingIndicator';
-import SearchResult from 'sentry/components/search/searchResult';
-import SearchResultWrapper from 'sentry/components/search/searchResultWrapper';
 import SearchSources from 'sentry/components/search/sources';
 import ApiSource from 'sentry/components/search/sources/apiSource';
 import CommandSource from 'sentry/components/search/sources/commandSource';
 import FormSource from 'sentry/components/search/sources/formSource';
 import RouteSource from 'sentry/components/search/sources/routeSource';
 import {t} from 'sentry/locale';
-import space from 'sentry/styles/space';
 import trackAdvancedAnalyticsEvent from 'sentry/utils/analytics/trackAdvancedAnalyticsEvent';
 import type {Fuse} from 'sentry/utils/fuzzySearch';
 import replaceRouterParams from 'sentry/utils/replaceRouterParams';
 
 import {Result} from './sources/types';
+import List from './list';
 
-interface InputProps
-  extends Pick<
-    Parameters<AutoComplete<Result['item']>['props']['children']>[0],
-    'getInputProps'
-  > {}
+type AutoCompleteOpts = Parameters<AutoComplete<Result['item']>['props']['children']>[0];
 
-/**
- * Render prop for search results
- *
- * Args: {
- *  item: Search Item
- *  index: item's index in results
- *  highlighted: is item highlighted
- *  itemProps: props that should be spread for root item
- * }
- */
-interface ItemProps {
-  highlighted: boolean;
-  index: number;
-  item: Result['item'];
-  itemProps: React.ComponentProps<typeof SearchResultWrapper>;
-  matches: Result['matches'];
-}
+type ListProps = React.ComponentProps<typeof List>;
+
+interface InputProps extends Pick<AutoCompleteOpts, 'getInputProps'> {}
 
 interface SearchProps extends WithRouterProps<{orgId: string}> {
   /**
    * For analytics
    */
   entryPoint: 'settings_search' | 'command_palette' | 'sidebar_help';
-  /**
-   * Maximum number of results to display
-   */
-  maxResults: number;
   /**
    * Minimum number of characters before search activates
    */
@@ -63,7 +38,6 @@ interface SearchProps extends WithRouterProps<{orgId: string}> {
    * Render prop for the main input for the search
    */
   renderInput: (props: InputProps) => React.ReactNode;
-
   /**
    * Passed to the underlying AutoComplete component
    */
@@ -71,15 +45,19 @@ interface SearchProps extends WithRouterProps<{orgId: string}> {
   /**
    * Additional CSS for the dropdown menu.
    */
-  dropdownStyle?: string;
+  dropdownClassName?: string;
   /**
-   * Render an item in the search results.
+   * Maximum number of results to display
+   */
+  maxResults?: number;
+  /**
+   * Renders the result item
    */
-  renderItem?: (props: ItemProps) => React.ReactElement;
+  renderItem?: ListProps['renderItem'];
   /**
    * Adds a footer below the results when the search is complete
    */
-  resultFooter?: React.ReactElement;
+  resultFooter?: React.ReactNode;
   /**
    * Fuse search options
    */
@@ -87,38 +65,31 @@ interface SearchProps extends WithRouterProps<{orgId: string}> {
   /**
    * The sources to query
    */
-  sources?: React.ComponentType[];
-}
-
-function defaultItemRenderer({item, matches, itemProps, highlighted}: ItemProps) {
-  return (
-    <SearchResultWrapper {...itemProps} highlighted={highlighted}>
-      <SearchResult highlighted={highlighted} item={item} matches={matches} />
-    </SearchResultWrapper>
-  );
+  // TODO(ts): Improve any type here
+  sources?: React.ComponentType<any>[];
 }
 
 function Search({
-  renderItem = defaultItemRenderer,
   entryPoint,
   maxResults,
   minSearch,
   renderInput,
+  renderItem,
   closeOnSelect,
-  dropdownStyle,
+  dropdownClassName,
   resultFooter,
   searchOptions,
   sources,
   router,
   params,
 }: SearchProps): React.ReactElement {
-  React.useEffect(() => {
+  useEffect(() => {
     trackAdvancedAnalyticsEvent(`${entryPoint}.open`, {
       organization: null,
     });
   }, [entryPoint]);
 
-  const handleSelectItem = React.useCallback(
+  const handleSelectItem = useCallback(
     (item: Result['item'], state?: AutoComplete<Result['item']>['state']) => {
       if (!item) {
         return;
@@ -164,7 +135,7 @@ function Search({
     [entryPoint, router, params]
   );
 
-  const saveQueryMetrics = React.useCallback(
+  const saveQueryMetrics = useCallback(
     (query: string) => {
       if (!query) {
         return;
@@ -178,7 +149,7 @@ function Search({
     [entryPoint]
   );
 
-  const debouncedSaveQueryMetrics = React.useMemo(() => {
+  const debouncedSaveQueryMetrics = useMemo(() => {
     return debounce(saveQueryMetrics, 200);
   }, [entryPoint, saveQueryMetrics]);
 
@@ -188,7 +159,7 @@ function Search({
       onSelect={handleSelectItem}
       closeOnSelect={closeOnSelect ?? true}
     >
-      {({getInputProps, getItemProps, isOpen, inputValue, highlightedIndex}) => {
+      {({getInputProps, isOpen, inputValue, ...autocompleteProps}) => {
         const searchQuery = inputValue.toLowerCase().trim();
         const isValidSearch = inputValue.length >= minSearch;
 
@@ -214,34 +185,18 @@ function Search({
                 }
               >
                 {({isLoading, results, hasAnyResults}) => (
-                  <DropdownBox className={dropdownStyle}>
-                    {isLoading ? (
-                      <LoadingWrapper>
-                        <LoadingIndicator mini hideMessage relative />
-                      </LoadingWrapper>
-                    ) : !hasAnyResults ? (
-                      <EmptyItem>{t('No results found')}</EmptyItem>
-                    ) : (
-                      results.slice(0, maxResults).map((resultObj, index) => {
-                        return React.cloneElement(
-                          renderItem({
-                            index,
-                            item: resultObj.item,
-                            matches: resultObj.matches,
-                            highlighted: index === highlightedIndex,
-                            itemProps: getItemProps({
-                              item: resultObj.item,
-                              index,
-                            }),
-                          }),
-                          {key: `${resultObj.item.title}-${index}`}
-                        );
-                      })
-                    )}
-                    {!isLoading && resultFooter ? (
-                      <ResultFooter>{resultFooter}</ResultFooter>
-                    ) : null}
-                  </DropdownBox>
+                  <List
+                    {...{
+                      isLoading,
+                      results,
+                      hasAnyResults,
+                      maxResults,
+                      resultFooter,
+                      dropdownClassName,
+                      renderItem,
+                      ...autocompleteProps,
+                    }}
+                  />
                 )}
               </SearchSources>
             ) : null}
@@ -255,39 +210,6 @@ function Search({
 const WithRouterSearch = withRouter(Search);
 export {WithRouterSearch as Search, SearchProps};
 
-const DropdownBox = styled('div')`
-  background: ${p => p.theme.background};
-  border: 1px solid ${p => p.theme.border};
-  box-shadow: ${p => p.theme.dropShadowHeavy};
-  position: absolute;
-  top: 36px;
-  right: 0;
-  width: 400px;
-  border-radius: 5px;
-  overflow: auto;
-  max-height: 60vh;
-`;
-
 const SearchWrapper = styled('div')`
   position: relative;
 `;
-
-const ResultFooter = styled('div')`
-  position: sticky;
-  bottom: 0;
-  left: 0;
-  right: 0;
-`;
-
-const EmptyItem = styled(SearchResultWrapper)`
-  text-align: center;
-  padding: 16px;
-  opacity: 0.5;
-`;
-
-const LoadingWrapper = styled('div')`
-  display: flex;
-  justify-content: center;
-  align-items: center;
-  padding: ${space(1)};
-`;

+ 118 - 0
static/app/components/search/list.tsx

@@ -0,0 +1,118 @@
+import {Fragment} from 'react';
+import styled from '@emotion/styled';
+
+import AutoComplete from 'sentry/components/autoComplete';
+import LoadingIndicator from 'sentry/components/loadingIndicator';
+import {t} from 'sentry/locale';
+import space from 'sentry/styles/space';
+
+import {Result} from './sources/types';
+import SearchResult from './searchResult';
+import SearchResultWrapper from './searchResultWrapper';
+
+type AutoCompleteOpts = Parameters<AutoComplete<Result['item']>['props']['children']>[0];
+
+interface RenderItemProps {
+  highlighted: boolean;
+  item: Result['item'];
+  itemProps: ReturnType<AutoCompleteOpts['getItemProps']>;
+  matches: Result['matches'];
+}
+
+type Props = {
+  getItemProps: AutoCompleteOpts['getItemProps'];
+  hasAnyResults: boolean;
+  highlightedIndex: number;
+  isLoading: boolean;
+  resultFooter: React.ReactNode;
+  results: Result[];
+  dropdownClassName?: string;
+  maxResults?: number;
+  renderItem?: (props: RenderItemProps) => React.ReactNode;
+};
+
+function defaultItemRenderer({item, highlighted, itemProps, matches}: RenderItemProps) {
+  return (
+    <SearchResultWrapper highlighted={highlighted} {...itemProps}>
+      <SearchResult highlighted={highlighted} item={item} matches={matches} />
+    </SearchResultWrapper>
+  );
+}
+
+function List({
+  dropdownClassName,
+  isLoading,
+  hasAnyResults,
+  results,
+  maxResults,
+  getItemProps,
+  highlightedIndex,
+  resultFooter,
+  renderItem = defaultItemRenderer,
+}: Props) {
+  const resultList = results.slice(0, maxResults);
+
+  return (
+    <DropdownBox className={dropdownClassName}>
+      {isLoading ? (
+        <LoadingWrapper>
+          <LoadingIndicator mini hideMessage relative />
+        </LoadingWrapper>
+      ) : !hasAnyResults ? (
+        <EmptyItem>{t('No results found')}</EmptyItem>
+      ) : (
+        resultList.map((result, index) => {
+          const {item, matches, refIndex} = result;
+          const highlighted = index === highlightedIndex;
+
+          const itemProps = getItemProps({
+            item: result.item,
+            index,
+          });
+
+          return (
+            <Fragment key={`${index}-${refIndex}`}>
+              {renderItem({highlighted, itemProps, item, matches})}
+            </Fragment>
+          );
+        })
+      )}
+      {!isLoading && resultFooter ? <ResultFooter>{resultFooter}</ResultFooter> : null}
+    </DropdownBox>
+  );
+}
+
+export default List;
+
+const DropdownBox = styled('div')`
+  background: ${p => p.theme.background};
+  border: 1px solid ${p => p.theme.border};
+  box-shadow: ${p => p.theme.dropShadowHeavy};
+  position: absolute;
+  top: 36px;
+  right: 0;
+  width: 400px;
+  border-radius: 5px;
+  overflow: auto;
+  max-height: 60vh;
+`;
+
+const ResultFooter = styled('div')`
+  position: sticky;
+  bottom: 0;
+  left: 0;
+  right: 0;
+`;
+
+const EmptyItem = styled(SearchResultWrapper)`
+  text-align: center;
+  padding: 16px;
+  opacity: 0.5;
+`;
+
+const LoadingWrapper = styled('div')`
+  display: flex;
+  justify-content: center;
+  align-items: center;
+  padding: ${space(1)};
+`;

+ 5 - 4
static/app/components/search/searchResultWrapper.tsx

@@ -6,11 +6,12 @@ interface Props extends React.HTMLAttributes<HTMLDivElement> {
   highlighted?: boolean;
 }
 
+function scrollIntoView(element: HTMLDivElement) {
+  element?.scrollIntoView?.(true);
+}
+
 const SearchResultWrapper = styled(({highlighted, ...props}: Props) => (
-  <div
-    {...props}
-    ref={element => highlighted && element?.scrollIntoView?.({block: 'nearest'})}
-  />
+  <div {...props} ref={highlighted ? scrollIntoView : undefined} />
 ))`
   cursor: pointer;
   display: block;

+ 3 - 1
tests/acceptance/test_sidebar.py

@@ -36,7 +36,9 @@ class SidebarTest(AcceptanceTestCase):
         self.browser.click('[data-test-id="help-sidebar"]')
         self.browser.wait_until_test_id("search-docs-and-faqs")
         self.browser.click('[data-test-id="search-docs-and-faqs"]')
-        self.browser.wait_until('input[label="Search for documentation, FAQs, blog posts..."]')
+        self.browser.wait_until(
+            'input[placeholder="Search for documentation, FAQs, blog posts..."]'
+        )
 
     def test_sandbox_sidebar(self):
         user = self.create_user("another@example.com")