-
Notifications
You must be signed in to change notification settings - Fork 6
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
[TAS-182] Feat/web3 #21
Conversation
- add wagmi, viem, rainbowKit - add providers lib for wagmi context - add .env for wallet connect project id, add to gitignore - update eslint imports rule - add browserlistrc for bigint support
- add providers for wagmi connected and prices - add generic inputs WIP🚧 - integration in Swap module - add token icons - add infura key env - eslint fix
- add theme module override - inject theme declarations in libs/app - update sx props to use mui theme - remove typescript strict
@@ -0,0 +1,108 @@ | |||
import { formatUnits, parseUnits } from 'viem'; |
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.
Nice utility lib
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.
I'm still not sure if we should use it... I think this kind of abstraction would root deeply in the code base and might not be sustainable... Anyway, I'll leave it there for now, it's not used anywhere, let's see if we want to keep it or not
import type { Token } from '@origin/shared/contracts'; | ||
|
||
export const { Provider: SwapProvider, useTracked: useSwapState } = | ||
createContainer(() => { |
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.
This is interesting, never used react-tracked
but I like the interface
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.
very cool library made by the creator of Zustand, Jotai and Valtio. Super clean and lean api, just a small wrapper around context api that provides proxy support. It allows for granular re-renders of children of context providers and this kind of pattern for "subscribing" to state changes.
import { useState } from 'react'; | ||
|
||
import { SwapCard } from '@origin/shared/components'; | ||
import random from 'lodash/random'; |
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.
Are we switching to ramda
still?
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.
I will, but not in this PR. I'll update right after
if (onChange && num !== value) { | ||
onChange(num); | ||
} | ||
} catch {} |
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.
We don't need to do anything on error state?
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.
yeah, this is something to discuss with Matt, I used to change color to palette.error.main
but it could also be the focus frame... anyway, it's there for now, to be refined
@@ -57,7 +58,7 @@ export function ConnectedButton({ | |||
height: (theme) => theme.spacing(3), | |||
}} | |||
/> | |||
<UserId userId={userId} /> | |||
<MiddleTruncated>{userId}</MiddleTruncated> |
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.
Should this be a component or just a function that acts on string?
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.
it needs to be a component because you don't know in advance where the ellipsis will happen. This component allows for having a "resizable" label that always keep the end
digits visible
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.
Minor nitpick things but overall LGTM, will approve as most comments are just suggestions that can also be revisited later.
libs/defi/oeth/src/components/Swap
becomes/libs/oeth/swap