fix: eliminate flake from account connect race condition (#7212)

* fix: reduce flake from race condition

* use interface instead of type for props

* closeMenu instead of closeModal with type

* simplify useCloseModal

* fix bug

* pr feedback

* remove unnecessary code

* fix button wrapper style

* remove ApplicationModal.WALLET

* update color for spore

cypress/e2e/swap/settings.test.ts

@@ -15,14 +15,22 @@ describe('Swap settings', () => {
   })
 
   it('should open the mobile settings menu', () => {
+    // Set viewport to iPhone 6
     cy.viewport('iphone-6')
     cy.visit('/swap')
-    cy.get(getTestSelector('open-settings-dialog-button')).click()
+
-    cy.get(getTestSelector('mobile-settings-menu')).should('exist')
+    // Click the button to open the settings dialog
-    cy.contains('Max slippage').should('exist')
+    cy.get(getTestSelector('open-settings-dialog-button')).click({ waitForAnimations: true })
-    cy.contains('Transaction deadline').should('exist')
+
-    cy.contains('UniswapX').should('exist')
+    // Verify the mobile settings menu and its contents
-    cy.contains('Local routing').should('exist')
+    cy.get(getTestSelector('mobile-settings-menu'))
-    cy.get(getTestSelector('mobile-settings-scrim')).click({ force: true })
+      .should('exist')
+      .within(() => {
+        cy.contains('Max slippage').should('exist')
+        cy.contains('UniswapX').should('exist')
+        cy.contains('Local routing').should('exist')
+        cy.contains('Transaction deadline').should('exist')
+        cy.get(getTestSelector('mobile-settings-close')).click()
+      })
   })
 })

src/components/AccountDrawer/index.tsx

@@ -40,7 +40,7 @@ export function useAccountDrawer(): [boolean, () => void] {
   return [accountDrawerOpen, useToggleAccountDrawer()]
 }
 
-const ScrimBackground = styled.div<{ open: boolean }>`
+const ScrimBackground = styled.div<{ $open: boolean }>`
   z-index: ${Z_INDEX.modalBackdrop};
   overflow: hidden;
   top: 0;
@@ -53,22 +53,27 @@ const ScrimBackground = styled.div<{ open: boolean }>`
   opacity: 0;
   pointer-events: none;
   @media only screen and (max-width: ${({ theme }) => `${theme.breakpoint.sm}px`}) {
-    opacity: ${({ open }) => (open ? 1 : 0)};
+    opacity: ${({ $open }) => ($open ? 1 : 0)};
-    pointer-events: ${({ open }) => (open ? 'auto' : 'none')};
+    pointer-events: ${({ $open }) => ($open ? 'auto' : 'none')};
     transition: opacity ${({ theme }) => theme.transition.duration.medium} ease-in-out;
   }
 `
-export const Scrim = ({ onClick, open, testId }: { onClick: () => void; open: boolean; testId?: string }) => {
+
+interface ScrimBackgroundProps extends React.ComponentPropsWithRef<'div'> {
+  $open: boolean
+}
+
+export const Scrim = (props: ScrimBackgroundProps) => {
   const { width } = useWindowSize()
 
   useEffect(() => {
-    if (width && width < BREAKPOINTS.sm && open) document.body.style.overflow = 'hidden'
+    if (width && width < BREAKPOINTS.sm && props.$open) document.body.style.overflow = 'hidden'
     return () => {
       document.body.style.overflow = 'visible'
     }
-  }, [open, width])
+  }, [props.$open, width])
 
