-
Notifications
You must be signed in to change notification settings - Fork 7
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
New metaport architecture #184
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
if (el) { | ||
// createRoot(el).render(<Widget config={config} />); | ||
} else { | ||
console.log('div with id="metaport" does not exist') | ||
} | ||
} |
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.
No element is being created here. Confirming this is correct?
Also would say ideally due to the empty commented if, if it is correct to switch to an if only where it is more like
if (el === undefined) {
console.log('div with id="metaport" does not exist')
}
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 one will utilised later, once we will have a standalone build of Metaport
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.
Majority of feedback is focused on minor cleanup and code clarity. Overhaul looks good 👍
export default function AddToken(props: { | ||
token: TokenData | ||
destChainName: string | ||
mpc: MetaportCore |
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.
suggest making this variable name metaportCore
export default function AmountErrorMessage() { | ||
const amountErrorMessage = useMetaportStore((state) => state.amountErrorMessage) | ||
return ( | ||
<Collapse in={!!amountErrorMessage || amountErrorMessage === ''} className="noMarg"> |
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.
suggest changing noMarg to noMargin**
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.
removed, this class name is obsolete
} | ||
|
||
.tokenSymbolPlaceholder { | ||
color: #979797; |
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.
Suggest moving colors to vars css file and pre-fixing all with --metaport-... if possible to avoid naming collisions
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.
removed some of the redundant CSS
@@ -14,14 +24,15 @@ | |||
border-radius: 4px 0 0 4px; | |||
padding: 9pt 15pt; | |||
font-weight: bold !important; | |||
font-size: 1.3rem !important; | |||
} | |||
|
|||
:global(.MuiFormControl-root) { | |||
width: 100%; | |||
// border-radius: 4px; |
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.
Remove unnecessary comment
const transferInProgress = useMetaportStore((state) => state.transferInProgress) | ||
const setAmount = useMetaportStore((state) => state.setAmount) | ||
const amount = useMetaportStore((state) => state.amount) | ||
const expandedTokens = useCollapseStore((state) => state.expandedTokens) | ||
|
||
const handleChange = (event: React.ChangeEvent<HTMLInputElement>) => { | ||
if (parseFloat(event.target.value) < 0) { |
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 no need to return an empty, suggest flipping so that the if checks if > 0 and then sets amount so that only an if is needed
chainName: string, | ||
address: AddressType, | ||
mpc: MetaportCore |
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.
suggest changing to metaportCore
src/core/metaport.ts
Outdated
|
||
export const createTokensMap = ( | ||
chainName1: string, | ||
chainName2: string | null | undefined, |
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.
Possible to reduce to this to null or undefined instead of both?
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.
upd: null
src/core/wagmi_network.ts
Outdated
// return chain.rpcUrls.default.webSocket ? chain.rpcUrls.default.webSocket[0] : ''; | ||
return chain.rpcUrls.default.webSocket | ||
? chain.rpcUrls.default.webSocket[0] | ||
: 'wss://goerli-light.eth.linkpool.io/ws' // TODO - IP! |
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 default to a goerli websocket endpoint?
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.
updated this function to return the correct URL for different networks
'staging-faint-slimy-achird', // Nebula | ||
'staging-perfect-parallel-gacrux', // Test Chain 1 | ||
'staging-severe-violet-wezen', // Test Chain 2 | ||
'staging-weepy-fitting-caph' // Tank War Zone |
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.
remove chain from config
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.
✅
// 'staging-faint-slimy-achird': { | ||
// hub: 'staging-legal-crazy-castor' | ||
// } |
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.
Remove unused?
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.
✅
Summary
This PR introduces major updates to the Metaport Widget, elevating it to version 2.0. From unifying account connections with RainbowKit to enabling multi-chain transfers, this release brings numerous new features and improvements.
A full list of tickets resolved by this PR can be found here: https://github.com/skalenetwork/metaport/milestone/4