From 65f74620030b5e933a29f6bba55fd2f5c4c27e3f Mon Sep 17 00:00:00 2001 From: Paul Cramer Date: Sun, 25 Aug 2024 22:05:39 -0700 Subject: [PATCH] Add button type prop --- .changeset/clean-mirrors-sparkle.md | 5 - .changeset/sixty-clocks-shop.md | 5 + site/docs/pages/swap/types.mdx | 10 - src/swap/components/Swap.tsx | 3 +- src/swap/components/SwapSettings.test.tsx | 179 +++++++++++++++--- src/swap/components/SwapSettings.tsx | 141 ++++++-------- .../components/SwapSettingsBottomSheet.tsx | 0 .../components/SwapSettingsSlippageInput.tsx | 29 ++- src/swap/index.ts | 4 +- src/swap/types.ts | 1 + 10 files changed, 243 insertions(+), 134 deletions(-) delete mode 100644 .changeset/clean-mirrors-sparkle.md create mode 100644 .changeset/sixty-clocks-shop.md delete mode 100644 src/swap/components/SwapSettingsBottomSheet.tsx diff --git a/.changeset/clean-mirrors-sparkle.md b/.changeset/clean-mirrors-sparkle.md deleted file mode 100644 index 81c21d35b9..0000000000 --- a/.changeset/clean-mirrors-sparkle.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@coinbase/onchainkit": patch ---- - -**feat**: Add SwapSettings sub-component. This allows the user to customize their swaps max slippage percentage. If max slippage is not specified then the component will default to a max slippage of 10%. by @0xAlec @cpcramer #1051 diff --git a/.changeset/sixty-clocks-shop.md b/.changeset/sixty-clocks-shop.md new file mode 100644 index 0000000000..771a986107 --- /dev/null +++ b/.changeset/sixty-clocks-shop.md @@ -0,0 +1,5 @@ +--- +"@coinbase/onchainkit": patch +--- + +- **feat**: Implement slippage support in Swap component with settings UI. This combined feat incorporates slippage functionality into the Swap component, including a dedicated settings section with a title, description, and input field for configuring slippage tolerance. By @cpcramer #1176 #1187 #1189 #1191 #1192 #1195 #1196 #1206 \ No newline at end of file diff --git a/site/docs/pages/swap/types.mdx b/site/docs/pages/swap/types.mdx index d5cb7333f2..73beff276b 100644 --- a/site/docs/pages/swap/types.mdx +++ b/site/docs/pages/swap/types.mdx @@ -180,16 +180,6 @@ type SwapReact = { }; ``` -## `SwapSettingsReact` - -```ts -type SwapSettingsReact = { - className?: string; // Optional className override for top div element. - icon?: ReactNode; // Optional icon override - text?: string; // Optional text override -}; -``` - ## `SwapToggleButtonReact` ```ts diff --git a/src/swap/components/Swap.tsx b/src/swap/components/Swap.tsx index 9fe8ca0745..b9d790efb6 100644 --- a/src/swap/components/Swap.tsx +++ b/src/swap/components/Swap.tsx @@ -37,7 +37,6 @@ export function Swap({ if (!isMounted) { return null; } - return ( {title} -
{swapSettings}
+ {swapSettings} {inputs[0]}
{toggleButton}
diff --git a/src/swap/components/SwapSettings.test.tsx b/src/swap/components/SwapSettings.test.tsx index 010c245d75..1406d1d6cf 100644 --- a/src/swap/components/SwapSettings.test.tsx +++ b/src/swap/components/SwapSettings.test.tsx @@ -1,7 +1,40 @@ -import { render, screen } from '@testing-library/react'; -import { describe, expect, it } from 'vitest'; +import '@testing-library/jest-dom'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { beforeEach, vi } from 'vitest'; import { useBreakpoints } from '../../useBreakpoints'; +import { useIcon } from '../../wallet/hooks/useIcon'; import { SwapSettings } from './SwapSettings'; +import { SwapSettingsSlippageDescription } from './SwapSettingsSlippageDescription'; +import { SwapSettingsSlippageInput } from './SwapSettingsSlippageInput'; +import { SwapSettingsSlippageTitle } from './SwapSettingsSlippageTitle'; + +vi.mock('../../wallet/hooks/useIcon', () => ({ + useIcon: vi.fn(() => ), +})); + +vi.mock('./SwapSettingsSlippageLayout', () => ({ + SwapSettingsSlippageLayout: vi.fn(({ children }) => ( +
{children}
+ )), +})); + +vi.mock('./SwapSettingsSlippageLayoutBottomSheet', () => ({ + SwapSettingsSlippageLayoutBottomSheet: vi.fn(({ children }) => ( +
{children}
+ )), +})); + +vi.mock('./SwapSettingsSlippageTitle', () => ({ + SwapSettingsSlippageTitle: vi.fn(() =>
Title
), +})); + +vi.mock('./SwapSettingsSlippageDescription', () => ({ + SwapSettingsSlippageDescription: vi.fn(() =>
Description
), +})); + +vi.mock('./SwapSettingsSlippageInput', () => ({ + SwapSettingsSlippageInput: vi.fn(() =>
Input
), +})); vi.mock('../../useBreakpoints', () => ({ useBreakpoints: vi.fn(), @@ -9,39 +42,139 @@ vi.mock('../../useBreakpoints', () => ({ const useBreakpointsMock = useBreakpoints as vi.Mock; +const renderComponent = (props = {}) => { + return render( + + + + + , + ); +}; + describe('SwapSettings', () => { - it('renders with default title', () => { + beforeEach(() => { + vi.clearAllMocks(); useBreakpointsMock.mockReturnValue('md'); - render(); - const settingsContainer = screen.getByTestId('ockSwapSettings_Settings'); - expect(settingsContainer.textContent).toBe(''); }); - it('renders with custom title', () => { - useBreakpointsMock.mockReturnValue('md'); - render(); - expect(screen.getByText('Custom')).toBeInTheDocument(); + it('should render with default props and handle dropdown toggle for desktop', async () => { + renderComponent(); + expect(screen.getByTestId('ockSwapSettings_Settings')).toBeInTheDocument(); + expect(screen.getByTestId('mock-icon')).toBeInTheDocument(); + const button = screen.getByRole('button', { + name: /toggle swap settings/i, + }); + fireEvent.click(button); + await waitFor(() => { + expect(screen.getByTestId('ockSwapSettingsDropdown')).toBeInTheDocument(); + expect(screen.getByTestId('mock-layout')).toBeInTheDocument(); + expect(screen.getByText('Title')).toBeInTheDocument(); + expect(screen.getByText('Description')).toBeInTheDocument(); + expect(screen.getByText('Input')).toBeInTheDocument(); + }); + fireEvent.click(button); + await waitFor(() => { + expect( + screen.queryByTestId('ockSwapSettingsDropdown'), + ).not.toBeInTheDocument(); + }); }); - it('renders custom icon when provided', () => { - useBreakpointsMock.mockReturnValue('md'); - const CustomIcon = () => ( - + it('should render with custom props and handle outside click', async () => { + render( +
+ +
Outside
+
, ); - render(} />); - expect(screen.getByTestId('custom-icon')).toBeInTheDocument(); + expect(screen.getByText('Custom Text')).toBeInTheDocument(); + expect(screen.getByTestId('ockSwapSettings_Settings')).toHaveClass( + 'custom-class', + ); + expect(useIcon).toHaveBeenCalledWith({ icon: 'custom-icon' }); + const button = screen.getByRole('button', { + name: /toggle swap settings/i, + }); + fireEvent.click(button); + await waitFor(() => { + expect(screen.getByTestId('ockSwapSettingsDropdown')).toBeInTheDocument(); + }); + fireEvent.mouseDown(screen.getByTestId('outside')); + await waitFor(() => { + expect( + screen.queryByTestId('ockSwapSettingsDropdown'), + ).not.toBeInTheDocument(); + }); }); - it('applies correct classes to the button', () => { - useBreakpointsMock.mockReturnValue('md'); - render(); + it('should handle non-valid React elements as children', async () => { + render( + + + Plain text child + + , + ); const button = screen.getByRole('button', { name: /toggle swap settings/i, }); - expect(button).toHaveClass( - 'rounded-full p-2 opacity-50 transition-opacity hover:opacity-100', + fireEvent.click(button); + await waitFor(() => { + expect(screen.getByTestId('ockSwapSettingsDropdown')).toBeInTheDocument(); + expect(screen.getByText('Title')).toBeInTheDocument(); + expect(screen.getByText('Plain text child')).toBeInTheDocument(); + expect(screen.getByText('Input')).toBeInTheDocument(); + }); + }); + + it('renders SwapSettingsSlippageLayoutBottomSheet when breakpoint is "sm"', async () => { + useBreakpointsMock.mockReturnValue('sm'); + renderComponent(); + const button = screen.getByRole('button', { + name: /toggle swap settings/i, + }); + const initialBottomSheet = screen.getByTestId( + 'ockSwapSettingsSlippageLayoutBottomSheet_container', + ); + expect(initialBottomSheet).toHaveClass('ease-in-out'); + expect(initialBottomSheet).toHaveClass('group-[]:bottom-0'); + fireEvent.click(button); + await waitFor(() => { + const parentDiv = screen + .getByTestId('ockSwapSettings_Settings') + .querySelector('div'); + expect(parentDiv).toHaveClass('group'); + const openBottomSheet = screen.getByTestId( + 'ockSwapSettingsSlippageLayoutBottomSheet_container', + ); + expect(openBottomSheet).toHaveClass('group-[]:bottom-0'); + }); + fireEvent.click(button); + await waitFor(() => { + const parentDiv = screen + .getByTestId('ockSwapSettings_Settings') + .querySelector('div'); + expect(parentDiv).not.toHaveClass('group'); + const closedBottomSheet = screen.getByTestId( + 'ockSwapSettingsSlippageLayoutBottomSheet_container', + ); + expect(closedBottomSheet).toHaveClass('ease-in-out'); + }); + }); + + it('removes event listener on unmount', () => { + const removeEventListenerSpy = vi.spyOn(document, 'removeEventListener'); + const { unmount } = renderComponent(); + unmount(); + expect(removeEventListenerSpy).toHaveBeenCalledWith( + 'mousedown', + expect.any(Function), ); + removeEventListenerSpy.mockRestore(); }); }); diff --git a/src/swap/components/SwapSettings.tsx b/src/swap/components/SwapSettings.tsx index bdff237903..657820303f 100644 --- a/src/swap/components/SwapSettings.tsx +++ b/src/swap/components/SwapSettings.tsx @@ -1,126 +1,95 @@ -import { useCallback, useState } from 'react'; -import { - background, - cn, - color, - pressable, - text as themeText, -} from '../../styles/theme'; +import { useCallback, useEffect, useRef, useState } from 'react'; +import { background, border, cn, pressable, text } from '../../styles/theme'; import { useBreakpoints } from '../../useBreakpoints'; import { useIcon } from '../../wallet/hooks/useIcon'; import type { SwapSettingsReact } from '../types'; +import { SwapSettingsSlippageLayout } from './SwapSettingsSlippageLayout'; +import { SwapSettingsSlippageLayoutBottomSheet } from './SwapSettingsSlippageLayoutBottomSheet'; export function SwapSettings({ + children, className, icon = 'swapSettings', - text = '', + text: buttonText = '', }: SwapSettingsReact) { - const [isOpen, setIsOpen] = useState(false); - const [slippageMode, setSlippageMode] = useState<'Auto' | 'Custom'>('Auto'); - const [customSlippage, setCustomSlippage] = useState('0.5'); const breakpoint = useBreakpoints(); + const [isOpen, setIsOpen] = useState(false); + const dropdownRef = useRef(null); const handleToggle = useCallback(() => { setIsOpen(!isOpen); }, [isOpen]); - const iconSvg = useIcon({ icon }); + const handleClickOutsideComponent = (event: MouseEvent) => { + if ( + dropdownRef.current && + !dropdownRef.current.contains(event.target as Node) + ) { + setIsOpen(false); + } + }; - if (!breakpoint) { - return null; - } + useEffect(() => { + document.addEventListener('mousedown', handleClickOutsideComponent); + return () => { + document.removeEventListener('mousedown', handleClickOutsideComponent); + }; + }, [handleClickOutsideComponent]); - // Placeholder for SwapSettingsBottomSheet - // Implement mobile version here, similar to WalletBottomSheet - if (breakpoint === 'sm') { - return
Mobile version not implemented
; - } + const iconSvg = useIcon({ icon }); return (
- {text} -
+ {buttonText && {buttonText}} +
- {isOpen && ( + {breakpoint === 'sm' ? (
-
-

- Max. slippage -

-

- Your swap will revert if the prices change by more than the - selected percentage. -

-
-
- - -
-
- setCustomSlippage(e.target.value)} - className={cn( - background.default, - 'w-16 rounded-l-md border-t border-b border-l px-2 py-1 text-left', - )} - disabled={slippageMode === 'Auto'} - /> - - % - -
-
-
+ + {children} +
+ ) : ( + isOpen && ( +
+ + {children} + +
+ ) )}
diff --git a/src/swap/components/SwapSettingsBottomSheet.tsx b/src/swap/components/SwapSettingsBottomSheet.tsx deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/src/swap/components/SwapSettingsSlippageInput.tsx b/src/swap/components/SwapSettingsSlippageInput.tsx index 3c1b12fdc7..ec17e59efe 100644 --- a/src/swap/components/SwapSettingsSlippageInput.tsx +++ b/src/swap/components/SwapSettingsSlippageInput.tsx @@ -12,10 +12,23 @@ export function SwapSettingsSlippageInput({ className, defaultSlippage = 3, }: SwapSettingsSlippageInputReact) { - const { setLifeCycleStatus } = useSwapContext(); - const [slippage, setSlippage] = useState(defaultSlippage); + const { setLifeCycleStatus, lifeCycleStatus } = useSwapContext(); + const getMaxSlippage = () => { + if ( + !!lifeCycleStatus.statusData && + 'maxSlippage' in lifeCycleStatus.statusData + ) { + return lifeCycleStatus.statusData.maxSlippage; + } + return defaultSlippage; + }; + + // TODO: Add comment about how we do these checks so the dropdown opens to the correct slippage value and button that it left off on + const [slippage, setSlippage] = useState(getMaxSlippage()); const [slippageSetting, setSlippageSetting] = useState( - SLIPPAGE_SETTINGS.AUTO, + getMaxSlippage() === defaultSlippage + ? SLIPPAGE_SETTINGS.AUTO + : SLIPPAGE_SETTINGS.CUSTOM, ); const updateSlippage = useCallback( @@ -33,9 +46,13 @@ export function SwapSettingsSlippageInput({ // Parses the input and updates slippage if valid const handleSlippageChange = useCallback( (newSlippage: string) => { - const newSlippageNumber = Number.parseFloat(newSlippage); - if (!Number.isNaN(newSlippageNumber)) { - updateSlippage(newSlippageNumber); + if (newSlippage === '') { + setSlippage(0); + } else { + const newSlippageNumber = Number.parseFloat(newSlippage); + if (!Number.isNaN(newSlippageNumber)) { + updateSlippage(newSlippageNumber); + } } }, [updateSlippage], diff --git a/src/swap/index.ts b/src/swap/index.ts index 3586996a83..33682b1513 100644 --- a/src/swap/index.ts +++ b/src/swap/index.ts @@ -4,9 +4,9 @@ export { SwapAmountInput } from './components/SwapAmountInput'; export { SwapButton } from './components/SwapButton'; export { SwapMessage } from './components/SwapMessage'; export { SwapSettings } from './components/SwapSettings'; +export { SwapSettingsSlippageDescription } from './components/SwapSettingsSlippageDescription'; export { SwapSettingsSlippageInput } from './components/SwapSettingsSlippageInput'; export { SwapSettingsSlippageTitle } from './components/SwapSettingsSlippageTitle'; -export { SwapSettingsSlippageDescription } from './components/SwapSettingsSlippageDescription'; export { SwapToggleButton } from './components/SwapToggleButton'; export type { BuildSwapTransaction, @@ -21,9 +21,9 @@ export type { SwapQuote, SwapReact, SwapSettingsReact, + SwapSettingsSlippageDescriptionReact, SwapSettingsSlippageInputReact, SwapSettingsSlippageTitleReact, - SwapSettingsSlippageDescriptionReact, SwapToggleButtonReact, Transaction, } from './types'; diff --git a/src/swap/types.ts b/src/swap/types.ts index f60fd4569f..db28e58384 100644 --- a/src/swap/types.ts +++ b/src/swap/types.ts @@ -237,6 +237,7 @@ export type SwapReact = { * Note: exported as public Type */ export type SwapSettingsReact = { + children: ReactNode; className?: string; // Optional className override for top div element. icon?: ReactNode; // Optional icon override text?: string; // Optional text override