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

Frontend desktop support for running DiscoverAssets on page refresh #26547

Closed
nvonpentz opened this issue Nov 4, 2022 · 3 comments · Fixed by brave/brave-core#16329
Closed
Assignees
Labels
feature/web3/wallet Integrating Ethereum+ wallet support front-end-change This task is a front end task and doesn't need any C++ changes OS/Desktop QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/include

Comments

@nvonpentz
Copy link
Member

nvonpentz commented Nov 4, 2022

Background

Backend support issue - #25820.

The backend will provide a DiscoverAssetsOnAllSupportedChains API to the frontend that it can call to run the discover assets logic when the user reloads the wallet UI. The API will look like this with with no callback:

interface BraveWalletService {
  DiscoverAssetsOnAllSupportedChains();
}

Upon completion/failure, the backend will emit an event for each chain.

interface BraveWalletServiceObserver {
  // Fired when a discovered asset query completes
  OnDiscoverAssetsCompleted(array<BlockchainToken> discovered_assets);
};

If it's useful for the frontend, the backend can also emit an event when discovery logic completes for all supported chains.

Spec

More input needed to determine the the specifics, but the frontend implementation should be something like this --

  1. Call this API when appropriate (on-refresh)
  2. Display some UI to the user indicating that the asset discovery has running
  3. Indicate to the user when that has completed
@nvonpentz nvonpentz added QA/Yes release-notes/include feature/web3/wallet Integrating Ethereum+ wallet support OS/Desktop front-end-change This task is a front end task and doesn't need any C++ changes labels Nov 4, 2022
@jamesmudgett
Copy link

We should use one single row "loading skeleton" at the bottom of any list on load and until the final completion/failure call has been executed.

@rebron rebron added this to Web3 Nov 4, 2022
@jamesmudgett jamesmudgett moved this to In Progress in Web3 Nov 11, 2022
@jamesmudgett jamesmudgett moved this from In Progress to In Review in Web3 Nov 11, 2022
@yrliou yrliou removed the status in Web3 Nov 11, 2022
@nvonpentz
Copy link
Member Author

@jamesmudgett should the OnDiscoverAssetsCompleted event the frontend receives contain data on whether the calls succeeded or failed, or just that they are complete?

@jamesmudgett jamesmudgett moved this to Backlog in Web3 Nov 18, 2022
@Douglashdaniel Douglashdaniel self-assigned this Dec 12, 2022
@Douglashdaniel Douglashdaniel moved this from Backlog to In Progress in Web3 Dec 12, 2022
@Douglashdaniel Douglashdaniel moved this from In Progress to In Review in Web3 Dec 12, 2022
@nvonpentz nvonpentz changed the title Frontend support for running DiscoverAssets on page refresh Frontend desktop support for running DiscoverAssets on page refresh Dec 13, 2022
Repository owner moved this from In Review to Done in Web3 Dec 13, 2022
@brave-builds brave-builds added this to the 1.48.x - Nightly milestone Dec 13, 2022
@srirambv
Copy link
Contributor

Verification passed on

Brave 1.48.141 Chromium: 109.0.5414.119 (Official Build) beta (64-bit)
Revision 772095164c7d5d4e73160f858efed3b5e87eca83-refs/branch-heads/5414@{#1458}
OS Linux
  • Verified steps from brave/brave-core#16329
  • Verified loading skeleton is shown at the bottom of the asset list on both portfolio page and account page
26547.mp4

Verification passed on

Brave 1.48.141 Chromium: 109.0.5414.119 (Official Build) beta (64-bit)
Revision 772095164c7d5d4e73160f858efed3b5e87eca83-refs/branch-heads/5414@{#1458}
OS Windows 11 Version 22H2 (Build 22621.755)
  • Verified steps from brave/brave-core#16329
  • Verified loading skeleton is shown at the bottom of the asset list on both portfolio page and account page
26547.mp4

Verification passed on

Brave 1.48.141 Chromium: 109.0.5414.119 (Official Build) beta (arm64)
Revision 772095164c7d5d4e73160f858efed3b5e87eca83-refs/branch-heads/5414@{#1458}
OS macOS Version 13.0 (Build 22A380)
  • Verified steps from brave/brave-core#16329
  • Verified loading skeleton is shown at the bottom of the asset list on both portfolio page and account page
26547.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/web3/wallet Integrating Ethereum+ wallet support front-end-change This task is a front end task and doesn't need any C++ changes OS/Desktop QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/include
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants