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.

Sign up for GitHub

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

Jump to bottom

fix: Filter zapable token for wallet #688

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix: Filter zapable token for wallet #688

wants to merge 1 commit into from

Conversation

huggingbot
Copy link
Contributor

@huggingbot huggingbot commented May 19, 2022 •
edited
Loading

Description

  • Added filter to filter only zapable tokens
  • Checked that sUSD is not zapable in yearn-sdk
  • Checked that sUSD yVault is not deprecated

Related Issue

Motivation and Context

  • Improve user experience by not showing unzapable vaults

How Has This Been Tested?

  • Locally by confirming sUSD not appearing in the wallet container

Screenshots (if appropriate):

AddressesGenerator_VAULT_V2_0x437758d475f70249e03eda6be23684ad1fc375f0

  • sUSD token not showing anymore

susd

@aws-amplify-us-east-1
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-688.d27dgpz01hmbvx.amplifyapp.com

karelianpie
karelianpie reviewed May 21, 2022
View reviewed changes
src/client/routes/Portfolio/index.tsx
invest: null,
}))}
data={userTokens
.filter((token) => token.isZapable)
Copy link
Member

Choose a reason for hiding this comment

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

I think we will deprecate this property very soon

karelianpie
karelianpie requested changes May 21, 2022
View reviewed changes
Copy link
Member

@karelianpie karelianpie left a comment

Choose a reason for hiding this comment

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

We will be using the new zap properties, which will most likely deprecate the isZapable property.

@huggingbot
Copy link
Contributor Author

huggingbot commented May 21, 2022 •
edited
Loading

We will be using the new zap properties, which will most likely deprecate the isZapable property.

I assume the new prop for the TokenView type will be sourceIsZapper. Could it just replace isZapable? Because I realize there are tokens where their isZapable is true, but their sourceIsZapper is false. Thanks

@karelianpie
Copy link
Member

We will be using the new zap properties, which will most likely deprecate the isZapable property.

I assume the new prop for the TokenView type will be sourceIsZapper. Could it just replace isZapable? Because I realize there are tokens where their isZapable is true, but their sourceIsZapper is false. Thanks

No, sourceIsZapper will probably be deprecated as well. We're changing the whole way this is working, it's currently in progress as I'm working on the frontend bit as we speak, we'll most likely use the supported prop which in turn has zapperZapIn/zapperZapOut/ftmApeZap and other zaps. Thanks for having a look at this @huggingbot but let's have this on hold for the time being.

@xgambitox xgambitox added the hold Hold pull request label May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karelianpie karelianpie karelianpie requested changes

@turtlemoji turtlemoji turtlemoji approved these changes

@FiboApe FiboApe FiboApe approved these changes

@xgambitox xgambitox Awaiting requested review from xgambitox

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
hold Hold pull request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

fix: susd modal not displaying
5 participants
@huggingbot @karelianpie @turtlemoji @FiboApe @xgambitox