-  return <ScrimBackground data-testid={testId} onClick={onClick} open={open} />
+  return <ScrimBackground {...props} />
 }
 
 const AccountDrawerScrollWrapper = styled.div`
@@ -245,7 +250,7 @@ function AccountDrawer() {
           </CloseDrawer>
         </TraceEvent>
       )}
-      <Scrim onClick={toggleWalletDrawer} open={walletDrawerOpen} />
+      <Scrim onClick={toggleWalletDrawer} $open={walletDrawerOpen} />
       <AccountDrawerWrapper
         open={walletDrawerOpen}
         {...(isMobile

src/components/FiatOnrampModal/index.tsx

@@ -125,7 +125,7 @@ export default function FiatOnrampModal() {
   }, [fetchSignedIframeUrl])
 
   return (
-    <Modal isOpen={fiatOnrampModalOpen} onDismiss={closeModal} height={80 /* vh */}>
+    <Modal isOpen={fiatOnrampModalOpen} onDismiss={() => closeModal()} height={80 /* vh */}>
       <Wrapper data-testid="fiat-onramp-modal" isDarkMode={isDarkMode}>
         {error ? (
           <>

src/components/Settings/index.tsx

@@ -10,13 +10,14 @@ import useDisableScrolling from 'hooks/useDisableScrolling'
 import { useOnClickOutside } from 'hooks/useOnClickOutside'
 import { Portal } from 'nft/components/common/Portal'
 import { useIsMobile } from 'nft/hooks'
-import { useMemo, useRef } from 'react'
+import { useCallback, useMemo, useRef } from 'react'
-import { useModalIsOpen, useToggleSettingsMenu } from 'state/application/hooks'
+import { X } from 'react-feather'
+import { useCloseModal, useModalIsOpen, useToggleSettingsMenu } from 'state/application/hooks'
 import { ApplicationModal } from 'state/application/reducer'
 import { InterfaceTrade } from 'state/routing/types'
 import { isUniswapXTrade } from 'state/routing/utils'
 import styled from 'styled-components'
-import { CloseIcon, Divider, ThemedText } from 'theme'
+import { Divider, ThemedText } from 'theme'
 import { Z_INDEX } from 'theme/zIndex'
 
 import MaxSlippageSettings from './MaxSlippageSettings'
@@ -24,6 +25,16 @@ import MenuButton from './MenuButton'
 import RouterPreferenceSettings from './RouterPreferenceSettings'
 import TransactionDeadlineSettings from './TransactionDeadlineSettings'
 
+const CloseButton = styled.button`
+  background: transparent;
+  border: none;
+  color: ${({ theme }) => theme.neutral1};
+  cursor: pointer;
+  height: 24px;
+  padding: 0;
+  width: 24px;
+`
+
 const Menu = styled.div`
   position: relative;
 `
@@ -64,14 +75,14 @@ const MobileMenuContainer = styled(Row)`
   z-index: ${Z_INDEX.fixed};
 `
 
-const MobileMenuWrapper = styled(Column)<{ open: boolean }>`
+const MobileMenuWrapper = styled(Column)<{ $open: boolean }>`
   height: min-content;
   width: 100%;
   padding: 8px 16px 24px;
   background-color: ${({ theme }) => theme.surface1};
   overflow: hidden;
   position: absolute;
-  bottom: ${({ open }) => (open ? `100vh` : 0)};
+  bottom: ${({ $open }) => ($open ? `100vh` : 0)};
   transition: bottom ${({ theme }) => theme.transition.duration.medium};
   border: ${({ theme }) => `1px solid ${theme.surface3}`};
   border-radius: 12px;
@@ -97,17 +108,18 @@ export default function SettingsTab({
 }) {
   const { chainId: connectedChainId } = useWeb3React()
   const showDeadlineSettings = Boolean(chainId && !L2_CHAIN_IDS.includes(chainId))
-
   const node = useRef<HTMLDivElement | null>(null)
   const isOpen = useModalIsOpen(ApplicationModal.SETTINGS)
 
+  const closeModal = useCloseModal()
+  const closeMenu = useCallback(() => closeModal(ApplicationModal.SETTINGS), [closeModal])
   const toggleMenu = useToggleSettingsMenu()
 
   const isMobile = useIsMobile()
   const isOpenMobile = isOpen && isMobile
   const isOpenDesktop = isOpen && !isMobile
 
-  useOnClickOutside(node, isOpenDesktop ? toggleMenu : undefined)
+  useOnClickOutside(node, isOpenDesktop ? closeMenu : undefined)
   useDisableScrolling(isOpen)
 
   const isChainSupported = isSupportedChain(chainId)
@@ -136,22 +148,18 @@ export default function SettingsTab({
   )
 
   return (
-    <>
+    <Menu ref={node}>
-      <Menu ref={node}>
+      <MenuButton disabled={!isChainSupported || chainId !== connectedChainId} isActive={isOpen} onClick={toggleMenu} />
-        <MenuButton
+      {isOpenDesktop && <MenuFlyout>{Settings}</MenuFlyout>}
-          disabled={!isChainSupported || chainId !== connectedChainId}
+      {isOpenMobile && (
-          isActive={isOpen}
-          onClick={toggleMenu}
-        />
-        {isOpenDesktop && !isMobile && <MenuFlyout>{Settings}</MenuFlyout>}
-      </Menu>
-      {isMobile && (
         <Portal>
           <MobileMenuContainer data-testid="mobile-settings-menu">
-            <Scrim testId="mobile-settings-scrim" onClick={toggleMenu} open={isOpenMobile} />
+            <Scrim onClick={closeMenu} $open />
-            <MobileMenuWrapper open={isOpenMobile}>
+            <MobileMenuWrapper $open>
               <MobileMenuHeader padding="8px 0px 4px">
-                <CloseIcon size={24} onClick={toggleMenu} />
+                <CloseButton data-testid="mobile-settings-close" onClick={closeMenu}>
+                  <X size={24} />
+                </CloseButton>
                 <Row padding="0px 24px 0px 0px" justify="center">
                   <ThemedText.SubHeader>
                     <Trans>Settings</Trans>
@@ -163,6 +171,6 @@ export default function SettingsTab({
           </MobileMenuContainer>
         </Portal>
       )}
-    </>
+    </Menu>
   )
 }

src/state/application/hooks.ts

@@ -88,9 +88,21 @@ export function useToggleModal(modal: ApplicationModal): () => void {
   return useCallback(() => dispatch(setOpenModal(isOpen ? null : modal)), [dispatch, modal, isOpen])
 }
 
-export function useCloseModal(): () => void {
+export function useCloseModal() {
   const dispatch = useAppDispatch()
-  return useCallback(() => dispatch(setOpenModal(null)), [dispatch])
+  const currentlyOpenModal = useAppSelector((state: AppState) => state.application.openModal)
+  return useCallback(
+    (modalToClose?: ApplicationModal) => {
+      if (!modalToClose) {
+        // Close any open modal if no modal is specified.
+        dispatch(setOpenModal(null))
+      } else if (currentlyOpenModal === modalToClose) {
+        // Close the currently open modal if it is the one specified.
+        dispatch(setOpenModal(null))
+      }
+    },
+    [currentlyOpenModal, dispatch]
+  )
 }
 
 export function useOpenModal(modal: ApplicationModal): () => void {

src/state/application/reducer.test.ts

@@ -72,11 +72,7 @@ describe('application reducer', () => {
   })
 
   describe('setOpenModal', () => {
-    it('set wallet modal', () => {
+    it('should correctly set the open modal', () => {
-      store.dispatch(setOpenModal(ApplicationModal.WALLET))
-      expect(store.getState().openModal).toEqual(ApplicationModal.WALLET)
-      store.dispatch(setOpenModal(ApplicationModal.WALLET))
-      expect(store.getState().openModal).toEqual(ApplicationModal.WALLET)
       store.dispatch(setOpenModal(ApplicationModal.CLAIM_POPUP))
       expect(store.getState().openModal).toEqual(ApplicationModal.CLAIM_POPUP)
       store.dispatch(setOpenModal(null))

src/state/application/reducer.ts

@@ -43,7 +43,6 @@ export enum ApplicationModal {
   TAX_SERVICE,
   TIME_SELECTOR,
   VOTE,
-  WALLET,
   UNISWAP_NFT_AIRDROP_CLAIM,
 }
 

src/state/application/updater.ts

@@ -2,28 +2,18 @@ import { useWeb3React } from '@web3-react/core'
 import { asSupportedChain } from 'constants/chains'
 import useDebounce from 'hooks/useDebounce'
 import useIsWindowVisible from 'hooks/useIsWindowVisible'
-import { useEffect, useRef, useState } from 'react'
+import { useEffect, useState } from 'react'
 import { useAppDispatch } from 'state/hooks'
 
-import { useCloseModal } from './hooks'
 import { updateChainId } from './reducer'
 
 export default function Updater(): null {
-  const { account, chainId, provider } = useWeb3React()
+  const { chainId, provider } = useWeb3React()
   const dispatch = useAppDispatch()
   const windowVisible = useIsWindowVisible()
 
   const [activeChainId, setActiveChainId] = useState(chainId)
 
-  const closeModal = useCloseModal()
-  const previousAccountValue = useRef(account)
-  useEffect(() => {
-    if (account && account !== previousAccountValue.current) {
-      previousAccountValue.current = account
-      closeModal()
-    }
-  }, [account, closeModal])
-
   useEffect(() => {
     if (provider && chainId && windowVisible) {
       setActiveChainId(chainId)