fix: using search hotkey enter navigates to the wrong result (#6735)

* fix: fix SearchBarDropdown selecting invalid result on enter after initial search when recent searches are filled

* add test

* Update cypress/e2e/universal-search.test.ts

Co-authored-by: Zach Pomerantz <zzmp@uniswap.org>

* Update cypress/e2e/universal-search.test.ts

Co-authored-by: Zach Pomerantz <zzmp@uniswap.org>

* Update cypress/e2e/universal-search.test.ts

Co-authored-by: Zach Pomerantz <zzmp@uniswap.org>

* use searhc hotkey

* Revert ""

This reverts commit 7b04d5d575e8f776e8127e9b43c3fff6f74d8919.

* chore: gitignore cypress/downloads.html

---------

Co-authored-by: Zach Pomerantz <zzmp@uniswap.org>

.gitignore

@@ -46,6 +46,7 @@ notes.txt
 
 package-lock.json
 
+cypress/downloads
 cypress/videos
 cypress/screenshots
 

cypress/e2e/universal-search.test.ts

@@ -1,12 +1,21 @@
 describe('Universal search bar', () => {
+  function openSearch() {
+    // can't just type "/" because on mobile it doesn't respond to that
+    cy.get('[data-cy="magnifying-icon"]').parent().eq(1).click()
+  }
+
   beforeEach(() => {
     cy.visit('/')
-    cy.get('[data-cy="magnifying-icon"]').parent().eq(1).click()
+    openSearch()
   })
 
+  function getSearchBar() {
+    return cy.get('[data-cy="search-bar-input"]').last()
+  }
+
   it('should yield clickable result for regular token or nft collection search term', () => {
     // Search for uni token by name.
-    cy.get('[data-cy="search-bar-input"]').last().clear().type('uni')
+    getSearchBar().clear().type('uni')
     cy.get('[data-cy="searchbar-token-row-UNI"]')
       .should('contain.text', 'Uniswap')
       .and('contain.text', 'UNI')
@@ -16,6 +25,32 @@ describe('Universal search bar', () => {
     cy.location('hash').should('equal', '#/tokens/ethereum/0x1f9840a85d5af5bf1d1762f925bdaddc4201f984')
   })
 
+  it('should go to the selected result when recent results are shown', () => {
+    // Search for uni token by name.
+    getSearchBar().type('uni')
+    cy.get('[data-cy="searchbar-token-row-UNI"]')
+
+    // Clear search
+    getSearchBar().clear()
+
+    // Close search
+    getSearchBar().type('{esc}')
+
+    openSearch()
+
+    // Search a different token by name.
+    getSearchBar().type('eth')
+
+    // Validate ETH result now exists.
+    cy.get('[data-cy="searchbar-token-row-ETH"]')
+
+    // Hit enter
+    getSearchBar().type('{enter}')
+
+    // Validate we went to ethereum address
+    cy.url().should('contain', 'tokens/ethereum/NATIVE')
+  })
+
   it.skip('should show recent tokens and popular tokens with empty search term', () => {
     cy.get('[data-cy="magnifying-icon"]')
       .parent()
@@ -23,7 +58,7 @@ describe('Universal search bar', () => {
         $navIcon.click()
       })
     // Recently searched UNI token should exist.
-    cy.get('[data-cy="search-bar-input"]').last().clear()
+    getSearchBar().clear()
     cy.get('[data-cy="searchbar-dropdown"]')
       .contains('[data-cy="searchbar-dropdown"]', 'Recent searches')
       .find('[data-cy="searchbar-token-row-UNI"]')
@@ -39,7 +74,7 @@ describe('Universal search bar', () => {
 
   it.skip('should show blocked badge when blocked token is searched for', () => {
     // Search for mTSLA, which is a blocked token.
-    cy.get('[data-cy="search-bar-input"]').last().clear().type('mtsla')
+    getSearchBar().clear().type('mtsla')
     cy.get('[data-cy="searchbar-token-row-mTSLA"]').find('[data-cy="blocked-icon"]').should('exist')
   })
 })

src/components/NavBar/SearchBarDropdown.tsx

@@ -17,12 +17,14 @@ import { Box } from 'nft/components/Box'
 import { Column, Row } from 'nft/components/Flex'
 import { subheadSmall } from 'nft/css/common.css'
 import { GenieCollection, TrendingCollection } from 'nft/types'
-import { ReactNode, useEffect, useMemo, useState } from 'react'
+import { useEffect, useMemo, useState } from 'react'
 import { useLocation } from 'react-router-dom'
 import styled from 'styled-components/macro'
 import { ThemedText } from 'theme'
 
 import { ClockIcon, TrendingArrow } from '../../nft/components/icons'
+import { SuspendConditionally } from '../Suspense/SuspendConditionally'
+import { SuspenseWithPreviousRenderAsFallback } from '../Suspense/SuspenseWithPreviousRenderAsFallback'
 import { useRecentlySearchedAssets } from './RecentlySearchedAssets'
 import * as styles from './SearchBar.css'
 import { CollectionRow, SkeletonRow, TokenRow } from './SuggestionRow'
@@ -132,24 +134,46 @@ interface SearchBarDropdownProps {
   isLoading: boolean
 }
 
-export const SearchBarDropdown = ({
+export const SearchBarDropdown = (props: SearchBarDropdownProps) => {
+  const { isLoading } = props
+  const { chainId } = useWeb3React()
+  const showChainComingSoonBadge = chainId && BACKEND_NOT_YET_SUPPORTED_CHAIN_IDS.includes(chainId) && !isLoading
+  const logoUri = getChainInfo(chainId)?.logoUrl
+
+  return (
+    <Column overflow="hidden" className={clsx(styles.searchBarDropdownNft, styles.searchBarScrollable)}>
+      <Box opacity={isLoading ? '0.3' : '1'} transition="125">
+        <SuspenseWithPreviousRenderAsFallback>
+          <SuspendConditionally if={isLoading}>
+            <SearchBarDropdownContents {...props} />
+          </SuspendConditionally>
+        </SuspenseWithPreviousRenderAsFallback>
+        {showChainComingSoonBadge && (
+          <ChainComingSoonBadge>
+            <ChainLogo src={logoUri} />
+            <ThemedText.BodySmall color="textSecondary" fontSize="14px" fontWeight="400" lineHeight="20px">
+              <ComingSoonText chainId={chainId} />
+            </ThemedText.BodySmall>
+          </ChainComingSoonBadge>
+        )}
+      </Box>
+    </Column>
+  )
+}
+
+function SearchBarDropdownContents({
   toggleOpen,
   tokens,
   collections,
   queryText,
   hasInput,
-  isLoading,
-}: SearchBarDropdownProps) => {
+}: SearchBarDropdownProps): JSX.Element {
   const [hoveredIndex, setHoveredIndex] = useState<number | undefined>(0)
-
   const { data: searchHistory } = useRecentlySearchedAssets()
   const shortenedHistory = useMemo(() => searchHistory?.slice(0, 2) ?? [...Array<SearchToken>(2)], [searchHistory])
-
   const { pathname } = useLocation()
-  const { chainId } = useWeb3React()
   const isNFTPage = useIsNftPage()
   const isTokenPage = pathname.includes('/tokens')
-  const [resultsState, setResultsState] = useState<ReactNode>()
   const shouldDisableNFTRoutes = useDisableNFTRoutes()
 
   const { data: trendingCollections, loading: trendingCollectionsAreLoading } = useTrendingCollections(
@@ -222,9 +246,8 @@ export const SearchBarDropdown = ({
 
   const trace = JSON.stringify(useTrace({ section: InterfaceSectionName.NAVBAR_SEARCH }))
 
-  useEffect(() => {
   const eventProperties = { total_suggestions: totalSuggestions, query_text: queryText, ...JSON.parse(trace) }
-    if (!isLoading) {
+
   const tokenSearchResults =
     tokens.length > 0 ? (
       <SearchBarDropdownSection
@@ -263,8 +286,7 @@ export const SearchBarDropdown = ({
       <Box className={styles.notFoundContainer}>No NFT collections found.</Box>
     )
 
-      const currentState = () =>
-        hasInput ? (
+  return hasInput ? (
     // Empty or Up to 8 combined tokens and nfts
     <Column gap="20">
       {showCollectionsFirst ? (
@@ -332,49 +354,6 @@ export const SearchBarDropdown = ({
       )}
     </Column>
   )
-
-      setResultsState(currentState)
-    }
-  }, [
-    isLoading,
-    tokens,
-    collections,
-    formattedTrendingCollections,
-    trendingTokens,
-    trendingTokenData,
-    hoveredIndex,
-    toggleOpen,
-    shortenedHistory,
-    hasInput,
-    isNFTPage,
-    isTokenPage,
-    showCollectionsFirst,
-    queryText,
-    totalSuggestions,
-    trace,
-    searchHistory,
-    trendingCollectionsAreLoading,
-    shouldDisableNFTRoutes,
-  ])
-
-  const showChainComingSoonBadge = chainId && BACKEND_NOT_YET_SUPPORTED_CHAIN_IDS.includes(chainId) && !isLoading
-  const logoUri = getChainInfo(chainId)?.logoUrl
-
-  return (
-    <Column overflow="hidden" className={clsx(styles.searchBarDropdownNft, styles.searchBarScrollable)}>
-      <Box opacity={isLoading ? '0.3' : '1'} transition="125">
-        {resultsState}
-        {showChainComingSoonBadge && (
-          <ChainComingSoonBadge>
-            <ChainLogo src={logoUri} />
-            <ThemedText.BodySmall color="textSecondary" fontSize="14px" fontWeight="400" lineHeight="20px">
-              <ComingSoonText chainId={chainId} />
-            </ThemedText.BodySmall>
-          </ChainComingSoonBadge>
-        )}
-      </Box>
-    </Column>
-  )
 }
 
 function ComingSoonText({ chainId }: { chainId: ChainId }) {

src/components/Suspense/SuspendConditionally.tsx

@@ -0,0 +1,21 @@
+import React from 'react'
+import { useState } from 'react'
+
+export const SuspendConditionally = (props: { if: boolean; children: React.ReactNode }) => {
+  useSuspendIf(props.if)
+  return <>{props.children}</>
+}
+
+function useSuspendIf(shouldSuspend = false) {
+  const [resolve, setResolve] = useState<((val?: unknown) => void) | undefined>()
+
+  if (!resolve && shouldSuspend) {
+    const promise = new Promise((res) => {
+      setResolve(res)
+    })
+    throw promise
+  } else if (resolve && !shouldSuspend) {
+    resolve()
+    setResolve(undefined)
+  }
+}

src/components/Suspense/SuspenseWithPreviousRenderAsFallback.tsx

@@ -0,0 +1,14 @@
+import usePrevious from 'hooks/usePrevious'
+import React, { Suspense } from 'react'
+
+/**
+ * This is useful for keeping the "last rendered" components on-screen while any suspense
+ * is triggered below this component.
+ *
+ * It stores a reference to the current children, and then returns them as the fallback.
+ */
+
+export const SuspenseWithPreviousRenderAsFallback = (props: { children: React.ReactNode }) => {
+  const previousContents = usePrevious(props.children)
+  return <Suspense fallback={previousContents ?? null}>{props.children}</Suspense>
+}