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: state corruption of import accounts drawer #6817

Merged
merged 3 commits into from
May 7, 2024

Conversation

woodenfurniture
Copy link
Member

Description

Addresses several state corruption issues related to import accounts drawer in account management, namely state being rendered for the previously opened chain.

Key changes:

  1. Migrates the account import queries to useInfiniteQuery in order to simplify state management
  2. Resets component state when a different chain is being managed
  3. Uses an actually unique key for account rows (instead of account number which is shared across different chains)

Pull Request Type

  • 🐛 Bug fix (Non-breaking Change: Fixes an issue)
  • 🛠️ Chore (Non-breaking Change: Doc updates, pkg upgrades, typos, etc..)
  • 💅 New Feature (Breaking/Non-breaking Change)

Issue (if applicable)

NA

Risk

High Risk PRs Require 2 approvals

Low risk.

What protocols, transaction types or contract interactions might be affected by this PR?

Testing

Check that navigating to different chains in the "import accounts" drawer renders the correct toggled state of the accounts.

Engineering

Operations

Screenshots (if applicable)

Demo showing corrected state where toggles are actually representative of chain being managed:

Screen.Recording.2024-05-04.at.12.52.33.PM.mov

@woodenfurniture woodenfurniture requested a review from a team as a code owner May 4, 2024 02:54
@woodenfurniture woodenfurniture changed the base branch from develop to account-toggles-use-toggles May 4, 2024 02:54
Copy link
Contributor

@gomesalexandre gomesalexandre left a comment

Choose a reason for hiding this comment

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

Looks sane to me conceptually - added two smallish non-blocking comments.

Tested at runtime and confirmed this does what this says on the box:

  • "Load More" is still happy
  • Toggling state is properly decoupled per chain
image image image image

Base automatically changed from account-toggles-use-toggles to develop May 7, 2024 01:30
@woodenfurniture woodenfurniture enabled auto-merge (squash) May 7, 2024 01:37
@woodenfurniture woodenfurniture merged commit 0c2f821 into develop May 7, 2024
3 checks passed
@woodenfurniture woodenfurniture deleted the fix-toggle-state-corruption branch May 7, 2024 01:42
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.

2 participants