-
Notifications
You must be signed in to change notification settings - Fork 15
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
EVM Staking - Add wallet connect and show connected network #1818
Conversation
✅ Deploy Preview for hubble-stats ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for tangle-dapp ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Awesome work @devpavan04 ! Designs look great @monaiuu |
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.
LG, a lot changed and small things removed/added. Can you explain rationale?
@@ -450,11 +453,6 @@ export class NoteManager { | |||
|
|||
/** | |||
* Generate a note | |||
* @param sourceTypedChainId The source typed chain id |
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.
Why remove this?
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.
Hey, @devpavan04, could you please assist me in reverting this documentation? I might have accidentally removed it. Please note that I removed the tokenDecimals
argument.
CI is also failing @devpavan04 |
@AtelyPham Could you provide more context regarding the changes made to resolve the build error encountered when importing |
@drewstone Sorry for the oversight regarding the use of |
✅ Deploy Preview for stats-dapp development is ready! Thanks for the contribution @devpavan04
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for stats-dapp development is ready! Thanks for the contribution @devpavan04
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for bridge-dapp development is ready! Thanks for the contribution @devpavan04
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for stats-dapp development is ready! Thanks for the contribution @devpavan04
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for bridge-dapp development is ready! Thanks for the contribution @devpavan04
To edit notification comments on pull requests, go to your Netlify site settings. |
() => calculateTypedChainId(activeChain!.chainType, activeChain!.id), | ||
[] | ||
[activeChain] |
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.
Pass the activeChain
as the prop here to prevent non-null assertion here.
@@ -1,3 +1,4 @@ | |||
/* eslint-disable @typescript-eslint/no-non-null-assertion */ |
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.
Why do you need to add this? This is not good actually.
@@ -1,3 +1,4 @@ | |||
/* eslint-disable @typescript-eslint/no-non-null-assertion */ |
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.
Same here.
@@ -1,3 +1,4 @@ | |||
/* eslint-disable @typescript-eslint/no-non-null-assertion */ |
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.
Same 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.
Deduplicate this please, I think it is the same with the WalletModal
on the bridge.
export interface ConnectWalletMobileButtonProps | ||
extends IWebbComponentBase, | ||
Pick< | ||
ButtonProps, | ||
'variant' | 'isFullWidth' | 'leftIcon' | 'rightIcon' | 'size' | 'variant' | ||
> {} | ||
>, | ||
MobileButtonProps {} |
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.
Why don't we just add the new properties inside the interface curly brackets {}
? 🤔 Or why do you want to add the MobileButtonProps
type 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.
Left some comments.
✅ Deploy Preview for stats-dapp development is ready! Thanks for the contribution @devpavan04
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for bridge-dapp development is ready! Thanks for the contribution @devpavan04
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for stats-dapp development is ready! Thanks for the contribution @AtelyPham
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for bridge-dapp development is ready! Thanks for the contribution @AtelyPham
To edit notification comments on pull requests, go to your Netlify site settings. |
Summary of changes
Proposed area of change
apps/bridge-dapp
apps/hubble-stats
apps/stats-dapp
apps/tangle-dapp
apps/faucet
libs/webb-ui-components
Reference issue to close (if applicable)
Screen Recording
CleanShot.2023-11-07.at.12.49.11.mp4