diff --git a/.changeset/late-bottles-greet.md b/.changeset/late-bottles-greet.md new file mode 100644 index 000000000..8c960e8b8 --- /dev/null +++ b/.changeset/late-bottles-greet.md @@ -0,0 +1,5 @@ +--- +'@coinbase/onchainkit': patch +--- + +-**chore**: updated `SwapSettingsSlippageInput` to use the input config defaultMaxSlippage value. By @cpcramer #1263. diff --git a/src/swap/components/SwapProvider.tsx b/src/swap/components/SwapProvider.tsx index b682e7bac..3e19404a2 100644 --- a/src/swap/components/SwapProvider.tsx +++ b/src/swap/components/SwapProvider.tsx @@ -330,6 +330,7 @@ export function SwapProvider({ const value = useValue({ address, + config, from, handleAmountChange, handleToggle, diff --git a/src/swap/components/SwapSettingsSlippageInput.test.tsx b/src/swap/components/SwapSettingsSlippageInput.test.tsx index 96caeaa40..85b950daf 100644 --- a/src/swap/components/SwapSettingsSlippageInput.test.tsx +++ b/src/swap/components/SwapSettingsSlippageInput.test.tsx @@ -1,5 +1,6 @@ import { fireEvent, render, screen } from '@testing-library/react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { DEFAULT_MAX_SLIPPAGE } from '../constants'; import { SwapSettingsSlippageInput } from './SwapSettingsSlippageInput'; const mockSetLifecycleStatus = vi.fn(); @@ -7,7 +8,7 @@ let mockLifecycleStatus = { statusName: 'init', statusData: { isMissingRequiredField: false, - maxSlippage: 3, + maxSlippage: DEFAULT_MAX_SLIPPAGE, }, }; @@ -15,6 +16,7 @@ vi.mock('./SwapProvider', () => ({ useSwapContext: () => ({ updateLifecycleStatus: mockSetLifecycleStatus, lifecycleStatus: mockLifecycleStatus, + config: { maxSlippage: DEFAULT_MAX_SLIPPAGE }, }), })); @@ -29,14 +31,16 @@ describe('SwapSettingsSlippageInput', () => { statusName: 'init', statusData: { isMissingRequiredField: false, - maxSlippage: 3, + maxSlippage: DEFAULT_MAX_SLIPPAGE, }, }; }); it('renders with default props', () => { render(); - expect(screen.getByRole('textbox')).toHaveValue('3'); + expect(screen.getByRole('textbox')).toHaveValue( + DEFAULT_MAX_SLIPPAGE.toString(), + ); expect(screen.getByText('%')).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Auto' })).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Custom' })).toBeInTheDocument(); @@ -58,11 +62,13 @@ describe('SwapSettingsSlippageInput', () => { expect(screen.getByRole('textbox')).toHaveValue('1.5'); }); - it('allows input changes in Custom mode', () => { + it('allows input changes in Custom mode but maintains default value', () => { render(); fireEvent.click(screen.getByRole('button', { name: 'Custom' })); fireEvent.change(screen.getByRole('textbox'), { target: { value: '2.5' } }); - expect(screen.getByRole('textbox')).toHaveValue('2.5'); + expect(screen.getByRole('textbox')).toHaveValue( + DEFAULT_MAX_SLIPPAGE.toString(), + ); expect(mockSetLifecycleStatus).toHaveBeenCalledWith({ statusName: 'slippageChange', statusData: { @@ -76,12 +82,19 @@ describe('SwapSettingsSlippageInput', () => { expect(screen.getByRole('textbox')).toBeDisabled(); }); - it('prevents invalid input', () => { + it('handles invalid input by maintaining default value', () => { render(); fireEvent.click(screen.getByRole('button', { name: 'Custom' })); fireEvent.change(screen.getByRole('textbox'), { target: { value: 'abc' } }); - expect(screen.getByRole('textbox')).toHaveValue('3'); - expect(mockSetLifecycleStatus).not.toHaveBeenCalled(); + expect(screen.getByRole('textbox')).toHaveValue( + DEFAULT_MAX_SLIPPAGE.toString(), + ); + expect(mockSetLifecycleStatus).toHaveBeenCalledWith({ + statusName: 'slippageChange', + statusData: { + maxSlippage: 0, + }, + }); }); it('handles decimal input correctly', () => { @@ -90,7 +103,9 @@ describe('SwapSettingsSlippageInput', () => { fireEvent.change(screen.getByRole('textbox'), { target: { value: '2.75' }, }); - expect(screen.getByRole('textbox')).toHaveValue('2.75'); + expect(screen.getByRole('textbox')).toHaveValue( + DEFAULT_MAX_SLIPPAGE.toString(), + ); expect(mockSetLifecycleStatus).toHaveBeenCalledWith({ statusName: 'slippageChange', statusData: { @@ -124,12 +139,19 @@ describe('SwapSettingsSlippageInput', () => { ); }); - it('handles empty input correctly', () => { + it('handles empty input by maintaining default value', () => { render(); fireEvent.click(screen.getByRole('button', { name: 'Custom' })); fireEvent.change(screen.getByRole('textbox'), { target: { value: '' } }); - expect(screen.getByRole('textbox')).toHaveValue('0'); - expect(mockSetLifecycleStatus).not.toHaveBeenCalled(); + expect(screen.getByRole('textbox')).toHaveValue( + DEFAULT_MAX_SLIPPAGE.toString(), + ); + expect(mockSetLifecycleStatus).toHaveBeenCalledWith({ + statusName: 'slippageChange', + statusData: { + maxSlippage: 0, + }, + }); }); it('uses lifecycleStatus maxSlippage when available', () => { @@ -159,12 +181,19 @@ describe('SwapSettingsSlippageInput', () => { expect(screen.getByRole('textbox')).not.toBeDisabled(); }); - it('handles non-numeric input correctly', () => { + it('handles non-numeric input by maintaining default value', () => { render(); fireEvent.click(screen.getByRole('button', { name: 'Custom' })); fireEvent.change(screen.getByRole('textbox'), { target: { value: 'abc' } }); - expect(screen.getByRole('textbox')).toHaveValue('3'); - expect(mockSetLifecycleStatus).not.toHaveBeenCalled(); + expect(screen.getByRole('textbox')).toHaveValue( + DEFAULT_MAX_SLIPPAGE.toString(), + ); + expect(mockSetLifecycleStatus).toHaveBeenCalledWith({ + statusName: 'slippageChange', + statusData: { + maxSlippage: 0, + }, + }); }); it('updates slippage when switching from Custom to Auto', () => { @@ -172,23 +201,33 @@ describe('SwapSettingsSlippageInput', () => { fireEvent.click(screen.getByRole('button', { name: 'Custom' })); fireEvent.change(screen.getByRole('textbox'), { target: { value: '5' } }); fireEvent.click(screen.getByRole('button', { name: 'Auto' })); - expect(screen.getByRole('textbox')).toHaveValue('3'); + expect(screen.getByRole('textbox')).toHaveValue( + DEFAULT_MAX_SLIPPAGE.toString(), + ); expect(mockSetLifecycleStatus).toHaveBeenLastCalledWith({ statusName: 'slippageChange', statusData: { - maxSlippage: 3, + maxSlippage: 5, }, }); }); - it('maintains custom value when in Custom mode', () => { + it('maintains default value when switching between Auto and Custom mode', () => { render(); + expect(screen.getByRole('textbox')).toHaveValue( + DEFAULT_MAX_SLIPPAGE.toString(), + ); fireEvent.click(screen.getByRole('button', { name: 'Custom' })); + expect(screen.getByRole('textbox')).toHaveValue( + DEFAULT_MAX_SLIPPAGE.toString(), + ); fireEvent.change(screen.getByRole('textbox'), { target: { value: '5' } }); - expect(screen.getByRole('textbox')).toHaveValue('5'); + expect(screen.getByRole('textbox')).toHaveValue( + DEFAULT_MAX_SLIPPAGE.toString(), + ); fireEvent.click(screen.getByRole('button', { name: 'Auto' })); - fireEvent.click(screen.getByRole('button', { name: 'Custom' })); - fireEvent.change(screen.getByRole('textbox'), { target: { value: '7' } }); - expect(screen.getByRole('textbox')).toHaveValue('7'); + expect(screen.getByRole('textbox')).toHaveValue( + DEFAULT_MAX_SLIPPAGE.toString(), + ); }); }); diff --git a/src/swap/components/SwapSettingsSlippageInput.tsx b/src/swap/components/SwapSettingsSlippageInput.tsx index 18ed07b3f..df3d682cf 100644 --- a/src/swap/components/SwapSettingsSlippageInput.tsx +++ b/src/swap/components/SwapSettingsSlippageInput.tsx @@ -12,47 +12,44 @@ const SLIPPAGE_SETTINGS = { export function SwapSettingsSlippageInput({ className, }: SwapSettingsSlippageInputReact) { - const { updateLifecycleStatus, lifecycleStatus } = useSwapContext(); - const getMaxSlippage = useCallback(() => { - return lifecycleStatus.statusData.maxSlippage; - }, [lifecycleStatus.statusData]); + const { + config: { maxSlippage: defaultMaxSlippage }, + updateLifecycleStatus, + lifecycleStatus, + } = useSwapContext(); // Set initial slippage values to match previous selection or default, // ensuring consistency when dropdown is reopened - const [slippage, setSlippage] = useState(getMaxSlippage()); const [slippageSetting, setSlippageSetting] = useState( - getMaxSlippage() === DEFAULT_MAX_SLIPPAGE + lifecycleStatus.statusData.maxSlippage === defaultMaxSlippage ? SLIPPAGE_SETTINGS.AUTO : SLIPPAGE_SETTINGS.CUSTOM, ); const updateSlippage = useCallback( (newSlippage: number) => { - setSlippage(newSlippage); - updateLifecycleStatus({ - statusName: 'slippageChange', - statusData: { - maxSlippage: newSlippage, - }, - }); + if (newSlippage !== lifecycleStatus.statusData.maxSlippage) { + updateLifecycleStatus({ + statusName: 'slippageChange', + statusData: { + maxSlippage: newSlippage, + }, + }); + } }, - [updateLifecycleStatus], + [lifecycleStatus.statusData.maxSlippage, updateLifecycleStatus], ); - // Handles user input for custom slippage - // Parses the input and updates slippage if valid + // Handles user input for custom slippage. + // Parses the input and updates slippage state. const handleSlippageChange = useCallback( - (newSlippage: string) => { - // Empty '' when the input field is cleared. - if (newSlippage === '') { - setSlippage(0); - return; - } + (e: React.ChangeEvent) => { + const newSlippage = e.target.value; + const parsedSlippage = Number.parseFloat(newSlippage); + const isValidNumber = !Number.isNaN(parsedSlippage); - const newSlippageNumber = Number.parseFloat(newSlippage); - if (!Number.isNaN(newSlippageNumber)) { - updateSlippage(newSlippageNumber); - } + // Update slippage to parsed value if valid, otherwise set to 0 + updateSlippage(isValidNumber ? parsedSlippage : 0); }, [updateSlippage], ); @@ -119,8 +116,8 @@ export function SwapSettingsSlippageInput({ handleSlippageChange(e.target.value)} + value={lifecycleStatus.statusData.maxSlippage} + onChange={handleSlippageChange} disabled={slippageSetting === SLIPPAGE_SETTINGS.AUTO} className={cn( color.foreground, diff --git a/src/swap/types.ts b/src/swap/types.ts index 8498e2b3d..6050ec36a 100644 --- a/src/swap/types.ts +++ b/src/swap/types.ts @@ -162,8 +162,13 @@ export type SwapButtonReact = { disabled?: boolean; // Disables swap button }; +type SwapConfig = { + maxSlippage: number; // Maximum acceptable slippage for a swap. (default: 10) This is as a percent, not basis points; +}; + export type SwapContextType = { address?: Address; // Used to check if user is connected in SwapButton + config: SwapConfig; from: SwapUnit; lifecycleStatus: LifecycleStatus; handleAmountChange: ( @@ -239,9 +244,7 @@ export type SwapProviderReact = { export type SwapReact = { children: ReactNode; className?: string; // Optional className override for top div element. - config?: { - maxSlippage: number; // Maximum acceptable slippage for a swap. (default: 10) This is as a percent, not basis points - }; + config?: SwapConfig; experimental?: { useAggregator: boolean; // Whether to use a DEX aggregator. (default: true) };