Make specific warnings for adding custom asset #3478
Conversation
| assetData?.mainCurrencyAmount !== undefined && | ||
| assetData?.mainCurrencyAmount < userValueDustThreshold | ||
|
|
||
| const showWarningAboutNoBalance = | ||
| !showWarningAboutDust && assetData?.mainCurrencyAmount === 0 |
There was a problem hiding this comment.
Do we want to show this warning only if hide dust is enabled and display the visibility message?
There was a problem hiding this comment.
In fact, this message should be displayed when the balance is zero.
https://github.com/tahowallet/extension/assets/23117945/2fee5d65-33d1-4483-a573-7614e9eedfbb
There was a problem hiding this comment.
Sorry I missed a word, comment should've said "Do we want to show this warning only if hide dust is enabled and display the visibility message otherwise?"
My question was: if hide dust is not enabled should we display "Asset already exists, you should see it in your asset list "?
There was a problem hiding this comment.
And if its balance is not zero then yes.
There was a problem hiding this comment.
Ah I see! We should check that setting's value here too then. In the video, after disabling hide dust, the same warning shows up.
ui/_locales/en/messages.json
Outdated
| "warning.alreadyExists.desc": { | ||
| "dust": "Your balance is below {{sign}}{{amount}}. You will see it once you turn off “{{settings}}” in Settings.", | ||
| "noBalance": "Your balance is 0, you will see it once your balance is over {{sign}}{{amount}}.", | ||
| "visibility": "You should see in it in your asset list." |
There was a problem hiding this comment.
| "visibility": "You should see in it in your asset list." | |
| "visibility": "You should see it in your asset list." |
| {showWarningAboutDust && | ||
| t("warning.alreadyExists.desc.dust", { | ||
| ...warningOptions, | ||
| settings: sharedT( | ||
| "settings.hideSmallAssetBalance", | ||
| warningOptions | ||
| ), | ||
| })} | ||
| {showWarningAboutNoBalance && | ||
| t("warning.alreadyExists.desc.noBalance", warningOptions)} | ||
| {showWarningAboutVisibility && | ||
| t("warning.alreadyExists.desc.visibility")} |
There was a problem hiding this comment.
We can move these to a function and structure by priority with either a switch statement or early returns, this would also allow us to get rid of the !showOtherWarning checks.
e.g.
const renderWarningText = () => {
const showWarningAboutDust = ...something
if (showWarningAboutDust) {
return t("warning.alreadyExists.desc.dust", {
...warningOptions,
settings: sharedT("settings.hideSmallAssetBalance", warningOptions),
})
}
// showWarningAboutNoBalance
if (hideDustEnabled && ...) {
return t("warning.alreadyExists.desc.noBalance", warningOptions)
}
// showWarningAboutVisibility
return t("warning.alreadyExists.desc.visibility")
}
|
After changing the copy, there was a problem with the right padding. IMO, it didn't look good. @VladUXUI Could you please confirm if 24 px is fine? Before After |
|
24px looks good @kkosiorowska ! |
## What's Changed * Repeater Connection: Forward requests to new default when default is switched off during dApp connection flow by @Shadowfiend in #3462 * Auto-Not-So-Matic: Fix two matic.network URLs to polygon.technology by @Shadowfiend in #3483 * v0.38.0 by @kkosiorowska in #3480 * Case Dismissed: Forcibly show DAppConnectionInfoBar popover on first dApp connection by @Shadowfiend in #3464 * Add Hardhat Fork Functionality by @0xDaedalus in #3247 * Faded Jeans: Rename fadeIn class to fade_in by @Shadowfiend in #3485 * Fix issue for discovery transaction hash by @kkosiorowska in #3458 * Full Sweep: Drop the USE_UPDATED_SIGNING_UI feature flag by @Shadowfiend in #3475 * Token Discovery - Remap redux asset balances by @hyphenized in #3195 * Make specific warnings for adding custom asset by @kkosiorowska in #3478 * Run NFTs e2e tests on a controlled wallet address by @michalinacienciala in #3487 * v0.38.1 by @jagodarybacka in #3484 **Full Changelog**: v0.38.1...v0.39.0 Latest build: [extension-builds-3496](https://github.com/tahowallet/extension/suites/13792957673/artifacts/764738599) (as of Thu, 22 Jun 2023 14:27:32 GMT).
Closes #3325
What
We should display more detailed warnings to the user when adding assets. Users are trying to add an asset that already exists, this can be for 1 of 3 reasons:
1 - They have 0 balance so they don't see it
2 - They only have dust, need to toggle off "hide assets"
3 - Asset is already on the list but they didn't notice it.
UI
Before
After
Testing
Check that the messages are displayed correctly:
Hide asset balances under $2is onHide asset balances under $2is off0x841fad6eae12c286d1fd18d1d525dffa75c7effe0x841fad6eae12c286d1fd18d1d525dffa75c7effeLatest build: extension-builds-3478 (as of Tue, 20 Jun 2023 16:21:31 GMT).