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

Update Portfolio data and cache with SWR #1047

Merged
merged 7 commits into from
Apr 20, 2023
Merged

Conversation

ladytrekker
Copy link
Contributor

@ladytrekker ladytrekker commented Apr 18, 2023

Description

Please test that the following works:

Connect

  • Go to your Portfolio on the preview link
  • You should see the LoginCard ✔️
  • When you connect your wallet, you should see a loading spinner ✔️
  • When your wallet is connected, you see your assets correctly displayed ✔️
  • If you do not have any assets, you should see an empty state message ✔️

Create a Listing

  • Click on "Sell" of one your assets
  • Create a Listing with one of your assets
  • Wait until the success modal appears => CLOSE THIS MODAL ! (Going to the Profile is not in scope of this PR)
  • You should see now two loadings spinners with "Loading your assets" ✔️
  • The frontend polls the API to wait until the backend updated your user data with the newly created listing (interval: 1 second, max attempts: 50)
  • When the max attempts have been reached, an error message appears "please refresh the page" ✔️
  • When the polling succeeded with the updated data, your assets should be updated correctly ✔️
  • When the polling succeeded with the updated data, your activity data should show your new listing ✔️

Related Ticket

Part of #1019

Changes

This PR contains additional refactorings:

  • Extract the Assets components from the main index page
  • Put an overlay above all assets when the polling starts to ensure that the user is not changing anything while pending

Bildschirm­foto 2023-04-18 um 15 53 51

Checklist

  • I have run npm run build-all without errors
  • I have formatted JS and TS files with npm run format-all

@vercel
Copy link

vercel bot commented Apr 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
carbonmark ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 18, 2023 3:13pm
klimadao-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 18, 2023 3:13pm
klimadao-site ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 18, 2023 3:13pm

Copy link
Collaborator

@Atmosfearful Atmosfearful left a comment

Choose a reason for hiding this comment

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

code looks awesome 💯 (haven't hands-on tested yet)

@ShimonD-KlimaDAO
Copy link

Works well. Two minor comments:

  1. You should see now two loadings spinners with "Loading your assets" ✔️

    "Loading your assets" is missing from the Activities spinner. According to the testing instructions, that shouldn't be the case.
    Not sure what is expected in general.
    image

  2. Minor edge case:
    -Log in
    -Create a listing, close success modal
    -While the spinners are spinning, log out.
    -Log in again -> wrong info is loaded.
    -Refresh after waiting a bit -> correct info.

    Not sure if this is practical to resolve, since we probably don't want to poll the API for 50 seconds every time the user logs in.

@ladytrekker
Copy link
Contributor Author

"Loading your assets" is missing from the Activities spinner.

Sorry, the description wasn't explicit. Activity Spinner has no label.

Not sure if this is practical to resolve, since we probably don't want to poll the API for 50 seconds every time the user logs in.

Yeah, the backend wasn't ready yet and returned the old data on page load.
As I do not think that people would often leave the page during progressing the data, I would leave it as such.

@Atmosfearful Atmosfearful merged commit f198fff into staging Apr 20, 2023
@Atmosfearful Atmosfearful deleted the lady/swr-portfolio branch April 20, 2023 18:21
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.

3 participants