Hide assets from views #1797

aristidesstaffieri · 2025-01-16T19:15:24Z

Closes #1620
Design: https://www.figma.com/design/C3G0a4Gd6RQyplRBppGDsL/Freighter-(SDS-v3)?node-id=2932-3383&t=KhdFFpbyyxRgbwuo-1

What
Adds a "Toggle Assets" page, where users can select assets to hide from their views.

Why
Users may no longer want to see assets where they have "dust" and no longer care or for whatever other reason they may want to remove the asset from view while still keeping it in their assets list.

Notable Changes:
Adds new internal actions GET_HIDDEN_ASSETS & CHANGE_ASSET_VISIBILITY which operate on a new storage key HIDDEN_ASSETS. HIDDEN_ASSETS is a map of <issuer key, "hidden" | "visible">.
Adds new "toggle assets" page and navigation action from the "Manage Assets" page.
Changes getAccountBalances to accept a new arg, showHidden. This can be used to filter out assets according to their state in HIDDEN_ASSETS.
Refactors ChooseAsset to fetch it's own balances, it now needs to account for hidden assets.
Extracts a new hook, useFetchDomains to be used in both ChooseAsset and AssetVisibility.
Adds new test for toggling an asset's visibility.

…nsion settings duck state

…en assets

aristidesstaffieri · 2025-01-16T19:17:39Z

extension/src/popup/components/manageAssets/ChooseAsset/index.tsx

