Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Fix #6965: Duplicate add network requests can be shown #7047

Merged
merged 3 commits into from
Mar 6, 2023

Conversation

StephenHeaps
Copy link
Contributor

Summary of Changes

  • Adding a new ethereum chain requires a network call, however we were not waiting for the response of attempting to add the new chain before dismissing the Add Network request modal. This could create a race condition where we still have the add network request pending when we are dismissing the Add Network request modal, so it would be shown again.
  • Added a loading state and error alert to SuggestedNetworkView. Error will dismiss the modal after as the add chain request is removed in core: https://github.com/brave/brave-core/blob/b31c48fff4b9aecdef95a9bba344f9329b3ab1e7/components/brave_wallet/browser/json_rpc_service.cc#L521
  • Added an optional completion closure to handleWebpageRequestResponse so SuggestedNetworkView knows when to start/stop loading, and can now show an error alert when failing to add the network

This pull request fixes #6965

Submitter Checklist:

  • Unit Tests are updated to cover new or changed functionality
  • User-facing strings use NSLocalizableString()
  • New or updated UI has been tested across:
    • Light & dark mode
    • Different size classes (iPhone, landscape, iPad)
    • Different dynamic type sizes

Test Plan:

  1. Visit Chainlist.org
  2. Connect your wallet and try adding a few chains.
  3. Verify each only shows 'add network' modal once before showing 'switch network' modal.
  4. Find Astar on Chainlist.org and try to add it.
  5. (assuming it still fails) Verify loading state is shown, and wait for timeout on the request.
  6. Verify closing the alert will dismiss the Add Network modal as the request is removed when error is received.

Screenshots:

Add Network Fixed:

add.network.request.mov

Add Network error alert (new):

add.chain.timeout.mov

Reviewer Checklist:

  • Issues include necessary QA labels:
    • QA/(Yes|No)
    • bug / enhancement
  • Necessary security reviews have taken place.
  • Adequate unit test coverage exists to prevent regressions.
  • Adequate test plan exists for QA to validate (if applicable).
  • Issue and pull request is assigned to a milestone (should happen at merge time).

…orkaround to add network dapp request; changes behaviour of bug when failing to add a network to get stuck in loading state instead of dismissing.
…ex. when failing to add network, waiting for timeout).
@StephenHeaps StephenHeaps added this to the 1.49 milestone Mar 2, 2023
@StephenHeaps StephenHeaps self-assigned this Mar 2, 2023
@StephenHeaps StephenHeaps marked this pull request as ready for review March 3, 2023 15:44
@StephenHeaps StephenHeaps requested a review from a team as a code owner March 3, 2023 15:44
@StephenHeaps StephenHeaps requested a review from nuo-xu March 3, 2023 15:44
…nary for each chain_id incase we received a new Add Chain request while one is loading.
@StephenHeaps StephenHeaps requested a review from nuo-xu March 6, 2023 18:29
@StephenHeaps StephenHeaps merged commit 73e08b9 into development Mar 6, 2023
@StephenHeaps StephenHeaps deleted the wallet/add-network-dapp-bug branch March 6, 2023 23:59
arthuredelstein pushed a commit to brave/brave-core that referenced this pull request Feb 13, 2024
…brave/brave-ios#7047)

* Add loading state to `SuggestedNetworkView`. Add completion closure workaround to add network dapp request; changes behaviour of bug when failing to add a network to get stuck in loading state instead of dismissing.
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to add custom network via panel prompt
2 participants