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

Update RoleFormAssetSelector with drawer #2238

Merged
merged 10 commits into from
Aug 15, 2024

Conversation

mudrila
Copy link
Contributor

@mudrila mudrila commented Aug 13, 2024

Closes #2227

Figma Design

  • Update Stream asset selector to display "Asset drawer" and implement proper design
  • Also updated default coin icon
Screenshot 2024-08-13 at 19 38 34

@mudrila mudrila added the enhancement New feature or request label Aug 13, 2024
@mudrila mudrila self-assigned this Aug 13, 2024
Copy link

netlify bot commented Aug 13, 2024

Deploy Preview for decent-interface-dev ready!

Name Link
🔨 Latest commit ec733b6
🔍 Latest deploy log https://app.netlify.com/sites/decent-interface-dev/deploys/66be491877a2650008c9c1f5
😎 Deploy Preview https://deploy-preview-2238.app.dev.decentdao.org
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@adamgall adamgall left a comment

Choose a reason for hiding this comment

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

  1. Need some message for when the treasury is empty:
Screenshot 2024-08-13 at 11 56 42 PM
  1. idk if it's part of this PR or not, but this dropdown kind of looks like ass on desktop:
Screenshot 2024-08-13 at 11 56 03 PM

@mudrila mudrila requested a review from adamgall August 14, 2024 13:17
@mudrila
Copy link
Contributor Author

mudrila commented Aug 14, 2024

  1. Need some message for when the treasury is empty:
Screenshot 2024-08-13 at 11 56 42 PM 2. idk if it's part of this PR or not, but this dropdown kind of looks like ass on desktop: Screenshot 2024-08-13 at 11 56 03 PM

Yeah, empty message wasn't there before both for desktop and mobile - added one, check dat out pls

Copy link

@xraystyle1980 xraystyle1980 left a comment

Choose a reason for hiding this comment

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

Looking good, thanks team! A few requests:

  • Please update the border radius and hover value of the treasury menu asset bg (see screenshot)
    image
  • Proposed language for the "no assets found" message (see screenshot)
    image

@mudrila
Copy link
Contributor Author

mudrila commented Aug 14, 2024

Looking good, thanks team! A few requests:

  • Please update the border radius and hover value of the treasury menu asset bg (see screenshot)
    image
  • Proposed language for the "no assets found" message (see screenshot)
    image

@xraystyle1980 Thank you for the feedback! I'll update dropdown designs based off your comments.
As for copy update - I don't think that proposed copy works.

  1. Streaming of native(ETH/MATIC/etc.) tokens is not supported on Sablier side, so we can't do that technically.
  2. On the other side - any other token (not only stablecoin or DAOs voting token) can be streamed if it's present in treasury. For instance, Shutter DAO is considering streaming sDAI - which is itself not a stablecoin but DeFi token with generated yield.

@mudrila
Copy link
Contributor Author

mudrila commented Aug 14, 2024

@xraystyle1980 Updated design / copy based on feedback - though as I mentioned I didn't added the part with "Only native coins and stablecoins can be streamed" - it will only say No assets available for payments.

@mudrila mudrila requested a review from xraystyle1980 August 14, 2024 19:49
Copy link

@xraystyle1980 xraystyle1980 left a comment

Choose a reason for hiding this comment

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

More requests for the Asset dropdown: (apologies for not sharing earlier)

  • Can you mimic the pattern we have on the wallet menu? This means a smaller menu header and consistent top and bottom padding (on hover)
    image
    image

Copy link
Contributor

@Da-Colon Da-Colon 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. Can you format the balance?

image

Copy link
Contributor

@DarksightKellar DarksightKellar left a comment

Choose a reason for hiding this comment

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

lgtm, codewise

@mudrila
Copy link
Contributor Author

mudrila commented Aug 15, 2024

Looks good. Can you format the balance?

image

@Da-Colon I see that balance is already formatted 🤔
But seems like not in all the cases.

@mudrila
Copy link
Contributor Author

mudrila commented Aug 15, 2024

More requests for the Asset dropdown: (apologies for not sharing earlier)

  • Can you mimic the pattern we have on the wallet menu? This means a smaller menu header and consistent top and bottom padding (on hover)
    image
    image

@xraystyle1980 I can't find the design you're referring to - can you share the link to Figma? Meanwhile I'll try to follow pattern from wallet menu, but would highly appreciate design link as it will be faster

@mudrila
Copy link
Contributor Author

mudrila commented Aug 15, 2024

Looks good. Can you format the balance?
image

@Da-Colon I see that balance is already formatted 🤔 But seems like not in all the cases.

Done sir

@mudrila
Copy link
Contributor Author

mudrila commented Aug 15, 2024

More requests for the Asset dropdown: (apologies for not sharing earlier)

  • Can you mimic the pattern we have on the wallet menu? This means a smaller menu header and consistent top and bottom padding (on hover)
    image
    image

@xraystyle1980 I can't find the design you're referring to - can you share the link to Figma? Meanwhile I'll try to follow pattern from wallet menu, but would highly appreciate design link as it will be faster

@xraystyle1980
I've made adjustments you requested but the Dropdown still looks little bit off to me
Given there's no feedback on the drawer side - I'm gonna merge this PR, as it's scope initially was purely around Asset Drawer, but please share the design link (here or over Slack) - and I'll make any adjustments to the Dropdown needed

@mudrila mudrila dismissed stale reviews from Da-Colon and xraystyle1980 August 15, 2024 18:33

Changes are implemented

@mudrila mudrila merged commit 2069e5c into roles-0.2.0/streams Aug 15, 2024
7 checks passed
@mudrila mudrila deleted the roles-0.2.0/asset-drawer branch August 15, 2024 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Design Handoff | Payments] Asset Drawer
5 participants