-    soroswapTokens,
-    networkDetails,
-  ]);
+  const goBack = () => {


Now that we need to fetch balances here, this pattern suffers more from the stale store state so this resets the store in order for the state to be correct on the Account page once navigation lands there. I noted a possible refactor to help this in the new hook fetchDomains.

aristidesstaffieri · 2025-01-16T19:21:05Z

extension/src/popup/helpers/useFetchDomains.ts

+
+  const [assets, setAssets] = useState({
+    assetRows: [] as ManageAssetCurrency[],
+    // TODO: REFACTOR getAccountBalances to a hook


I think it would be good to line up getAccountBalances to move to a hook similar to useGetHistory. This shared state made it tricky to ensure the correct loading/initial state across several views sharing it. Each view relies on unrelated views to "clean up" the state in order for the pages to work correctly.

Yeah, good call - that'd be a much nicer improvement than having to manually rehydrate

aristidesstaffieri · 2025-01-16T19:21:26Z

extension/src/popup/views/Account/index.tsx

 import "./styles.scss";

-export const defaultAccountBalances = {


Just getting rid of a little cruft here.

aristidesstaffieri · 2025-01-16T19:22:21Z

extension/src/popup/views/ManageAssets/index.tsx

 import { PrivateKeyRoute } from "popup/Router";
 import { ROUTES } from "popup/constants/routes";

-export const ManageAssets = () => {


Both views that need balances here now need to fetch them on render, this is because AssetVisibility needs to not filter hidden assets, and ChooseAsset needs to take the hidden assets into account.

piyalbasu · 2025-01-16T20:38:38Z

@shared/api/internal.ts

+  response = await sendMessageToBackground({
+    type: SERVICE_TYPES.CHANGE_ASSET_VISIBILITY,
+    assetVisibility: {
+      issuer: assetIssuer,


We'll run into an issue here when a G address is the issuer for multiple assets.

Here's an example of an issuer on Mainnet who issues multiple assets under one address: GBX23RCWKV7DA23J2MA2OFMTRV3XZHHKOTHBQF6AKOEP7AGQUIZZXLMG

I think you might just want to use {assetCode}:{assetIssuer} as your key here

@piyalbasu very good point thank you, will update to use the composite key.
Can an issuer have two assets that use the same ticker?

done in 0512f6c

Oh, that's a good question about multiple assets using the same ticket. Off the top of my head, I'm not 100% sure. We may need to test it out and see if the network lets us do it

piyalbasu · 2025-01-16T20:44:55Z

extension/src/popup/components/manageAssets/ToggleAssetRows/index.tsx

+                  />
+                  <Toggle
+                    checked={isAssetVisible(hiddenAssets, issuer)}
+                    id="isVisible"


NIT: you'll potentially end up with multiple elements with the same id. Not sure how much that actually matters here - but to be syntactically correct, you might wanna use something like isVisible-${code}:${issuer}

ah yeah I didn't use the ID in this case but will update for completeness

done in 0512f6c

piyalbasu · 2025-01-16T20:45:55Z

extension/src/popup/components/manageAssets/ToggleAssetRows/styles.scss

+      color: var(--sds-clr-gray-09);
+
+      svg {
+        margin-right: 0.25rem;


NIT: would you mind using pxToRem? It'd be nice to start removing rem altogether from the codebase over time

done in 0512f6c

piyalbasu · 2025-01-16T20:48:09Z

extension/src/popup/views/Account/index.tsx

@@ -175,7 +165,6 @@ export const Account = () => {
  return (
    <>
      <AccountHeader
-        // accountDropDownRef={accountDropDownRef}


piyalbasu · 2025-01-16T20:52:55Z

extension/src/popup/views/__tests__/ManageAssets.test.tsx

+    expect(childInput).toBeChecked();
+    fireEvent.change(childInput, { target: { checked: false } });
+    expect(childInput).not.toBeChecked();
+  });


It might be nice to have an e2e test for this. It would cover the UI stuff plus make sure the background is properly saving hidden assets

yeah good call it would make sense to navigate to the account view and test for the hidden asset being missing from the list also, let me see what I can add here.

CassioMG · 2025-01-16T21:31:51Z

extension/src/constants/localStorageTypes.ts

@@ -23,3 +23,4 @@ export const IS_HASH_SIGNING_ENABLED_ID = "isHashSigningEnabled";
 export const IS_NON_SSL_ENABLED_ID = "isNonSSLEnabled";
 export const IS_BLOCKAID_ANNOUNCED_ID = "isBlockaidAnnounced";
 export const IS_HIDE_DUST_ENABLED_ID = "isHideDustEnabled";
+export const HIDDEN_ASSETS = "hiddenAsset";


Suggested change

export const HIDDEN_ASSETS = "hiddenAsset";

export const HIDDEN_ASSETS = "hiddenAssets";

😄 thanks

done in 365d79b

CassioMG · 2025-01-16T21:37:44Z

extension/src/popup/components/manageAssets/AssetVisibility/styles.scss

+    &--short {
+      flex-grow: 1;
+    }


does it make sense to have this one since it's not adding any actual change?

oh good call this came from a similar view but its not needed here. Removing.

done in 365d79b

CassioMG · 2025-01-16T21:41:45Z

extension/src/popup/components/manageAssets/ToggleAssetRows/styles.scss

+  &__content {
+    display: flex;
+    flex-direction: column;
+    gap: 1.5rem;


should we use pxToRem() in all values from this file?

Yup! Adding...

done in 365d79b

…storage key name

aristidesstaffieri added 9 commits January 16, 2025 10:44

adds background settings key and methods for hidden assets, adds exte…

6ee9edd

…nsion settings duck state

adds ducks for gettung hidden assets, adds button to navigate to hidd…

deb3838

…en assets

adds asset visibility toggle in asset rows

1103ed1

adds isAssetVisible helper, tweaks background handling of hidden assets

7b14b6e

updates toggle asset classes

733b24e

refactor getAccountBalances to allow filtering of hidden assets

467b7e4

wip: reset account balance after managing assets

973a9a8

adds test for hiding a token

7134974

adds useFetchDomains helper

5ecd930

aristidesstaffieri self-assigned this Jan 16, 2025

aristidesstaffieri commented Jan 16, 2025

View reviewed changes

mocks getHiddenAssets in existing tests

19e83ed

piyalbasu reviewed Jan 16, 2025

View reviewed changes

switches to composite key for asset key in hidden asset map

0512f6c

CassioMG reviewed Jan 16, 2025

View reviewed changes

aristidesstaffieri added 4 commits January 16, 2025 15:09

updates e2e tests for new Manage Assets header

e0efa9f

uses pxToRem helper in place of raw rem values, tweaks hidden assets …

365d79b

…storage key name

Merge branch 'release/5.28.0' into feature/hide-tokens-2

e82dd31

Merge branch 'release/5.28.0' into feature/hide-tokens-2

16f98b5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hide assets from views #1797

Hide assets from views #1797

aristidesstaffieri commented Jan 16, 2025 •

edited

Loading

aristidesstaffieri Jan 16, 2025

aristidesstaffieri Jan 16, 2025

piyalbasu Jan 16, 2025

aristidesstaffieri Jan 16, 2025

aristidesstaffieri Jan 16, 2025

piyalbasu Jan 16, 2025

Mateusz9611 Jan 16, 2025

aristidesstaffieri Jan 16, 2025

aristidesstaffieri Jan 16, 2025

piyalbasu Jan 16, 2025

piyalbasu Jan 16, 2025

aristidesstaffieri Jan 16, 2025

aristidesstaffieri Jan 16, 2025

piyalbasu Jan 16, 2025

aristidesstaffieri Jan 16, 2025

piyalbasu Jan 16, 2025

piyalbasu Jan 16, 2025

aristidesstaffieri Jan 16, 2025

CassioMG Jan 16, 2025

aristidesstaffieri Jan 16, 2025

aristidesstaffieri Jan 16, 2025

CassioMG Jan 16, 2025

aristidesstaffieri Jan 16, 2025

aristidesstaffieri Jan 16, 2025

CassioMG Jan 16, 2025

aristidesstaffieri Jan 16, 2025

aristidesstaffieri Jan 16, 2025

		import "./styles.scss";

		export const defaultAccountBalances = {

	export const HIDDEN_ASSETS = "hiddenAsset";
	export const HIDDEN_ASSETS = "hiddenAssets";

Hide assets from views #1797

Are you sure you want to change the base?

Hide assets from views #1797

Conversation

aristidesstaffieri commented Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aristidesstaffieri commented Jan 16, 2025 •

edited

Loading