From c07c401189c748fc3876593c1075726b6330dd4f Mon Sep 17 00:00:00 2001
From: Mike Grabowski <grabbou@gmail.com>
Date: Wed, 24 May 2023 22:02:59 +0400
Subject: [PATCH] feat: add animation to Settings menu (#6617)

* feat: add price impact back

* chore: update tes tname

* chore: update snapshot for price impact

* fix

* fix

* update snapshot after rebase

* update snapshot

* chore: finish

* chore: remove snapshot

* feat: add test matcher

* cleanup

* chore: add animation test

* add comment

* update comment
---
 .../AnimatedDropdown/index.test.tsx           | 31 ++++++++++++++++++
 src/components/AnimatedDropdown/index.tsx     | 14 +++-----
 src/components/Expand/index.test.tsx          |  4 +--
 src/components/Expand/index.tsx               | 11 +++++--
 .../MaxSlippageSettings/index.test.tsx        | 25 ++++++---------
 .../index.test.tsx                            | 21 ++++--------
 src/setupTests.ts                             |  5 +++
 src/test-utils/matchers.test.tsx              | 22 +++++++++++++
 src/test-utils/matchers.ts                    | 32 +++++++++++++++++++
 9 files changed, 122 insertions(+), 43 deletions(-)
 create mode 100644 src/components/AnimatedDropdown/index.test.tsx
 create mode 100644 src/test-utils/matchers.test.tsx
 create mode 100644 src/test-utils/matchers.ts

diff --git a/src/components/AnimatedDropdown/index.test.tsx b/src/components/AnimatedDropdown/index.test.tsx
new file mode 100644
index 0000000000..e7c53ca7d7
--- /dev/null
+++ b/src/components/AnimatedDropdown/index.test.tsx
@@ -0,0 +1,31 @@
+import { render, screen, waitFor } from 'test-utils/render'
+
+import AnimatedDropdown from './index'
+
+describe('AnimatedDropdown', () => {
+  it('does not render children when closed', () => {
+    render(<AnimatedDropdown open={false}>Body</AnimatedDropdown>)
+    expect(screen.getByText('Body')).not.toBeVisible()
+  })
+
+  it('renders children when open', () => {
+    render(<AnimatedDropdown open={true}>Body</AnimatedDropdown>)
+    expect(screen.getByText('Body')).toBeVisible()
+  })
+
+  it('animates when open changes', async () => {
+    const { rerender } = render(<AnimatedDropdown open={false}>Body</AnimatedDropdown>)
+
+    const body = screen.getByText('Body')
+
+    expect(body).not.toBeVisible()
+
+    rerender(<AnimatedDropdown open={true}>Body</AnimatedDropdown>)
+    expect(body).not.toBeVisible()
+
+    // wait for React Spring animation to finish
+    await waitFor(() => {
+      expect(body).toBeVisible()
+    })
+  })
+})
diff --git a/src/components/AnimatedDropdown/index.tsx b/src/components/AnimatedDropdown/index.tsx
index 9e1f2a0f55..d0f7805f65 100644
--- a/src/components/AnimatedDropdown/index.tsx
+++ b/src/components/AnimatedDropdown/index.tsx
@@ -9,7 +9,10 @@ export default function AnimatedDropdown({ open, children }: React.PropsWithChil
   const { ref, height } = useResizeObserver()
 
   const props = useSpring({
-    height: open ? height ?? 0 : 0,
+    // On initial render, `height` will be undefined as ref has not been set yet.
+    // If the dropdown should be open, we fallback to `auto` to avoid flickering.
+    // Otherwise, we just animate between actual height (when open) and 0 (when closed).
+    height: open ? height ?? 'auto' : 0,
     config: {
       mass: 1.2,
       tension: 300,
@@ -20,14 +23,7 @@ export default function AnimatedDropdown({ open, children }: React.PropsWithChil
   })
 
   return (
-    <animated.div
-      style={{
-        ...props,
-        overflow: 'hidden',
-        width: '100%',
-        willChange: 'height',
-      }}
-    >
+    <animated.div style={{ ...props, overflow: 'hidden', width: '100%', willChange: 'height' }}>
       <div ref={ref}>{children}</div>
     </animated.div>
   )
diff --git a/src/components/Expand/index.test.tsx b/src/components/Expand/index.test.tsx
index 7701107d9b..c5a495e5df 100644
--- a/src/components/Expand/index.test.tsx
+++ b/src/components/Expand/index.test.tsx
@@ -10,7 +10,7 @@ describe('Expand', () => {
         Body
       </Expand>
     )
-    expect(screen.queryByText('Body')).not.toBeInTheDocument()
+    expect(screen.queryByText('Body')).not.toBeVisible()
   })
 
   it('renders children when open', () => {
@@ -19,7 +19,7 @@ describe('Expand', () => {
         Body
       </Expand>
     )
-    expect(screen.queryByText('Body')).toBeInTheDocument()
+    expect(screen.queryByText('Body')).toBeVisible()
   })
 
   it('calls `onToggle` when button is pressed', () => {
diff --git a/src/components/Expand/index.tsx b/src/components/Expand/index.tsx
index 3415c0ea74..6fa41735ed 100644
--- a/src/components/Expand/index.tsx
+++ b/src/components/Expand/index.tsx
@@ -1,3 +1,4 @@
+import AnimatedDropdown from 'components/AnimatedDropdown'
 import Column from 'components/Column'
 import React, { PropsWithChildren, ReactElement } from 'react'
 import { ChevronDown } from 'react-feather'
@@ -17,6 +18,10 @@ const ExpandIcon = styled(ChevronDown)<{ $isOpen: boolean }>`
   transition: transform ${({ theme }) => theme.transition.duration.medium};
 `
 
+const Content = styled(Column)`
+  padding-top: ${({ theme }) => theme.grids.md};
+`
+
 export default function Expand({
   header,
   button,
@@ -32,7 +37,7 @@ export default function Expand({
   onToggle: () => void
 }>) {
   return (
-    <Column gap="md">
+    <Column>
       <RowBetween>
         {header}
         <ButtonContainer data-testid={testId} onClick={onToggle} aria-expanded={isOpen}>
@@ -40,7 +45,9 @@ export default function Expand({
           <ExpandIcon $isOpen={isOpen} />
         </ButtonContainer>
       </RowBetween>
-      {isOpen && children}
+      <AnimatedDropdown open={isOpen}>
+        <Content gap="md">{children}</Content>
+      </AnimatedDropdown>
     </Column>
   )
 }
diff --git a/src/components/Settings/MaxSlippageSettings/index.test.tsx b/src/components/Settings/MaxSlippageSettings/index.test.tsx
index f17fb70ee3..bfafb16955 100644
--- a/src/components/Settings/MaxSlippageSettings/index.test.tsx
+++ b/src/components/Settings/MaxSlippageSettings/index.test.tsx
@@ -12,13 +12,6 @@ const renderSlippageSettings = () => {
   render(<MaxSlippageSettings autoSlippage={AUTO_SLIPPAGE} />)
 }
 
-const renderAndExpandSlippageSettings = () => {
-  renderSlippageSettings()
-
-  // By default, the button to expand Slippage component and show `input` will have `Auto` label
-  fireEvent.click(screen.getByText('Auto'))
-}
-
 // Switch to custom mode by tapping on `Custom` label
 const switchToCustomSlippage = () => {
   fireEvent.click(screen.getByText('Custom'))
@@ -34,21 +27,21 @@ describe('MaxSlippageSettings', () => {
     })
     it('is not expanded by default', () => {
       renderSlippageSettings()
-      expect(getSlippageInput()).not.toBeInTheDocument()
+      expect(getSlippageInput()).not.toBeVisible()
     })
     it('is expanded by default when custom slippage is set', () => {
       store.dispatch(updateUserSlippageTolerance({ userSlippageTolerance: 10 }))
       renderSlippageSettings()
-      expect(getSlippageInput()).toBeInTheDocument()
+      expect(getSlippageInput()).toBeVisible()
     })
     it('does not render auto slippage as a value, but a placeholder', () => {
-      renderAndExpandSlippageSettings()
+      renderSlippageSettings()
       switchToCustomSlippage()
 
       expect(getSlippageInput().value).toBe('')
     })
     it('renders custom slippage above the input', () => {
-      renderAndExpandSlippageSettings()
+      renderSlippageSettings()
       switchToCustomSlippage()
 
       fireEvent.change(getSlippageInput(), { target: { value: '0.5' } })
@@ -56,7 +49,7 @@ describe('MaxSlippageSettings', () => {
       expect(screen.queryAllByText('0.50%').length).toEqual(1)
     })
     it('updates input value on blur with the slippage in store', () => {
-      renderAndExpandSlippageSettings()
+      renderSlippageSettings()
       switchToCustomSlippage()
 
       const input = getSlippageInput()
@@ -66,7 +59,7 @@ describe('MaxSlippageSettings', () => {
       expect(input.value).toBe('0.50')
     })
     it('clears errors on blur and overwrites incorrect value with the latest correct value', () => {
-      renderAndExpandSlippageSettings()
+      renderSlippageSettings()
       switchToCustomSlippage()
 
       const input = getSlippageInput()
@@ -78,7 +71,7 @@ describe('MaxSlippageSettings', () => {
       expect(input.value).toBe('50.00')
     })
     it('does not allow to enter more than 2 digits after the decimal point', () => {
-      renderAndExpandSlippageSettings()
+      renderSlippageSettings()
       switchToCustomSlippage()
 
       const input = getSlippageInput()
@@ -88,7 +81,7 @@ describe('MaxSlippageSettings', () => {
       expect(input.value).toBe('0.01')
     })
     it('does not accept non-numerical values', () => {
-      renderAndExpandSlippageSettings()
+      renderSlippageSettings()
       switchToCustomSlippage()
 
       const input = getSlippageInput()
@@ -97,7 +90,7 @@ describe('MaxSlippageSettings', () => {
       expect(input.value).toBe('')
     })
     it('does not set slippage when user enters `.` value', () => {
-      renderAndExpandSlippageSettings()
+      renderSlippageSettings()
       switchToCustomSlippage()
 
       const input = getSlippageInput()
diff --git a/src/components/Settings/TransactionDeadlineSettings/index.test.tsx b/src/components/Settings/TransactionDeadlineSettings/index.test.tsx
index e0553be137..433a1559ea 100644
--- a/src/components/Settings/TransactionDeadlineSettings/index.test.tsx
+++ b/src/components/Settings/TransactionDeadlineSettings/index.test.tsx
@@ -9,13 +9,6 @@ const renderTransactionDeadlineSettings = () => {
   render(<TransactionDeadlineSettings />)
 }
 
-const renderAndExpandTransactionDeadlineSettings = () => {
-  renderTransactionDeadlineSettings()
-
-  // By default, the button to expand Slippage component and show `input` will have `<deadline>m` label
-  fireEvent.click(screen.getByText(`${DEFAULT_DEADLINE_FROM_NOW / 60}m`))
-}
-
 const getDeadlineInput = () => screen.queryByTestId('deadline-input') as HTMLInputElement
 
 describe('TransactionDeadlineSettings', () => {
@@ -26,26 +19,26 @@ describe('TransactionDeadlineSettings', () => {
     })
     it('is not expanded by default', () => {
       renderTransactionDeadlineSettings()
-      expect(getDeadlineInput()).not.toBeInTheDocument()
+      expect(getDeadlineInput()).not.toBeVisible()
     })
     it('is expanded by default when custom deadline is set', () => {
       store.dispatch(updateUserDeadline({ userDeadline: DEFAULT_DEADLINE_FROM_NOW * 2 }))
       renderTransactionDeadlineSettings()
-      expect(getDeadlineInput()).toBeInTheDocument()
+      expect(getDeadlineInput()).toBeVisible()
     })
     it('does not render default deadline as a value, but a placeholder', () => {
-      renderAndExpandTransactionDeadlineSettings()
+      renderTransactionDeadlineSettings()
       expect(getDeadlineInput().value).toBe('')
     })
     it('renders custom deadline above the input', () => {
-      renderAndExpandTransactionDeadlineSettings()
+      renderTransactionDeadlineSettings()
 
       fireEvent.change(getDeadlineInput(), { target: { value: '50' } })
 
       expect(screen.queryAllByText('50m').length).toEqual(1)
     })
     it('marks deadline as invalid if it is greater than 4320m (3 days) or 0m', () => {
-      renderAndExpandTransactionDeadlineSettings()
+      renderTransactionDeadlineSettings()
 
       const input = getDeadlineInput()
       fireEvent.change(input, { target: { value: '4321' } })
@@ -55,7 +48,7 @@ describe('TransactionDeadlineSettings', () => {
       expect(input.value).toBe('')
     })
     it('clears errors on blur and overwrites incorrect value with the latest correct value', () => {
-      renderAndExpandTransactionDeadlineSettings()
+      renderTransactionDeadlineSettings()
 
       const input = getDeadlineInput()
       fireEvent.change(input, { target: { value: '5' } })
@@ -69,7 +62,7 @@ describe('TransactionDeadlineSettings', () => {
       expect(input.value).toBe('5')
     })
     it('does not accept non-numerical values', () => {
-      renderAndExpandTransactionDeadlineSettings()
+      renderTransactionDeadlineSettings()
 
       const input = getDeadlineInput()
       fireEvent.change(input, { target: { value: 'c' } })
diff --git a/src/setupTests.ts b/src/setupTests.ts
index 1ec83bd87e..5ca98c6d67 100644
--- a/src/setupTests.ts
+++ b/src/setupTests.ts
@@ -7,6 +7,7 @@ import type { createPopper } from '@popperjs/core'
 import { useWeb3React } from '@web3-react/core'
 import failOnConsole from 'jest-fail-on-console'
 import { Readable } from 'stream'
+import { toBeVisible } from 'test-utils/matchers'
 import { mocked } from 'test-utils/mocked'
 import { TextDecoder, TextEncoder } from 'util'
 
@@ -100,3 +101,7 @@ failOnConsole({
   shouldFailOnLog: true,
   shouldFailOnWarn: true,
 })
+
+expect.extend({
+  toBeVisible,
+})
diff --git a/src/test-utils/matchers.test.tsx b/src/test-utils/matchers.test.tsx
new file mode 100644
index 0000000000..b96dc5dc29
--- /dev/null
+++ b/src/test-utils/matchers.test.tsx
@@ -0,0 +1,22 @@
+import { render, screen } from './render'
+
+describe('matchers', () => {
+  describe('toBeVisible', () => {
+    it('should return true if element is visible', () => {
+      render(<div>test</div>)
+      expect(screen.getByText('test')).toBeVisible()
+    })
+    it('should return false if element is hidden', () => {
+      render(<div style={{ height: 0 }}>test</div>)
+      expect(screen.getByText('test')).not.toBeVisible()
+    })
+    it('should return false if parent element is hidden', () => {
+      render(
+        <div style={{ height: 0 }}>
+          <div>test</div>
+        </div>
+      )
+      expect(screen.getByText('test')).not.toBeVisible()
+    })
+  })
+})
diff --git a/src/test-utils/matchers.ts b/src/test-utils/matchers.ts
new file mode 100644
index 0000000000..e116c17628
--- /dev/null
+++ b/src/test-utils/matchers.ts
@@ -0,0 +1,32 @@
+// This type is not exported from Jest, so we need to infer it from the expect.extend function.
+type MatcherFunction = Parameters<typeof expect.extend>[0] extends { [key: string]: infer I } ? I : never
+
+const isElementVisible = (element: HTMLElement): boolean => {
+  return element.style.height !== '0px' && (!element.parentElement || isElementVisible(element.parentElement))
+}
+
+// Overrides the Testing Library matcher to check for height when determining whether an element is visible.
+// We are doing this because:
+// - original `toBeVisible()` does not take `height` into account
+// https://github.com/testing-library/jest-dom/issues/450
+// - original `toBeVisible()` and `toHaveStyle()` does not work at all in some cases
+// https://github.com/testing-library/jest-dom/issues/209
+// - `getComputedStyles()` returns empty object, making it impossible to check for Styled Components styles
+// https://github.com/styled-components/styled-components/issues/3262
+// https://github.com/jsdom/jsdom/issues/2986
+// For the reasons above, this matcher only works for inline styles.
+export const toBeVisible: MatcherFunction = function (element: HTMLElement) {
+  const isVisible = isElementVisible(element)
+  return {
+    pass: isVisible,
+    message: () => {
+      const is = isVisible ? 'is' : 'is not'
+      return [
+        this.utils.matcherHint(`${this.isNot ? '.not' : ''}.toBeVisible`, 'element', ''),
+        '',
+        `Received element ${is} visible:`,
+        `  ${this.utils.printReceived(element.cloneNode(false))}`,
+      ].join('\n')
+    },
+  }
+}