-
Notifications
You must be signed in to change notification settings - Fork 12
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: address_format_update #1107
base: main
Are you sure you want to change the base?
feat: address_format_update #1107
Conversation
❌ Deploy Preview for dev-astral failed. Why did it fail? →
|
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.
Thank you for your contribution @varunguleriaCodes 🙇♂️
I left a few notes and changes request 👍
@@ -130,7 +146,7 @@ export const AccountList: FC = () => { | |||
).toString() | |||
} | |||
} | |||
|
|||
setUpdatedFilters(tempUpdatedFilters); |
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 goal of a useMemo, is to be a pure function and memorize the values, to prevent re-rendering. By setting a state inside the useMemo we lose the purpose of the memoization
@@ -256,6 +272,12 @@ export const AccountList: FC = () => { | |||
setIsVisible(inView) | |||
}, [inView, setIsVisible]) | |||
|
|||
useEffect(() => { |
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 don't believe we need this useEffect to achieve what we want
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.
Hi @marc-aurele-besner , I am unable to find a way to bypass this can you give some hint about how should I try 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.
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 realized the address I provided as example did not had any entry in mainnet and testnet (it was a address use on 3g or 3h only) so one way to convert any address to the st is to go on a address detail page, then in utils/formatAddress line 6 change SUBSPACE_ACC_PREFIX
for 2254
Fixes #1065
On entering the test case it converts it to the address mentioned in image