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

Update Balance screen #605

Merged
merged 1 commit into from
Jul 17, 2023
Merged

Update Balance screen #605

merged 1 commit into from
Jul 17, 2023

Conversation

erdemyerebasmaz
Copy link
Collaborator

@erdemyerebasmaz erdemyerebasmaz commented Jul 17, 2023 •
edited
Loading

This PR partially addresses

Changelist:

  • Converted WalletDashboard and it's child widgets to StatefulWidget's.

Notes:

@roeierez Haven't debugged it yet on the SDK side but my initial impression from testing on UI is this is a asynchronous communication issue calling sync before payment is persisted in the db.

@erdemyerebasmaz erdemyerebasmaz added bug Something isn't working sdk labels Jul 17, 2023
@erdemyerebasmaz erdemyerebasmaz added this to the v0.1.1-alpha milestone Jul 17, 2023
@erdemyerebasmaz erdemyerebasmaz self-assigned this Jul 17, 2023
@roeierez
Copy link
Member

@erdemyerebasmaz From my research this is not the case as the sdk only returns from payment after it syncs.
Also found an issue in greenlight regarding that and is under Christian's investigation.

@erdemyerebasmaz erdemyerebasmaz marked this pull request as ready for review July 17, 2023 11:01
@erdemyerebasmaz
Copy link
Collaborator Author

erdemyerebasmaz commented Jul 17, 2023 •
edited
Loading

@roeierez Thanks, saw your discussion now. Either way, this refactoring is a plus and is more alike to breezmobile codebase. The differences were introduced in

Changelist:

  • Converted WalletDashboard and it's child widgets to StatefulWidget's.
  • All child widgets use Bloc states from a single source,
  • Extracted fiat balance text into it's own widget.

ubbabeck
ubbabeck approved these changes Jul 17, 2023
View reviewed changes
Copy link
Contributor

@ubbabeck ubbabeck left a comment

Choose a reason for hiding this comment

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

Looks good! Nice refactor 👍
Tested receive and fiat currency flow on an android emulator, did not notice any issues.

@erdemyerebasmaz erdemyerebasmaz merged commit b2f2c29 into main Jul 17, 2023
1 check passed
@erdemyerebasmaz erdemyerebasmaz deleted the 604-balance-update branch September 27, 2023 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ubbabeck ubbabeck ubbabeck approved these changes

@ademar111190 ademar111190 Awaiting requested review from ademar111190

@roeierez roeierez Awaiting requested review from roeierez

Assignees

@erdemyerebasmaz erdemyerebasmaz

Labels
bug Something isn't working sdk
Projects
None yet
Milestone
v0.1.1-alpha
Development

Successfully merging this pull request may close these issues.
3 participants
@erdemyerebasmaz @roeierez @ubbabeck