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-518] Tile view of stake pools ranked by rewards #2426

Conversation

DeeJayElly
Copy link
Contributor

@DeeJayElly DeeJayElly commented Mar 2, 2021

This PR updates Stake pools tile view with tiles ranked by rewards.

User story:

As a Daedalus user, I want to be able to see the pools in a user-friendly tile view, ranked descending according to potential reward yield so that I can have a complete overview and make potential educated decisions.

Scenario: Viewing the tile view in the Delegation Center

  • Given that I am in Daedalus stake pools page
  • When I click “Rewards tile view” icon
  • Then I am shown a tile view of the stake pools
  • And they are ranked in a descending order according to potential reward yield

Todos

  • Add sorting/ordering button to search toolbar of stake pools tile view
  • Add ranking number to tiles
  • Implement ordering by ranking
  • Update storybook for stake pools tile view
  • Display - instead of 0 ada when estimated rewards are 0
  • Make this Flight and Testnet only feature (configurable)

Screenshots

Screenshot 2021-03-05 at 19 17 56
Screenshot 2021-03-05 at 19 18 14
Screenshot 2021-03-05 at 19 18 04
Screenshot 2021-03-05 at 19 18 16


Testing Checklist


Test Cases

Scenario 1: Viewing the tile view in the Delegation Center
Given that I am in Daedalus stake pools page
When I click “Rewards tile view” icon
Then I am shown a tile view of the stake pools 
And they are ranked in a descending order according to potential reward yield

Scenario 2. Choosing rewards tile view when no wallets are loaded in Daedalus
Given that I am on Daedalus
And the chain folder is fully synced 
And I don’t have any wallets loaded
And I navigate to the Stake Pool Tab
And I click on the new Rewards Tile View Icon
When I change the value for how much I intend to delegate on the slider
Then the stake pools will be reordered to match my selection from highest to lowest

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

@DeeJayElly DeeJayElly requested a review from a team March 3, 2021 11:12
@DeeJayElly DeeJayElly changed the title DDW-518 - Tile view of stake pools ranked by rewards [DDW-518] - Tile view of stake pools ranked by rewards Mar 3, 2021
@DeeJayElly DeeJayElly removed the WIP label Mar 3, 2021
@DeeJayElly DeeJayElly changed the title [DDW-518] - Tile view of stake pools ranked by rewards [DDW-518] Tile view of stake pools ranked by rewards Mar 3, 2021
@ManusMcCole ManusMcCole self-requested a review March 3, 2021 12:45
Copy link

@ManusMcCole ManusMcCole left a comment

Choose a reason for hiding this comment

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

Looks great @DeeJayElly 👍

Copy link

@miorsufianiohk miorsufianiohk left a comment

Choose a reason for hiding this comment

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

Apart from my suggestion on the need for tooltips, LGTM. Well done @aleksandardjordjeviciohk 👍

@gabriela-ponce
Copy link

gabriela-ponce commented Mar 3, 2021

@aleksandardjordjeviciohk I found two small issues on build 16910:

  • The X button on the search bar is not working properly. Check this video.
  • Also, it overlaps the divider. Check this screenshot on another theme where is more visible.

@DeeJayElly
Copy link
Contributor Author

@aleksandardjordjeviciohk I found two small issues on build 16910:

  • The X button on the search bar is not working properly. Check this video.
  • Also, it overlaps the divider. Check this screenshot on another theme where is more visible.

@gabriela-ponce can you recheck now, both of these problems are fixed

@gabriela-ponce
Copy link

@aleksandardjordjeviciohk The mentioned issues are fixed. However, on the new tooltips, the translation is missing for the "clear" button. Check this screenshot.

@nikolaglumac
Copy link
Contributor

@aleksandardjordjeviciohk The mentioned issues are fixed. However, on the new tooltips, the translation is missing for the "clear" button. Check this screenshot.

@aleksandardjordjeviciohk please fix this 🙏

We already have translation for this here: https://github.com/input-output-hk/daedalus/blob/develop/source/renderer/app/i18n/locales/ja-JP.json#L929

@DeeJayElly
Copy link
Contributor Author

@aleksandardjordjeviciohk The mentioned issues are fixed. However, on the new tooltips, the translation is missing for the "clear" button. Check this screenshot.

@aleksandardjordjeviciohk please fix this 🙏

We already have translation for this here: https://github.com/input-output-hk/daedalus/blob/develop/source/renderer/app/i18n/locales/ja-JP.json#L929

@gabriela-ponce @nikolaglumac
It was wrongly referenced in the translations file, i have updated it with correct translation. Tnx for pointing it out.

@darko-mijic
Copy link
Contributor

The new view is not available in the latest Flight build.
image

@nikolaglumac
Copy link
Contributor

@darko-mijic all issues have been fixed now. Please review again 🙏

@gabriela-ponce
Copy link

gabriela-ponce commented Mar 8, 2021

@aleksandardjordjeviciohk I checked again this issue about the "clear" button and it still reproduces in grid and tile view (no longer reproduces on the new tile view). Check this video.
Also, the tooltip for the "clear" button is not visible on those views.

@miorsufianiohk
Copy link

There's a pipe symbol, on the very left hand side of the buttons when 'Reward view' is chosen which is inconsistent with other views. Tested on 17020
List view
List view
Reward view
Reward view
Tile view
Tile view

@nikolaglumac
Copy link
Contributor

There's a pipe symbol, on the very left hand side of the buttons when 'Reward view' is chosen which is inconsistent with other views. Tested on 17020
List view
List view
Reward view
Reward view
Tile view
Tile view

I believe the rewards view (having the pipe) is the correct one.
@alexander-rukin can you please confirm?

@nikolaglumac
Copy link
Contributor

@miorsufianiohk @gabriela-ponce I have fixed all the remaining issues. Please quickly check only those fixes and approve 🙏

@nikolaglumac nikolaglumac removed the WIP label Mar 8, 2021
@nikolaglumac nikolaglumac merged commit 403a4f8 into develop Mar 8, 2021
@iohk-bors iohk-bors bot deleted the feature/ddw-518-tile-view-of-stake-pools-ranked-by-rewards branch March 8, 2021 17:37
@alexander-rukin
Copy link
Contributor

There's a pipe symbol, on the very left hand side of the buttons when 'Reward view' is chosen which is inconsistent with other views. Tested on 17020
List view
List view
Reward view
Reward view
Tile view
Tile view

I believe the rewards view (having the pipe) is the correct one.
@alexander-rukin can you please confirm?

Yes -> The rewards view (having the pipe) is the correct one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants