[Spam tokens] manually hide tokens #1377

schmanu · 2022-12-15T11:22:37Z

What it solves

Unwanted / spam tokens could not be hidden from the interface.

Resolves #1337

How this PR fixes it

New button in Assets page to manually hide / unhide tokens
new parameter for useBalances to get all or only not hidden tokens

How to test it

Hide some tokens in the Assets page of a safe
These tokens should be hidden for all Safes on that chain
- Not appear in assets
- not counted in total balance
- not appear through safe apps sdk getBalance
- not show in Dashboard Overview

Analytics changes

New events for hiding / unhiding tokens
We decided not to track which token addresses get hidden

Screenshots

github-actions · 2022-12-15T11:22:51Z

Branch preview

✅ Deploy successful!

https://spam_tokens_manually_hide--webcore.review-web-core.5afe.dev

github-actions · 2022-12-15T11:25:21Z

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	0	0
Ignored	0	N/A

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

katspaugh

I would suggest rethinking the terminology. E.g. UserTokenBlocklist instead of HiddenAssets.

src/hooks/useBalances.ts

src/store/hiddenAssetsSlice.ts

src/components/balances/HiddenAssetsProvider/index.tsx

- remove SelectDialog => add ToggleButton - change icon order for hiding / unhiding - improve sticky TokenMenu positioning - make buttons the same size in TokenMenu

- Moves hiddenAssetsSlice into settingsSlice - renames variables / hooks - moves TokenMenu inside of AssetsTable - adjusts tests

github-actions · 2022-12-27T09:15:30Z

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	0	0
Ignored	0	N/A

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

- show text in toggle button instead of tooltip - rename TokenListSelect to HiddenTokenToggle - increase size of hide token / show token button in assetTable

- click on IconButton immediately hides token with collapse animation - rename apply -> save - header button will only open menu but not toggle it - in hide token menu all tokens are shown, not just the hidden ones - checkboxes when in hide token menu instead of eye-icon - add Deselect all button

- New property collapsed for rows of EnhancedTable - collapsed rows will be collapsed by wrapping it's Cells in Collapse - fixes tests

src/components/balances/AssetsTable/index.tsx

katspaugh

As you TODO says, there's a lot of new logic inside the AssetsTable that would be good to extract into a module dedicated to token hiding.

src/components/balances/AssetsTable/index.tsx

src/hooks/useBalances.ts

src/components/balances/TokenMenu/styles.module.css

src/hooks/useBalances.ts

src/services/analytics/events/assets.ts

- reverts useBalances changes - adds useVisibleBalance - does not change the fiatTotal anymore - hidden balances only have an effect on AssetTable, Asset Autocompletions is SendAssetForm and SpendingLimitForm

katspaugh · 2023-01-05T10:26:23Z

src/hooks/useVisibleBalances.ts

