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

fix(ConnectWalletForm): preserve errors on popup reopen #638

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

sidvishnoi
Copy link
Member

Context

Part of #613

Changes proposed in this pull request

Screencast.from.02-10-24.05.45.59.PM.IST.webm

@github-actions github-actions bot added area: background Improvements or additions to extension background script area: popup Improvements or additions to extension popup labels Oct 2, 2024
Copy link

github-actions bot commented Oct 2, 2024

Extension builds preview

Name Link
Latest commit a356a05
Latest job logs Run #11146038174
BadgeDownload
BadgeDownload

Copy link
Member

@raducristianpopa raducristianpopa left a comment

Choose a reason for hiding this comment

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

A little bit confused. So we will preserve the error even if the popup is opened multiple times?

@sidvishnoi
Copy link
Member Author

sidvishnoi commented Oct 2, 2024

Yes, so user knows what went wrong. For example, we open new tab, popup closes:

  • user closes tab, then we can show "you closed tab too soon" error message.
  • key add fails, user opens popup again, they connect again - we end up trying to add key over and over, as we lost context on popup reload.

We need that context (even if we don't show error message). These will be cleared when user successfully connects (or resets when user tries to connect again, to be replaced with newer error message); or when browser is closed.

Example when we won't show redirect page with error (but popup was closed on tab open):
image

don't retry adding key if failed before popup open

handle more issues on errors gone on reload

handle more issues on errors gone on reload
@sidvishnoi
Copy link
Member Author

sidvishnoi commented Oct 2, 2024

TODO: reset all error and state when wallet address is changed (or add a "reset" button somewhere in error message?)

@sidvishnoi sidvishnoi merged commit e41f23e into main Oct 2, 2024
9 checks passed
@sidvishnoi sidvishnoi deleted the connect-preserve-errors branch October 2, 2024 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: background Improvements or additions to extension background script area: popup Improvements or additions to extension popup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants