Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

[CoW Subsidy] Connect modal and add comp to swap mod #2551

Merged
merged 4 commits into from
Mar 17, 2022
Merged

[CoW Subsidy] Connect modal and add comp to swap mod #2551

merged 4 commits into from
Mar 17, 2022

Conversation

W3stside
Copy link
Contributor

Summary

Part of and #2549

Connects more data to modal and adds Fee discount comp to swap mod. Clickable to open

image

@W3stside W3stside requested review from a team March 16, 2022 13:31
@W3stside
Copy link
Contributor Author

prob with builds here and #2549

@elena-zh
Copy link

Seems that the build has failed.

@W3stside W3stside mentioned this pull request Mar 16, 2022
Merged
8 tasks
@github-actions
Copy link
Contributor

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

@elena-zh
Copy link

Hey @W3stside !

Nice changes! And here are my early issues:

  1. I think, it would be nice not to show this button to a not connected user
    image
    image

  2. Then, it would be great to display discount modal nicer in a mobile view (without blurred effect on the right side)
    image

  3. Maybe, we shouldn't show this fee discount when a user does not have (v)COW balance in the wallet?
    image

  4. In a real mobile device, when I try opening the Fee discount tooltip, the modal is opened:
    image

Thanks!

@W3stside
Copy link
Contributor Author

Hey @W3stside !

Nice changes! And here are my early issues:

  1. I think, it would be nice not to show this button to a not connected user
    image
    image

@gnosis/gp-frontend thoughts? i think we should always show. 0 vcow? show. Not connected? Show

  1. Then, it would be great to display discount modal nicer in a mobile view (without blurred effect on the right side)
    image

@fairlighteth will handle this stuff, could we make a note of it somewhere? thanks!

  1. Maybe, we shouldn't show this fee discount when a user does not have (v)COW balance in the wallet?
    image

@gnosis/gp-frontend I think we should show it even then

  1. In a real mobile device, when I try opening the Fee discount tooltip, the modal is opened:
    image

@fairlighteth also mentioned this, good point. Could we add this to the style fixes notes?

Thanks!

@W3stside W3stside mentioned this pull request Mar 17, 2022
Merged
@W3stside
Copy link
Contributor Author

@elena-zh merging as consensus on slack for the showing was reached. Could you create an issue for @fairlighteth to style the other issues you found? thanks!

@W3stside
Copy link
Contributor Author

anyone else from @gnosis/gp-frontend with concerns? as this is a waterfall and @elena-zh already documented issues i will merge this, please post review if u need or just wait for consolidation

@W3stside W3stside merged commit b7effe2 into cow-fee-subsidy Mar 17, 2022
@alfetopito alfetopito deleted the cow-fee-subsidy-connected branch March 17, 2022 18:46
This was referenced Mar 18, 2022
Closed
Closed
@elena-zh
Copy link

Hey @W3stside , I have created separate issues #2560 and #2561 for the UI cases I reported above.
Returning back to the question about displaying modal to a not connected user/user with no balance, then I should say that at least we should show 0% discount, and 0 balance in the modal:
image
image

Let me know please If I need to create a separate issue for this.
Thanks!

@anxolin
Copy link
Contributor

anxolin commented Mar 18, 2022

https://github.com/orgs/gnosis/teams/gp-frontend thoughts? i think we should always show. 0 vcow? show. Not connected? Show

I would show always.
For the balance, I think we can show 0 (and we could add a message saying, connect your wallet to check your discount). Alternativelly, replace the balance with the message.

@anxolin
Copy link
Contributor

anxolin commented Mar 18, 2022

@W3stside this line doesn't make too much sense to me. >0 u get 0?

image

@elena-zh
Copy link

@anxolin , it is fixed in #2558 PR :)
image

W3stside added a commit that referenced this pull request Mar 22, 2022
@W3stside
[CoW Subsidy] Base PR (#2549)
9b55468 
* CoW subsidy modal + sub components

* force commit

* [CoW Subsidy] Connect modal and add comp to swap mod (#2551)

* revert !account test check

* add cow subsidy modal type to state

* add component to swapmod

* use AutoRow

* [Cow Subsidy] - Get user's (v)COW balance fn + tests (#2557)

* create discount utl fn & tests

* move constants to file closer to subsidy code

* minor changes to SubsidyTable

* fix broken path

* PR comments - ease up logic and data

* [CoW Subsidy] - Highlight current subsidy relative to CoW balance and show correct discount on SwapMod (#2558)

* add selected tier styling to table

* change return of util

* pass tier (mock) to modal

* more serious hook impl.

* change type to subsidy and use hook

* fix test

* use atom units for better math

- simpler for..of loop
- format table units down

* better tests and fix logic

* type change and minor logic lean

* fix test tier with lt change

* lt inclusive in loop

* fix typo

* dont show empty container

* hide balance on !account
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alfetopito alfetopito alfetopito approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@W3stside @elena-zh @anxolin @alfetopito