+  return {
+    ...balances,
+    balances: {
+      items: items.filter((item) => !hiddenTokens.includes(item.tokenInfo.address)),
+      fiatTotal,
+    },


Bear in mind this will filter the list and create a new object on each render. Would be good to memo it.

katspaugh · 2023-01-05T10:27:06Z

src/components/balances/AssetsTable/useHideAssets.ts

+import { useCallback, useState } from 'react'
+
+// This is the default for MUI Collapse
+export const COLLAPSE_TIMEOUT_MS = 300


No need to export it, it's used in the same module.

The test is using it.

usame-algan · 2023-01-05T10:32:05Z

There is some overlap/spacing issues on smaller screens. Do we have mobile designs? cc @liliiaorlenko

katspaugh

Looks good from my side, apart from the remaining comments.

src/components/balances/AssetsTable/useHideAssets.ts

usame-algan

👍

src/components/balances/AssetsTable/useHideAssets.ts

francovenica · 2023-01-06T03:25:24Z

1 - The feature works fine for desktop, but for mobile it not usable: you cannot see the checkboxes while the "hide tokens menu" is open so you cannot use the feature.
I'd suggest hiding the button for mobile views until we can design it to be mobile friendly

2 - The widget that counts the amount of tokens in the homepage still shows all the tokens you have, hidden or not. I'd argue that if you want to hide tokens, you don't want them to be counted with the ones you didn't hide.
I'd propose not count them at all or, if you have hidden tokens, you show a number in "()" indicating how many of them are hidden ones.

3 - Just refreshing a page with a bunch of tokens throws a ton of GTM calls. No tokens are hidden right now.
I'd assume the calls should be:
When the menu is opened/closed
When the cancel/deselect button are clicked
When the save button is clicked, sending how many tokens are being hidden by that save action.
When the "eye" icon is clicked to hide a token

Questions/Concerns:

Deselect all seems to clear the storage, so if you use it in a safe with barely no tokens you will end up deselecting a ton of tokens that you might have hidden in another safe without you knowing it. Maybe deselect can just trigger the click action in the checkboxes that are visible instead, so deselecting in one safe won't affect the others (unless they share the token being hidden/shown)
The total safe balance is not being affected by hiding tokens. I've been told that we are going to ask Johannes first about this, but I personally think that yes, the balances should reflect only the visible tokens
Is unlikely that somebody will set the "Safe" token as invisible, but if somebody does the token widget in the top bar and in the Home widget is still there. I'm gonna assume that is expected behaviour.

- adjusts total balance in useVisibleBalances() - uses useVisibleBalances in Overview and SidebarHeader - hides "Hide Tokens" button in mobile - moves analytics into useMetaEvents

github-actions · 2023-01-06T12:58:07Z

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	0	0
Ignored	0	N/A

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

schmanu · 2023-01-06T12:59:22Z

Hey @francovenica

I addresses most issues:

I removed the button in mobile.
The widget now does not count hidden assets anymore.
I moved the tracking to how we track the totalBalance (useMetaEvents). That should fix these issues. Although I wasn't able to reproduce those issues :/

For the questions:

Deselect all was implemented wrong. Now it only deselects tokens which are present in the opened Safe
After talking with product we agreed that the total balance should be adjusted. I added that now.
That is expected. The Safe token widget should not be affected by this.

francovenica · 2023-01-06T16:30:18Z

Yes, all the issues were addressed. The GTM calls are also working fine (not 10 calls just by refreshing the page anymore)

src/components/balances/AssetsHeader/index.tsx

src/hooks/useVisibleBalances.ts

katspaugh

Looks good!

Btw according to the SafeBalanceResponse type, items and fiatTotal are the only fields in this object, so you don't need the ...balances.

schmanu · 2023-01-09T13:41:43Z

@katspaugh
The names are a bit misleading there the returned object from const balances = useBalances will return

{
 balances: SafeBalanceResponse
  loading: boolean
  error?: string
}

So I spread to use the loading and error props from the useBalances hook as the visibleBalances would be loading / erroneous too.

schmanu added 2 commits December 9, 2022 18:13

feat: manually hide spam tokens

06ace67

feat: hidden balances are default for useBalances hook

ebdc93d

schmanu marked this pull request as draft December 16, 2022 09:32

schmanu added 5 commits December 19, 2022 18:24

change: review design isues

eaa7e4d

add: jest tests for AssetsTable and TokenListSelect

996ff34

add: more test cases

5a0f9f8

refactor: extract TokenMenu component

7b9be8c

test: refactored TokenMenu component

b42df29

schmanu marked this pull request as ready for review December 21, 2022 07:59

katspaugh reviewed Dec 21, 2022

View reviewed changes

schmanu added 4 commits December 21, 2022 12:24

add: design tweaks

64584e8

- remove SelectDialog => add ToggleButton - change icon order for hiding / unhiding - improve sticky TokenMenu positioning - make buttons the same size in TokenMenu

refactor: move hiddenAssetsSlice into settingsSlice

cf4fadf

- Moves hiddenAssetsSlice into settingsSlice - renames variables / hooks - moves TokenMenu inside of AssetsTable - adjusts tests

refactor: remove context provider and move logic into components

6ddc2b7

Merge remote-tracking branch 'origin/dev' into spam-tokens-manually-hide

23a3d16

schmanu added 4 commits December 27, 2022 15:28

change: small ui tweaks

e4c2dae

- show text in toggle button instead of tooltip - rename TokenListSelect to HiddenTokenToggle - increase size of hide token / show token button in assetTable

refactor & tests: collapse property for table rows

a5e0c0b

- New property collapsed for rows of EnhancedTable - collapsed rows will be collapsed by wrapping it's Cells in Collapse - fixes tests

add: tracking to checkbox

9028ce5

katspaugh reviewed Jan 4, 2023

View reviewed changes

src/components/balances/AssetsTable/index.tsx Outdated Show resolved Hide resolved

fix: styles of sticky menu

8ae035d

katspaugh reviewed Jan 4, 2023

View reviewed changes

src/components/balances/AssetsTable/index.tsx Outdated Show resolved Hide resolved

src/components/balances/AssetsTable/index.tsx Outdated Show resolved Hide resolved

refactor: move logic into hook

954b6eb

katspaugh reviewed Jan 4, 2023

View reviewed changes

src/hooks/useBalances.ts Outdated Show resolved Hide resolved

add: allow to hide native tokens, fix tracking

672fc9d

usame-algan reviewed Jan 4, 2023

View reviewed changes

src/components/balances/TokenMenu/styles.module.css Outdated Show resolved Hide resolved

src/hooks/useBalances.ts Outdated Show resolved Hide resolved

src/hooks/useBalances.ts Outdated Show resolved Hide resolved

src/services/analytics/events/assets.ts Outdated Show resolved Hide resolved

schmanu added 2 commits January 4, 2023 14:57

change: revert useBalances and add useVisibleBalances

1603188

- reverts useBalances changes - adds useVisibleBalance - does not change the fiatTotal anymore - hidden balances only have an effect on AssetTable, Asset Autocompletions is SendAssetForm and SpendingLimitForm

rename analytic events, remove padding

0705c38

katspaugh reviewed Jan 5, 2023

View reviewed changes

katspaugh approved these changes Jan 5, 2023

View reviewed changes

usame-algan reviewed Jan 5, 2023

View reviewed changes

src/components/balances/AssetsTable/useHideAssets.ts Outdated Show resolved Hide resolved

src/components/balances/AssetsTable/useHideAssets.ts Outdated Show resolved Hide resolved

schmanu added 2 commits January 5, 2023 12:00

fix: memoize visible balance

e47135c

fix: do not mutate state directly

90deddf

schmanu requested a review from usame-algan January 5, 2023 11:13

schmanu added 2 commits January 5, 2023 12:26

fix: uncomment line

0071cb4

fix: small css tweaks for mobile

8e45886

usame-algan approved these changes Jan 5, 2023

View reviewed changes

katspaugh reviewed Jan 5, 2023

View reviewed changes

src/components/balances/AssetsTable/useHideAssets.ts Outdated Show resolved Hide resolved

katspaugh reviewed Jan 5, 2023

View reviewed changes

src/components/balances/AssetsTable/useHideAssets.ts Outdated Show resolved Hide resolved

schmanu added 2 commits January 5, 2023 14:05

refactor: use filter instead of slice

79fb8cf

refactor: use concat instead of destructuring

2a69096

change: adjust total balance and dashboard overview

4eac84c

- adjusts total balance in useVisibleBalances() - uses useVisibleBalances in Overview and SidebarHeader - hides "Hide Tokens" button in mobile - moves analytics into useMetaEvents

fix: rerender problems after sorting assets (#1491)

3ee704c

usame-algan reviewed Jan 9, 2023

View reviewed changes

src/components/balances/AssetsHeader/index.tsx Outdated Show resolved Hide resolved

refactor: reduce AssetsHeader nesting

45f6630

katspaugh reviewed Jan 9, 2023

View reviewed changes

src/hooks/useVisibleBalances.ts Outdated Show resolved Hide resolved

refactor: split balance adjustment functions

70aeff3

katspaugh approved these changes Jan 9, 2023

View reviewed changes

schmanu merged commit a1c0502 into dev Jan 9, 2023

schmanu deleted the spam-tokens-manually-hide branch January 9, 2023 13:47

github-actions bot locked and limited conversation to collaborators Jan 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Spam tokens] manually hide tokens #1377

[Spam tokens] manually hide tokens #1377

schmanu commented Dec 15, 2022 •

edited

Loading

github-actions bot commented Dec 15, 2022 •

edited

Loading

github-actions bot commented Dec 15, 2022 •

edited

Loading

katspaugh left a comment

github-actions bot commented Dec 27, 2022 •

edited

Loading

katspaugh left a comment

katspaugh Jan 5, 2023

katspaugh Jan 5, 2023

schmanu Jan 5, 2023

usame-algan commented Jan 5, 2023

katspaugh left a comment

usame-algan left a comment

francovenica commented Jan 6, 2023

github-actions bot commented Jan 6, 2023 •

edited

Loading

schmanu commented Jan 6, 2023

francovenica commented Jan 6, 2023

katspaugh left a comment

schmanu commented Jan 9, 2023

[Spam tokens] manually hide tokens #1377

[Spam tokens] manually hide tokens #1377

Conversation

schmanu commented Dec 15, 2022 • edited Loading

What it solves

How this PR fixes it

How to test it

Analytics changes

Screenshots

github-actions bot commented Dec 15, 2022 • edited Loading

Branch preview

github-actions bot commented Dec 15, 2022 • edited Loading

ESLint Summary View Full Report

katspaugh left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 27, 2022 • edited Loading

ESLint Summary View Full Report

katspaugh left a comment

Choose a reason for hiding this comment

katspaugh Jan 5, 2023

Choose a reason for hiding this comment

katspaugh Jan 5, 2023

Choose a reason for hiding this comment

schmanu Jan 5, 2023

Choose a reason for hiding this comment

usame-algan commented Jan 5, 2023

katspaugh left a comment

Choose a reason for hiding this comment

usame-algan left a comment

Choose a reason for hiding this comment

francovenica commented Jan 6, 2023

github-actions bot commented Jan 6, 2023 • edited Loading

ESLint Summary View Full Report

schmanu commented Jan 6, 2023

francovenica commented Jan 6, 2023

katspaugh left a comment

Choose a reason for hiding this comment

schmanu commented Jan 9, 2023

schmanu commented Dec 15, 2022 •

edited

Loading

github-actions bot commented Dec 15, 2022 •

edited

Loading

github-actions bot commented Dec 15, 2022 •

edited

Loading

github-actions bot commented Dec 27, 2022 •

edited

Loading

github-actions bot commented Jan 6, 2023 •

edited

Loading