Base template for transactions#12
Conversation
Preview stand statusStand was demolished |
ev-d
changed the title
| @@ -5,7 +5,7 @@ export const VAULT_HUB_BY_NETWORK: { | |||
| [key in CHAINS]?: Address; | |||
| } = { | |||
| [CHAINS.Mainnet]: '0x', | |||
There was a problem hiding this comment.
Better delete than have invalid value
| <ToggleSwitch | ||
| options={adjustmentToggleList} | ||
| defaultActive={initialPath} | ||
| onToggleCb={({ value }) => handleToggleCb(value as AdjustmentPaths)} |
There was a problem hiding this comment.
Cb? callback? just onToggle is clear
There was a problem hiding this comment.
This is complicated system for just two value. Consider 'mint' | 'repay' type.
| import { FeatureTxInfo } from './feature-tx-info'; | ||
| import { FormContainer } from './styles'; | ||
|
|
||
| export const Form = () => { |
There was a problem hiding this comment.
consider naming MintForm for easier codebase nav
| import { useVaultInfo } from 'features/overview/contexts'; | ||
|
|
||
| export const useMintWithDelegation = (onMutate = () => {}) => { | ||
| const { chainId } = useDappStatus(); |
There was a problem hiding this comment.
Due to vault ui not having to support multiple set of supported chains (like stake widget). You don't need dappStatus. Wagmi will get chain from wagmi context.
Also you don't need to pass wagmiConfig, but you might need to use global type https://wagmi.sh/react/typescript#declaration-merging
| <LayoutStyles> | ||
| <Header /> | ||
| <Navigation /> | ||
| <Navigation address={address} /> |
|
|
||
| export const Navigation: FC = memo(() => { | ||
| type NavigationProps = { | ||
| address?: Address; |
There was a problem hiding this comment.
Not sure what is address here? Vault? User?
There was a problem hiding this comment.
If user - bad, delete . NEVER user address in routeing for better privacy
if vault, better naming, but also consider nested navigation and layout for vault managment and outside. Those are separate componentns.
| export const GET_REQUESTS: GetRequest[] = [ | ||
| { | ||
| uri: '/api/rewards?address=0x87c0e047F4e4D3e289A56a36570D4CB957A37Ef1¤cy=usd&onlyRewards=false&archiveRate=true&skip=0&limit=10', | ||
| uri: '/api/rewards?address-field=0x87c0e047F4e4D3e289A56a36570D4CB957A37Ef1¤cy=usd&onlyRewards=false&archiveRate=true&skip=0&limit=10', |
| return { | ||
| props: { address }, | ||
| }; | ||
| } catch (error) { |
There was a problem hiding this comment.
just 404 why redirect. Don't smooth out errors - it's bad ux
| // Check if address is address | ||
| // Check if address is deployed contract | ||
| // Check if address is vault contract | ||
| export const getDefaultServerSideProps: GetServerSideProps< |
There was a problem hiding this comment.
ABSOLUTLY zero sense for us to do this on server side. Develop like this is SPA.
| @@ -15,7 +15,7 @@ export interface PostRequest { | |||
|
|
|||
| export const GET_REQUESTS: GetRequest[] = [ | |||
| { | |||
There was a problem hiding this comment.
Let's remove this, this is leftover from widget, not actual
| const pathname = useRouterPath(); | ||
| const { isWalletConnected } = useDappStatus(); | ||
| const router = useRouter(); | ||
| const { address: vaultAddress } = router.query as { address: Address }; |
There was a problem hiding this comment.
consider change in next file to avoid confusion
| } else { | ||
| setError(`Vault with address ${address} not found`); | ||
| void router.push(AppPaths.main); | ||
| void router.push(AppPaths.notFound); |
There was a problem hiding this comment.
.replace is correct behavior for this
2 participants
Description
Fund, withdraw, mint, repay and claim forms
Checklist: