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

[DDW-667] Use token decimal places info from the cardano-wallet Api #2577

Conversation

nikolaglumac
Copy link
Contributor

@nikolaglumac nikolaglumac commented May 28, 2021

This PR integrates token decimal places info from the cardano-wallet Api.

Todos

  • Bump cardano-wallet to the latest release v2021-05-26
  • Use "recommended" setting as preselected value in the token configuration dialog in case user hasn't selected a value before
  • Keep token metadata in runtime memory even if the subsequent Api calls doesn't return metadata
  • Show zero balance tokens on the summary screen too [Will be done in a separate card: https://jira.iohk.io/browse/DDW-694]

Screenshots

Screenshot 2021-05-28 at 14 13 56
Screenshot 2021-05-28 at 14 13 50
Screenshot 2021-05-28 at 14 13 27
Screenshot 2021-05-28 at 14 13 37


Testing Checklist


Test Scenarios

Scenario 1 - Check recommended precision is preselected on settings

Given that I am on the "Summary" screen
And the wallet has a native token with a recommended decimal precision
And the precision for this token was never configured
When I click on the cog or warning icon to open the settings dialog
Then I can see that the recommended value is preselected on the dropdown

Scenario 2 - Check selected precision is persisted on settings

Given that I am on the "Summary" screen
And the wallet has a native token with a recommended decimal precision
And the precision selected for this token is different from the recommended
When I click on the cog or warning icon to open the settings dialog
Then I can see that the current decimal precision is preselected on the dropdown

Scenario 3 - Check default precision is preselected on settings

Given that I am on the "Summary" screen
And the wallet has a native token without a recommended decimal precision
And the precision for this token was never configured
When I click on the cog icon to open the settings dialog
Then I can see that the default value (0) is preselected on the dropdown

Scenario 4 - Check selected precision is persisted on settings

Given that I am on the "Summary" screen
And the wallet has a native token without a recommended decimal precision
And the precision for this token was updated
When I click on the cog icon to open the settings dialog
Then I can see that the current decimal precision is preselected on the dropdown

Scenario 5 - Check the token metadata is persisted if the API doesn't return data

Given that I am on the "Summary" screen
And the wallet has a native token
And the corresponding metadata was fetched before
When the API stops returning metadata
Then I can see that the previous metadata is saved for the token

Scenario 6 - Check the token metadata is persisted after restart if the API doesn't return data

Given that I am on the "Summary" screen
And the wallet has a native token
And the corresponding metadata was fetched before
When the API stops returning metadata
And I restart Daedalus
Then I can see that only the policy ID and the asset name are displayed for the token

Scenario 7 - Check Save button is disabled

Given that I am on the "Native Token Settings" dialog
And the Native Token has a configured precision
And the option selected in the dropdown is the already saved precision
When I check the "Save" option
Then I can see it is disabled

Scenario 8 - Check Save button is enabled

Given that I am on the "Native Token Settings" dialog
And the Native Token has a configured precision
When I select a different precision on the dropdown
And I check the "Save" button
Then I can see it is disabled

Scenario 9 - Check new precision is saved

Given that I am on the "Native Token Settings" dialog
When I select a different precision on the dropdown
And I click the "Save" button
Then I can see the token is displayed with the new decimal precision

Review Checklist

Basics

  • PR has been assigned and has appropriate labels (feature/bug/chore, release-x.x.x)
  • PR is updated to the most recent version of the target branch (and there are no conflicts)
  • PR has a good description that summarizes all changes
  • PR has default-sized Daedalus window screenshots or animated GIFs of important UI changes:
    • In English
    • In Japanese
  • CHANGELOG entry has been added to the top of the appropriate section (Features, Fixes, Chores) and is linked to the correct PR on GitHub
  • Automated tests: All acceptance and unit tests are passing (yarn test)
  • Manual tests (minimum tests should cover newly added feature/fix): App works correctly in development build (yarn dev)
  • Manual tests (minimum tests should cover newly added feature/fix): App works correctly in production build (yarn package / CI builds)
  • There are no flow errors or warnings (yarn flow:test)
  • There are no lint errors or warnings (yarn lint)
  • There are no prettier errors or warnings (yarn prettier:check)
  • There are no missing translations (running yarn manage:translations produces no changes)
  • Text changes are proofread and approved (Jane Wild / Amy Reeve)
  • Japanese text changes are proofread and approved (Junko Oda)
  • UI changes look good in all themes (Alexander Rukin)
  • Storybook works and no stories are broken (yarn storybook)
  • In case of dependency changes yarn.lock file is updated

Code Quality

  • Important parts of the code are properly commented and documented
  • Code is properly typed with flow
  • React components are split-up enough to avoid unnecessary re-renderings
  • Any code that only works in main process is neatly separated from components

Testing

  • New feature/change is covered by acceptance tests
  • New feature/change is manually tested and approved by QA team
  • All existing acceptance tests are still up-to-date
  • New feature/change is covered by Daedalus Testing scenario
  • All existing Daedalus Testing scenarios are still up-to-date

After Review

  • Merge the PR
  • Delete the source branch
  • Move the ticket to done column on the YouTrack board
  • Update Slack QA thread by marking it with a green checkmark

@dmitrii-gaico dmitrii-gaico removed their request for review May 28, 2021 12:54
@dmitrii-gaico
Copy link

Shouldn't we highlight the currently selected option ? For now we always highlight the Default

cc @alexander-rukin

Copy link
Contributor

@alexander-rukin alexander-rukin 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

@alexander-rukin
Copy link
Contributor

Shouldn't we highlight the currently selected option ? For now we always highlight the Default

cc @alexander-rukin

We should discuss it and fix it in a separate ticket. This may be a UX improvement for the whole app.

@nikolaglumac
Copy link
Contributor Author

Default

That's not a part of this PR.

@dmitrii-gaico dmitrii-gaico self-requested a review May 28, 2021 18:02
@nikolaglumac nikolaglumac merged commit 12a0c06 into develop May 29, 2021
@iohk-bors iohk-bors bot deleted the feature/ddw-667-use-token-decimal-places-info-from-the-cardano-wallet-api branch May 29, 2021 12:23
@nikolaglumac
Copy link
Contributor Author

@daniloprates I have merged this one but would still appreciate if you could take a look at it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature release-4.1.0-FC1 Daedalus Flight release-4.1.0 Daedalus Mainnet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants