-
Notifications
You must be signed in to change notification settings - Fork 273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add Swap Settings #1210
Merged
Merged
feat: Add Swap Settings #1210
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
fa3c2cd
`SwapSettings`
0xAlec 65f7462
Add button type prop
cpcramer 39bebad
changeset
cpcramer 818f332
tests
cpcramer 7735d76
cleaning
cpcramer 719acc2
cleaning
cpcramer 0893f30
formatting and linting
cpcramer ee25865
asdf
cpcramer a684dc6
memoize getMaxSlippage
cpcramer 07b0f2d
asdf
cpcramer 7e99805
buildg
cpcramer 68b1e6f
processSwapTransaction fixes
cpcramer 550d020
asdf
cpcramer 5bdd025
asfd
cpcramer b4992d1
fix test
cpcramer 03e0f86
asdf
cpcramer 73ac6df
asdf
cpcramer 7fbe2ec
fixes
cpcramer fce027d
fix
cpcramer cab4738
Update to pass lifeCycleStatus instead of maxSlippage to processSwapT…
cpcramer c933993
fix tests
cpcramer 3579bc7
Add missing useCallback dependency
cpcramer 787b238
testing coverage
cpcramer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@coinbase/onchainkit': patch | ||
--- | ||
|
||
-**feat**: added custom slippage support settings sub-component in the `Swap` component. By @cpcramer #1210 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,180 @@ | ||
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(() => <svg data-testid="mock-icon" />), | ||
})); | ||
|
||
vi.mock('./SwapSettingsSlippageLayout', () => ({ | ||
SwapSettingsSlippageLayout: vi.fn(({ children }) => ( | ||
<div data-testid="mock-layout">{children}</div> | ||
)), | ||
})); | ||
|
||
vi.mock('./SwapSettingsSlippageLayoutBottomSheet', () => ({ | ||
SwapSettingsSlippageLayoutBottomSheet: vi.fn(({ children }) => ( | ||
<div data-testid="mock-bottom-sheet">{children}</div> | ||
)), | ||
})); | ||
|
||
vi.mock('./SwapSettingsSlippageTitle', () => ({ | ||
SwapSettingsSlippageTitle: vi.fn(() => <div>Title</div>), | ||
})); | ||
|
||
vi.mock('./SwapSettingsSlippageDescription', () => ({ | ||
SwapSettingsSlippageDescription: vi.fn(() => <div>Description</div>), | ||
})); | ||
|
||
vi.mock('./SwapSettingsSlippageInput', () => ({ | ||
SwapSettingsSlippageInput: vi.fn(() => <div>Input</div>), | ||
})); | ||
|
||
vi.mock('../../useBreakpoints', () => ({ | ||
useBreakpoints: vi.fn(), | ||
})); | ||
|
||
const useBreakpointsMock = useBreakpoints as vi.Mock; | ||
|
||
const renderComponent = (props = {}) => { | ||
return render( | ||
<SwapSettings {...props}> | ||
<SwapSettingsSlippageTitle /> | ||
<SwapSettingsSlippageDescription /> | ||
<SwapSettingsSlippageInput /> | ||
</SwapSettings>, | ||
); | ||
}; | ||
|
||
describe('SwapSettings', () => { | ||
beforeEach(() => { | ||
vi.clearAllMocks(); | ||
useBreakpointsMock.mockReturnValue('md'); | ||
}); | ||
|
||
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('should render with custom props and handle outside click', async () => { | ||
render( | ||
<div> | ||
<SwapSettings | ||
text="Custom Text" | ||
className="custom-class" | ||
icon="custom-icon" | ||
/> | ||
<div data-testid="outside">Outside</div> | ||
</div>, | ||
); | ||
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('should handle non-valid React elements as children', async () => { | ||
render( | ||
<SwapSettings> | ||
<SwapSettingsSlippageTitle /> | ||
Plain text child | ||
<SwapSettingsSlippageInput /> | ||
</SwapSettings>, | ||
); | ||
const button = screen.getByRole('button', { | ||
name: /toggle swap settings/i, | ||
}); | ||
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(); | ||
}); | ||
}); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't want this in state do we? If the experimental prop changes, should that update here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The experimental prop won't change after the component is rendered.
It will set the initial
maxSlippage
value. Any updates will be handled byLifeCycleStatus.slippageChange.maxSlippage
propertyhttps://github.com/coinbase/onchainkit/pull/1210/files#diff-9bca97e90be8d6cfaa2c6813dbf1d684ed792b80b395f6b033d3d06963f23371R91-R94
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And we update this value using
setLifeCycleStatus
'https://github.com/coinbase/onchainkit/pull/1210/files#diff-704c4aa565af1d2becb4f70ce49ac884069cdb8e451c6e3afe603a6d47de07bcR35-R38
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some context on why we are setting it this way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are only using this const to initialize lifecycle status, can it just be a const? or alternatively we could move the initiallifecycle status into a const?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cpcramer that sentence it's incorrect! We should always expect a prop can change, that's the all point of prop. :)