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.

Sign up for GitHub

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

Jump to bottom

feat: refresh balance #116

Merged
merged 15 commits into from
Apr 1, 2022
Merged

feat: refresh balance #116

merged 15 commits into from
Apr 1, 2022

Conversation

CarmitKl
Copy link

@CarmitKl CarmitKl commented Mar 31, 2022 •
edited by liorgold2
Loading

Add two refresh buttons: 1 at the Transfer menu- to refresh the balance of the selected token.
Another in the SelectToken menu- to refresh the balance of all tokens.

Checklist

  • Changes have been done against master branch, and PR does not conflict
  • New unit / functional tests have been added (whenever applicable)
  • Test are passing in local environment
  • Docs have been updated
  • PR title is follow the standard-version convention: <type>(optional subject): <description>, e.g: fix: minor typos in code

This change is Reviewable

@CarmitKl CarmitKl changed the base branch from dev to alpha March 31, 2022 11:53
@CarmitKl CarmitKl requested a review from dan-ziv March 31, 2022 11:53
@CarmitKl CarmitKl added the ready for review Pull request is ready to be reviewed label Mar 31, 2022
dan-ziv
dan-ziv requested changes Mar 31, 2022
View reviewed changes
Copy link
Collaborator

@dan-ziv dan-ziv left a comment

Choose a reason for hiding this comment

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

@CarmitKl I see the next following issues:

  1. No loading spinner appears when clicking on refresh. So it looks like there's no reaction from the operation.
  2. Wrong values for tokens in the Select Token menu.

@CarmitKl CarmitKl added in progress Pull request is still a work in progress and removed ready for review Pull request is ready to be reviewed labels Mar 31, 2022
Carmit Kleinik added 2 commits March 31, 2022 16:13
PR fixes: place the refresh buttons differently in the markup; call u…
f3a1a9e 
…pdateTokenBalance without another mediator function; add the RefreshButton comp to UI index;
updateTokenBalance: add arg 'shouldShowLoading', if true (like from R…
4f99197 
…efresh-btn), won't skip setting 'isLoading:true'
@CarmitKl CarmitKl requested a review from dan-ziv March 31, 2022 15:13
@CarmitKl CarmitKl added ready for review Pull request is ready to be reviewed and removed in progress Pull request is still a work in progress labels Mar 31, 2022
@dan-ziv dan-ziv changed the title feat: refresh balance buttons feat: refresh balance Mar 31, 2022
@dan-ziv dan-ziv merged commit e2dd5f0 into alpha Apr 1, 2022
@dan-ziv dan-ziv deleted the feat/refreshBalanceButtons branch April 1, 2022 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dan-ziv dan-ziv Awaiting requested review from dan-ziv

Assignees
No one assigned
Labels
ready for review Pull request is ready to be reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@CarmitKl @dan-ziv