Skip to content
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

Route to upgrade sub page based on token balance #66

Closed
wants to merge 4 commits into from

Conversation

georgeweiler
Copy link
Contributor

No description provided.

@github-actions
Copy link

src/hooks/useRouteByKeepNuBalance.ts Outdated Show resolved Hide resolved
src/hooks/useRouteByKeepNuBalance.ts Outdated Show resolved Hide resolved
@github-actions
Copy link

@github-actions
Copy link

@r-czajkowski r-czajkowski self-requested a review January 28, 2022 10:08
Copy link
Contributor

@r-czajkowski r-czajkowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hook redirects after successful upgrading KEEP/NU to T because the balances changes.

Scenario 1

  • I have KEEP and NU tokensKEEP balance > NU balance
  • I upgraded NU to T
  • dapp redirected me to the upgrade/keep

^ There is the same issue when I have KEEP balance < NU balance and want to upgrade from KEEP to T

Scenario 2

  • I have only KEEP. NU balance = 0
  • I upgraded all myKEEP tokens to T
  • dapp redirected me to the upgrade/nu

I leave final approval to @Battenfield.

@r-czajkowski r-czajkowski self-requested a review January 28, 2022 10:09
Comment on lines +22 to +27
const isFirstRender = useRef(true)

useEffect(() => {
if (isFirstRender.current) {
isFirstRender.current = false
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this flag is unnecessary because:

  • useEffect will be triggered on first render,
  • a condition in the first if statement is met- we set the isFirstRender to false,
  • so in a second if statement the !isFirstRender.current condition always be true

Maybe I missed something but I tested locally and it looks like if(active) is enough.

@georgeweiler
Copy link
Contributor Author

closing, as Chris asked for #77 instead

@georgeweiler georgeweiler deleted the route-by-account-balance branch September 12, 2022 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants