SelfKeyFoundation / Identity-Wallet

Code for the SelfKey Identity Wallet
https://selfkeyfoundation.github.io/Identity-Wallet/
MIT License
58 stars 26 forks source link

Loans Listing+Details UI Fixes #2198

Closed oanamangiurea closed 4 years ago

oanamangiurea commented 4 years ago
  1. Change icon for calculator to the one from design. It’s already in the storybook https://gyazo.com/ad7246d39e4de167a3f28f312e471cd5
  2. Loan Calculator box https://gyazo.com/b3347919e4911b20d859de1ada5f7ae2:
    1. The box should be dismissible, in design I added a X icon to close it, and we need to remember that the user closed it
    2. Box padding should be: 30px 24px 60px;
    3. Icon padding should be: 0 24px 24px;
  3. Table filters: https://gyazo.com/52c49d423bf4e32529f28718b2cbc4fc
    1. Button group should look like a group, not like 2 buttons touching. I think the component is already in the storybook. The 2nd button should be “Centralized” not “Licensed”
    2. Current rates slider is missing the values for the range, plus it should be much wider, so we can fit those values
    3. Assets accepted dropdown it’s missing it’s placeholder “Choose…”
  4. Table Listing: https://gyazo.com/629cbf67f5072213ec242aae5fe2e575
    1. Table cell it’s missing top/bottom padding (issue raised as well for the material ui upgrade task)
  5. Details View: https://gyazo.com/7b36ded49afe7fcc4a0d967e21762fc8
    1. Title of the box, the grey text next to the loan provider should state the platform type, not say “Loans”
    2. Padding of the entire box is too big on the right, should have same padding bot left/right
    3. Button is too small, it’s container should be MuiGrid-grid-xs-4 from xs-3
    4. There’s an extra horizontal line before the tabs, it shouldn’t be there
    5. Assets should be separated by a comma
rhdelima commented 4 years ago

@oanamangiurea @andregoncalves Original issues were good on my end. However, I saw few more. Let me know if need to open a separate ticket.

  1. When opening Loans Marketplace, P2P is selected by default but listing shows ALL image

  2. Overlapped texts on Current Rates slider. See some screenshots below. image image

andregoncalves commented 4 years ago

Fix on PR review

Screenshot 2020-05-05 at 10 54 53
andregoncalves commented 4 years ago

Fix for overlapped markers on PR Review

oanamangiurea commented 4 years ago

Still some issues

  1. Buttongroup selected doesn't match styleguide in terms of background color, border color, round corners (only 1st and last child have round corners on the exterior). see https://projects.invisionapp.com/d/main#/console/15222689/317955444/preview
  2. When I use the range page starts to scroll - https://www.loom.com/share/113dc64885be44319bae28ae8a4b5d26 see from sec 0:10
  3. For calculator - loan amount - the 2 inputs are not vertically aligned - 1px issue https://gyazo.com/306e5d41de93982b4a06a268efd97c59
  4. Loan period slider is missing the range values
andregoncalves commented 4 years ago

Fixes are not merged yet.

oanamangiurea commented 4 years ago

Adding a few more fixes:

  1. Loans listings table - the small icon for provider should be like for the exchanges marketplace: 30x30px, border-radius 5px
  2. Remove custom padding form this padding: 5px 20px; it should be default like for all tables
  3. Dropdown arrow from assets accepted is misplaced, should be right: 15px;
  4. Button group - still has an issue with border-radius that is applied on all buttons. And the padding for it should be 0px 24px
  5. Dropdown from My Crypto should say "Other..." instead of "Choose" https://gyazo.com/f82eb94450f21c65461584003d174aa0
  6. For loan amount, I can't switch between USD and another currency la EUR
  7. The entire calculator is missing its container box (that has border and padding), see https://projects.invisionapp.com/d/main#/console/15222689/410372110/inspect
  8. Table from the calculator is missing vertical padding https://gyazo.com/334e1b2f29d3f2113b8d905033ff59f0. Should apply the standard one, so same as point 2
  9. For loans details page, can we write Max. instead of Maximum, it's a lot of text, I think it will just look easier to read https://gyazo.com/11269157b2cd0967b84aa30448e1b260
  10. Loans Details page - Requirements tab - text should be left-aligned, we don't use justified anywhere in the wallet + leave more space at the bottom, like for the first tab
rhdelima commented 4 years ago

@andregoncalves Some issues were not fixed.

  1. Dropdown arrow from assets accepted is misplaced, should be right: 15px;
  2. Button group - still has an issue with border-radius that is applied on all buttons. And the padding for it should be 0px 24px
  3. The entire calculator is missing its container box (that has border and padding), see https://projects.invisionapp.com/d/main#/console/15222689/410372110/inspect
  4. Loan calculator is misaligned image
  5. Loan period should start from 0 based on design.

@oanamangiurea Let me know if you agree.

andregoncalves commented 4 years ago

@designhorf are you working on 1. and 2. as part of the overall fixes, I'm using the selfkey-ui themed components there, so this is probably a global fix?

designhorf commented 4 years ago

@andregoncalves

  1. is fixed and PR was merged today.
  2. didn't touch it, so I'll create a ticket and fix it and will let you know if it's done.
andregoncalves commented 4 years ago

@rhdelima can you re-test with the latest build ? thanks.

rhdelima commented 4 years ago

@oanamangiurea Let me know if you are good with the fix. Also, when loan amount increases the width of the Loan Calculator increases as well. Ex. The table width of Loan Amount = 1000 is bigger than table width of Loan Amount = 100. See sample screenshot below

image

image

oanamangiurea commented 4 years ago

@rhdelima no, this is not ok, we shouldn't see any horizontal scroll in there. @designhorf or @andregoncalves we can decrease the padding of table cells from 15px 30px; to 15px; (all over) and please remove the overflow-x: scroll; from table container.

andregoncalves commented 4 years ago

Fixes for table scroll and table cell size are merged and ready for testing.