feat: hide asset list unsupported tokens in accordion, closes #16 #5344

alter-eggo · 2024-05-03T13:57:13Z

Try out Leather build bb66cb7 — Extension build, Test report, Storybook, Chromatic

This pr puts unsupported list of tokens in accordion, also adds Accordion ui component

Screen.Recording.2024-05-03.at.17.56.08.mov

markmhendrickson · 2024-05-03T14:24:32Z

@fabric-8 curious how we want to handle left and right padding here on hover? Or better no hover background effect?

alter-eggo · 2024-05-03T16:38:54Z

@markmhendrickson fixed paddings, needed some css hack to do it
https://github.com/leather-wallet/extension/assets/46521087/b271ea25-80ba-4354-9099-6642a30df0cb

src/app/query/stacks/balance/stacks-ft-balances.hooks.ts

fbwoolf · 2024-05-03T17:45:56Z

theme/keyframes.ts

@@ -8,6 +8,14 @@ export const keyframes: CssKeyframes = {
    from: { opacity: 1, transform: 'translateY(0)' },
    to: { opacity: 0, transform: 'translateY(4px)' },
  },
+  slideDown: {


We do have a slideUpAndFade and slideDownAndFade being used in other UI components (dropdown, select, and tooltip) ...wondering about naming here bc specific to this feature with the maxHeight?

hm, how do you think better to name? 🤔

Not necessary to change, but thought maybe just adding the maxHeight to the ones we already have might work? Not critical to address.

changing maxHeight there will break animation, dropdown's menu height will go from 0 to the actual height, I believe we don't expect it there

fbwoolf

Looks good @alter-eggo, just some naming suggestions.

fabric-8 · 2024-05-03T21:48:55Z

Amazing to see this coming to life already!

Realizing that the way it's set up in the design doesn't match the current implementation because the token list items are built differently (They don't have any padding inside and all the padding is given via the outer container).

In design we're using token list item "future versions" which have a built-in padding that would align with a button like used here. The hack you used is nifty, however given the current situation and the container getting really close to the left/right boundaries I'd also propose to not use a full background hover effect but instead do something simpler and just rely on a change of text color on hover (action-primary-hover) without changing the background, similar to what we do for the account switcher.

Apologies this is on me, should have checked how the current token list items are constructed instead of relying on the future-to-be-design-ones and made that suggestion from the start.

kyranjamie · 2024-05-04T12:44:40Z

src/app/query/stacks/balance/stacks-ft-balances.hooks.ts

+export function useFilteredStacksFungibleTokenList({
+  address,
+  filter = 'all',
+}: useFilteredStacksFungibleTokenListArgs) {
+  const stacksFtAssetBalances = useStacksFungibleTokenAssetBalancesWithMetadata(address);
+  const { data: swapAssets = [] } = useAlexSwappableAssets();
+
+  return useMemo(() => {
+    if (filter === 'supported') {
+      return stacksFtAssetBalances.filter(assetBalance =>
+        swapAssets.some(swapAsset =>
+          swapAsset.principal.includes(assetBalance.asset.contractAddress)
+        )
+      );
+    }
+
+    if (filter === 'unsupported') {
+      return stacksFtAssetBalances.filter(
+        assetBalance =>
+          !swapAssets.some(swapAsset =>
+            swapAsset.principal.includes(assetBalance.asset.contractAddress)
+          )
+      );
+    }
+
+    return stacksFtAssetBalances;
+  }, [stacksFtAssetBalances, swapAssets, filter]);
+}


Can we break apart the filtering from the hook that binds it to the state?

separated it

kyranjamie · 2024-05-04T12:45:19Z

src/app/ui/components/accordion/accordion.tsx

+    left: '-2%',
+    right: '-2%',


Should we use negative spacing units, rather than percentage here?

I wonder also if we can reuse any styles from <Pressable />, which does the same thing?

removed it, due to @fabric-8 comment above

kyranjamie · 2024-05-06T12:48:33Z

src/app/query/stacks/balance/stacks-ft-balances.hooks.ts

+/**
+ * @see https://github.com/leather-wallet/issues/issues/16
+ */
+type StacksFtTokensFilter = 'all' | 'supported' | 'unsupported';


Think this is a smart API choice 👍🏼

pete-watters · 2024-05-07T10:15:33Z

src/app/features/asset-list/asset-list.tsx

@@ -108,6 +109,9 @@ export function AssetsList() {
        )}
      </BitcoinNativeSegwitAccountLoader>

+      <CurrentStacksAccountLoader fallback={<AddStacksLedgerKeysItem />}>


@alter-eggo - can you please check this as it's causing a double display of the Connect Stacks button on Ledger if you only have a BTC account:

alter-eggo · 2024-05-08T12:57:56Z

thanks @pete-watters
fixed here #5352

markmhendrickson · 2024-05-13T12:44:57Z

Nice work, @alter-eggo 💪 Does the UI remember the user's preference in terms of hiding or showing? E.g. if the user shows the tokens and refreshes, are they still shown or hidden again?

If the UI doesn't remember, let's create a separate issue to track that as an enhancement.

alter-eggo · 2024-05-13T12:46:30Z

@markmhendrickson always hidden, will you create a task?

markmhendrickson · 2024-05-14T10:38:49Z

I've created it here.

alter-eggo requested review from kyranjamie, fbwoolf and fabric-8 May 3, 2024 13:58

alter-eggo force-pushed the feat/filter-token-list branch from 6e2ccc6 to 5befee1 Compare May 3, 2024 14:00

alter-eggo force-pushed the feat/filter-token-list branch from 5befee1 to d6484fa Compare May 3, 2024 16:38

fbwoolf reviewed May 3, 2024

View reviewed changes

src/app/query/stacks/balance/stacks-ft-balances.hooks.ts Outdated Show resolved Hide resolved

fbwoolf reviewed May 3, 2024

View reviewed changes

fbwoolf approved these changes May 3, 2024

View reviewed changes

alter-eggo force-pushed the feat/filter-token-list branch from d6484fa to 3fe305d Compare May 3, 2024 19:05

kyranjamie approved these changes May 4, 2024

View reviewed changes

alter-eggo force-pushed the feat/filter-token-list branch from 3fe305d to a3a1236 Compare May 6, 2024 12:41

kyranjamie reviewed May 6, 2024

View reviewed changes

feat: hide asset list unsupported tokens in accordion, closes #16

bb66cb7

alter-eggo force-pushed the feat/filter-token-list branch from a3a1236 to bb66cb7 Compare May 6, 2024 13:05

alter-eggo added this pull request to the merge queue May 6, 2024

Merged via the queue into dev with commit f37bb1b May 6, 2024
28 checks passed

alter-eggo deleted the feat/filter-token-list branch May 6, 2024 13:16

pete-watters reviewed May 7, 2024

View reviewed changes

alter-eggo mentioned this pull request May 8, 2024

fix: double stacks asset in ledger mode #5352

Merged

markmhendrickson mentioned this pull request May 14, 2024

Persist toggling of unrecognized Stacks fungible tokens #5369

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: hide asset list unsupported tokens in accordion, closes #16 #5344

feat: hide asset list unsupported tokens in accordion, closes #16 #5344

alter-eggo commented May 3, 2024 •

edited by leather-bot

Loading

markmhendrickson commented May 3, 2024

alter-eggo commented May 3, 2024

fbwoolf May 3, 2024

alter-eggo May 3, 2024

fbwoolf May 4, 2024

alter-eggo May 6, 2024

fbwoolf left a comment

fabric-8 commented May 3, 2024

kyranjamie May 4, 2024

alter-eggo May 6, 2024

kyranjamie May 4, 2024

kyranjamie May 4, 2024

alter-eggo May 6, 2024

kyranjamie May 6, 2024

pete-watters May 7, 2024

alter-eggo commented May 8, 2024

markmhendrickson commented May 13, 2024

alter-eggo commented May 13, 2024

markmhendrickson commented May 14, 2024

feat: hide asset list unsupported tokens in accordion, closes #16 #5344

feat: hide asset list unsupported tokens in accordion, closes #16 #5344

Conversation

alter-eggo commented May 3, 2024 • edited by leather-bot Loading

markmhendrickson commented May 3, 2024

alter-eggo commented May 3, 2024

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

fbwoolf left a comment

Choose a reason for hiding this comment

fabric-8 commented May 3, 2024

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

alter-eggo commented May 8, 2024

markmhendrickson commented May 13, 2024

alter-eggo commented May 13, 2024

markmhendrickson commented May 14, 2024

alter-eggo commented May 3, 2024 •

edited by leather-bot

Loading