From ee206c94dc303a8c40abcdb3106447fe3411c67b Mon Sep 17 00:00:00 2001 From: Rigidity Date: Sat, 14 Nov 2026 19:44:16 -0500 Subject: [PATCH 1/3] Redesign the asset rows on the create offer page - Put each asset's inputs in one bordered row, amount before asset, with the remove button inside it - Show the spendable balance under each offered token, and flag an amount above it - Look up requested NFT IDs and show what was found, or why nothing was - Drop the Tokens, NFTs and Options sub-headings and the per-row z-index workaround - Show the open-offer warning for NFTs and options as text under the row --- .../selectors/AssetSelector.test.tsx | 121 +++++ src/components/selectors/AssetSelector.tsx | 446 ++++++++++-------- src/components/selectors/OfferAssetRow.tsx | 129 +++++ src/hooks/useRequestedNft.test.tsx | 107 +++++ src/hooks/useRequestedNft.ts | 91 ++++ 5 files changed, 686 insertions(+), 208 deletions(-) create mode 100644 src/components/selectors/AssetSelector.test.tsx create mode 100644 src/components/selectors/OfferAssetRow.tsx create mode 100644 src/hooks/useRequestedNft.test.tsx create mode 100644 src/hooks/useRequestedNft.ts diff --git a/src/components/selectors/AssetSelector.test.tsx b/src/components/selectors/AssetSelector.test.tsx new file mode 100644 index 000000000..da1de2ed8 --- /dev/null +++ b/src/components/selectors/AssetSelector.test.tsx @@ -0,0 +1,121 @@ +// @vitest-environment jsdom + +import { i18n } from '@lingui/core'; +import { I18nProvider } from '@lingui/react'; +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; + +vi.mock('@/bindings', () => ({ + commands: { + getCats: vi.fn(() => + Promise.resolve({ + cats: [ + { + asset_id: 'cat1', + name: 'Cat', + ticker: 'CAT', + precision: 3, + balance: '500000', + selectable_balance: '500000', + visible: true, + }, + ], + }), + ), + getToken: vi.fn(() => + Promise.resolve({ + token: { + asset_id: null, + name: 'Chia', + ticker: 'XCH', + precision: 12, + balance: '10000000000000', + selectable_balance: '10000000000000', + visible: true, + }, + }), + ), + getOffersForAsset: vi.fn(() => Promise.resolve({ offers: [] })), + }, + events: { syncEvent: { listen: vi.fn(() => Promise.resolve(vi.fn())) } }, +})); +vi.mock('./TokenSelector', () => ({ TokenSelector: () => null })); +vi.mock('./NftSelector', () => ({ NftSelector: () => null })); +vi.mock('./OptionSelector', () => ({ OptionSelector: () => null })); +vi.mock('@/components/ui/masked-input', () => ({ + TokenAmountInput: (props: { + value: string; + onValueChange: (values: { value: string }) => void; + }) => ( + props.onValueChange({ value: e.target.value })} + /> + ), +})); + +import { Assets } from '@/state'; +import { AssetSelector } from './AssetSelector'; + +beforeAll(() => { + i18n.loadAndActivate({ locale: 'en', messages: {} }); +}); + +afterEach(cleanup); + +function renderOffering(assets: Assets, fee = '0') { + const setAssets = vi.fn(); + render( + + + , + ); + return setAssets; +} + +const tokens = (assetId: string | null, amount: string): Assets => ({ + tokens: [{ asset_id: assetId, amount }], + nfts: [], + options: [], +}); + +describe('AssetSelector offered tokens', () => { + it('shows the spendable balance under the row', async () => { + renderOffering(tokens('cat1', '')); + expect(await screen.findByText(/500 CAT spendable/)).toBeTruthy(); + }); + + it('flags an amount above the spendable balance', async () => { + renderOffering(tokens('cat1', '501')); + expect( + await screen.findByText('More than your spendable balance'), + ).toBeTruthy(); + }); + + it('counts the network fee against an XCH amount', async () => { + renderOffering(tokens(null, '10'), '0.5'); + expect( + await screen.findByText( + 'More than your spendable balance after the network fee', + ), + ).toBeTruthy(); + }); + + it('fills in the spendable balance less the fee with Max', async () => { + const setAssets = renderOffering(tokens(null, ''), '0.5'); + await screen.findByText(/XCH spendable/); + + fireEvent.click( + screen.getByRole('button', { name: 'Use maximum spendable balance' }), + ); + + expect(setAssets).toHaveBeenCalledWith(tokens(null, '9.5')); + }); +}); diff --git a/src/components/selectors/AssetSelector.tsx b/src/components/selectors/AssetSelector.tsx index af7de112f..3e6da984b 100644 --- a/src/components/selectors/AssetSelector.tsx +++ b/src/components/selectors/AssetSelector.tsx @@ -3,29 +3,22 @@ import { NftSelector } from '@/components/selectors/NftSelector'; import { TokenSelector } from '@/components/selectors/TokenSelector'; import { Button } from '@/components/ui/button'; import { Input } from '@/components/ui/input'; -import { Label } from '@/components/ui/label'; import { TokenAmountInput } from '@/components/ui/masked-input'; -import { - Tooltip, - TooltipContent, - TooltipProvider, - TooltipTrigger, -} from '@/components/ui/tooltip'; +import { useRequestedNft } from '@/hooks/useRequestedNft'; import { optionsEnabled } from '@/lib/features'; -import { toDecimal } from '@/lib/utils'; +import { fromMojos, toDecimal } from '@/lib/utils'; import { Assets } from '@/state'; import BigNumber from 'bignumber.js'; import { t } from '@lingui/core/macro'; import { Trans } from '@lingui/react/macro'; -import { - AlertTriangle, - FilePenLine, - HandCoins, - ImageIcon, - PlusIcon, - TrashIcon, -} from 'lucide-react'; +import { AlertTriangle, PlusIcon } from 'lucide-react'; import { ReactNode, useCallback, useEffect, useState } from 'react'; +import { NumberFormat } from '../NumberFormat'; +import { + OfferAssetRow, + OfferAssetRowFooter, + RequestedNftPreview, +} from './OfferAssetRow'; import { OptionSelector } from './OptionSelector'; interface AssetSelectorProps { @@ -208,206 +201,185 @@ export function AssetSelector({ )} - {assets.tokens.length > 0 && ( -
- - {assets.tokens.map(({ asset_id: assetId, amount }, i) => { - const ownedToken = ownedTokens.find((t) => t.asset_id === assetId); - return ( -
- { - const newTokens = [...assets.tokens]; - newTokens[i] = { - ...newTokens[i], - asset_id: newAssetId, - amount: '', - }; - setAssets({ ...assets, tokens: newTokens }); - }} - disabled={assets.tokens - .filter((t, idx) => t.asset_id !== '' && idx !== i) - .map((t) => t.asset_id)} - className='!rounded-r-none' - hideZeroBalance={offering === true} - showAllCats={offering !== true} - includeXch={true} - /> -
- - updateToken(i, 'amount', values.value) - } - precision={assetId === null ? 12 : 3} - maxValue={ - ownedToken - ? BigNumber.max( - 0, - BigNumber( - toDecimal( - ownedToken.selectable_balance, - ownedToken.precision, - ), - ).minus(assetId === null ? fee || 0 : 0), - ).toString() - : undefined - } - maxButtonClassName='!rounded-r-none' - hideMaxButton={!offering} - /> - -
-
- ); - })} -
- )} - - {assets.nfts.length > 0 && ( -
- - {assets.nfts.map((nft, i) => ( -
- {offering === true ? ( - updateNft(i, nftId || '')} - disabled={assets.nfts.filter( - (id, idx) => id !== '' && idx !== i, + {assets.tokens.map(({ asset_id: assetId, amount }, i) => { + const ownedToken = offering + ? ownedTokens.find((t) => t.asset_id === assetId) + : undefined; + const maxValue = ownedToken + ? BigNumber.max( + 0, + BigNumber( + toDecimal(ownedToken.selectable_balance, ownedToken.precision), + ).minus(assetId === null ? fee || 0 : 0), + ) + : undefined; + const overBalance = + maxValue !== undefined && + amount !== '' && + BigNumber(amount.toString()).gt(maxValue); + + return ( + removeToken(i)} + removeLabel={t`Remove token`} + tone={overBalance ? 'error' : undefined} + footer={ + ownedToken && ( + + {overBalance ? ( + assetId === null ? ( + + More than your spendable balance after the network fee + + ) : ( + More than your spendable balance + ) + ) : ( + + + )} - className='!rounded-r-none' - /> - ) : ( - updateNft(i, e.target.value)} - /> - )} - - {/* Warning icon for assets in offers */} - {offering && nft && assetsInOffers.has(nft) && ( - - - -
- -
- - - This NFT is part of an open offer - - - - )} - + + ) + } + > +
+ + updateToken(i, 'amount', values.value) + } + precision={assetId === null ? 12 : 3} + hideMaxButton + /> +
+ {offering && ( + )} +
+ { + const newTokens = [...assets.tokens]; + newTokens[i] = { + ...newTokens[i], + asset_id: newAssetId, + amount: '', + }; + setAssets({ ...assets, tokens: newTokens }); + }} + disabled={assets.tokens + .filter((t, idx) => t.asset_id !== '' && idx !== i) + .map((t) => t.asset_id)} + className={rowSelectClassName} + hideZeroBalance={offering === true} + showAllCats={offering !== true} + includeXch={true} + />
- ))} -
+ + ); + })} + + {assets.nfts.map((nft, i) => + offering ? ( + removeNft(i)} + removeLabel={t`Remove NFT`} + tone={nft && assetsInOffers.has(nft) ? 'warning' : undefined} + footer={ + nft && + assetsInOffers.has(nft) && ( + + + ) + } + > +
+ updateNft(i, nftId || '')} + disabled={assets.nfts.filter( + (id, idx) => id !== '' && idx !== i, + )} + className={rowSelectClassName} + /> +
+ + ) : ( + updateNft(i, value)} + onRemove={() => removeNft(i)} + /> + ), )} - {assets.options.length > 0 && ( -
- - {assets.options.map((option, i) => ( -
- {offering === true ? ( - updateOption(i, optionId || '')} - disabled={assets.options.filter( - (id, idx) => id !== '' && idx !== i, - )} - className='!rounded-r-none' - /> - ) : ( - updateOption(i, e.target.value)} - /> - )} - - {/* Warning icon for assets in offers */} - {offering && option && assetsInOffers.has(option) && ( - - - -
- -
- - - This option is part of an open offer - - - - )} + {assets.options.map((option, i) => ( + removeOption(i)} + removeLabel={t`Remove option`} + tone={ + offering && option && assetsInOffers.has(option) + ? 'warning' + : undefined + } + footer={ + offering && + option && + assetsInOffers.has(option) && ( + + + ) + } + > +
+ {offering ? ( + updateOption(i, optionId || '')} + disabled={assets.options.filter( + (id, idx) => id !== '' && idx !== i, + )} + className={rowSelectClassName} + /> + ) : ( + updateOption(i, e.target.value)} + /> + )} +
+ + ))} - -
- ))} -
- )}
); } + +// The selectors and inputs sit inside an OfferAssetRow, which draws the border. +const rowSelectClassName = + 'h-12 rounded-none border-0 bg-transparent shadow-none'; +const rowInputClassName = + 'h-12 rounded-none border-0 bg-transparent font-mono text-sm shadow-none focus-visible:ring-0'; + +function SpendableBalance({ token }: { token: TokenRecord }) { + const ticker = token.ticker ?? ''; + + return ( + + {' '} + {ticker} spendable + + ); +} + +interface RequestedNftRowProps { + value: string; + onChange: (value: string) => void; + onRemove: () => void; +} + +function RequestedNftRow({ value, onChange, onRemove }: RequestedNftRowProps) { + const { lookup, thumbnail } = useRequestedNft(value); + + return ( + } + > +
+ onChange(e.target.value)} + /> +
+ + ); +} diff --git a/src/components/selectors/OfferAssetRow.tsx b/src/components/selectors/OfferAssetRow.tsx new file mode 100644 index 000000000..a2f4690db --- /dev/null +++ b/src/components/selectors/OfferAssetRow.tsx @@ -0,0 +1,129 @@ +import { MutableNftBadge } from '@/components/MutableNftBadge'; +import { Button } from '@/components/ui/button'; +import { NftLookup } from '@/hooks/useRequestedNft'; +import { nftUri } from '@/lib/nftUri'; +import { cn } from '@/lib/utils'; +import { t } from '@lingui/core/macro'; +import { Trans } from '@lingui/react/macro'; +import { Loader2, TrashIcon } from 'lucide-react'; +import { ReactNode } from 'react'; + +export interface OfferAssetRowProps { + /** The inputs, laid out left to right and separated by dividers. */ + children: ReactNode; + onRemove: () => void; + removeLabel: string; + /** Shown inside the frame, below the inputs. */ + footer?: ReactNode; + tone?: 'error' | 'warning'; +} + +/** One asset in an offer: its inputs in a single bordered frame. */ +export function OfferAssetRow({ + children, + onRemove, + removeLabel, + footer, + tone, +}: OfferAssetRowProps) { + return ( +
+
+ {children} + +
+ {footer} +
+ ); +} + +export interface OfferAssetRowFooterProps { + children: ReactNode; + tone?: 'error' | 'warning'; +} + +/** A thin strip along the bottom of an `OfferAssetRow`. */ +export function OfferAssetRowFooter({ + children, + tone, +}: OfferAssetRowFooterProps) { + return ( +
+ {children} +
+ ); +} + +export interface RequestedNftPreviewProps { + lookup: NftLookup; + thumbnail: string | null; +} + +/** What the wallet found for a requested NFT ID, shown under its row. */ +export function RequestedNftPreview({ + lookup, + thumbnail, +}: RequestedNftPreviewProps) { + switch (lookup.status) { + case 'idle': + return null; + case 'invalid': + return ( + + This isn't a valid NFT ID. + + ); + case 'loading': + return ( + + + ); + case 'missing': + return ( + + + Couldn't find this NFT. Check the ID, or try again once + you're connected to a peer. + + + ); + case 'found': + return ( +
+ +
+ + {lookup.name ?? t`Unnamed NFT`} + + +
+
+ ); + } +} diff --git a/src/hooks/useRequestedNft.test.tsx b/src/hooks/useRequestedNft.test.tsx new file mode 100644 index 000000000..b6701d58f --- /dev/null +++ b/src/hooks/useRequestedNft.test.tsx @@ -0,0 +1,107 @@ +// @vitest-environment jsdom + +import { act, cleanup, render, screen } from '@testing-library/react'; +import { bech32m } from 'bech32'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const getNftMetadataUpdater = vi.fn(); +const getNftThumbnail = vi.fn(); + +vi.mock('@/bindings', () => ({ + commands: { + getNftMetadataUpdater: (...args: unknown[]) => + getNftMetadataUpdater(...args), + getNftThumbnail: (...args: unknown[]) => getNftThumbnail(...args), + }, + events: { syncEvent: { listen: vi.fn(() => Promise.resolve(vi.fn())) } }, +})); + +import { useRequestedNft } from './useRequestedNft'; + +const nftId = bech32m.encode( + 'nft', + bech32m.toWords(new Uint8Array(32).fill(7)), +); + +function Probe({ id }: { id: string }) { + const { lookup, thumbnail } = useRequestedNft(id); + return ( +
+ {lookup.status} + + {lookup.status === 'found' ? (lookup.name ?? '') : ''} + + {thumbnail ?? ''} +
+ ); +} + +async function settle() { + await act(async () => { + await vi.advanceTimersByTimeAsync(500); + }); +} + +beforeEach(() => { + vi.useFakeTimers(); + getNftThumbnail.mockResolvedValue({ thumbnail: null }); +}); + +afterEach(() => { + cleanup(); + vi.useRealTimers(); + vi.clearAllMocks(); +}); + +describe('useRequestedNft', () => { + it('stays idle while the field is empty', async () => { + render(); + await settle(); + expect(screen.getByTestId('status').textContent).toBe('idle'); + expect(getNftMetadataUpdater).not.toHaveBeenCalled(); + }); + + it('rejects text that is not an NFT ID without a lookup', async () => { + render(); + await settle(); + expect(screen.getByTestId('status').textContent).toBe('invalid'); + expect(getNftMetadataUpdater).not.toHaveBeenCalled(); + }); + + it('shows the name of an NFT found on the network', async () => { + getNftMetadataUpdater.mockResolvedValue({ + metadata_updater_kind: 'default', + name: 'Cool NFT', + }); + + render(); + expect(screen.getByTestId('status').textContent).toBe('loading'); + await settle(); + + expect(getNftMetadataUpdater).toHaveBeenCalledWith({ nft_id: nftId }); + expect(screen.getByTestId('status').textContent).toBe('found'); + expect(screen.getByTestId('name').textContent).toBe('Cool NFT'); + }); + + it('reports an NFT the network does not know', async () => { + getNftMetadataUpdater.mockResolvedValue({ + metadata_updater_kind: null, + name: null, + }); + + render(); + await settle(); + + expect(screen.getByTestId('status').textContent).toBe('missing'); + expect(getNftThumbnail).not.toHaveBeenCalled(); + }); + + it('treats a failed lookup as missing', async () => { + getNftMetadataUpdater.mockRejectedValue(new Error('no peer')); + + render(); + await settle(); + + expect(screen.getByTestId('status').textContent).toBe('missing'); + }); +}); diff --git a/src/hooks/useRequestedNft.ts b/src/hooks/useRequestedNft.ts new file mode 100644 index 000000000..a0d53eb99 --- /dev/null +++ b/src/hooks/useRequestedNft.ts @@ -0,0 +1,91 @@ +import { commands, events, NftUpdaterKind } from '@/bindings'; +import { nftUri } from '@/lib/nftUri'; +import { isValidAddress } from '@/lib/utils'; +import { useEffect, useState } from 'react'; +import { useDebounce } from './useDebounce'; + +export type NftLookup = + | { status: 'idle' } + | { status: 'invalid' } + | { status: 'loading' } + | { status: 'missing' } + | { + status: 'found'; + name: string | null; + kind: NftUpdaterKind; + }; + +/** + * Looks up an NFT the user is requesting, so they can see they typed the + * right ID before making the offer. The lookup goes to the network, the same + * way the offer itself fetches a requested NFT. + */ +export function useRequestedNft(nftId: string): { + lookup: NftLookup; + thumbnail: string | null; +} { + const trimmed = nftId.trim(); + const debounced = useDebounce(trimmed, 400); + const [lookup, setLookup] = useState({ status: 'idle' }); + const [thumbnail, setThumbnail] = useState(null); + + useEffect(() => { + setThumbnail(null); + + if (!debounced) { + setLookup({ status: 'idle' }); + return; + } + + if (!isValidAddress(debounced, 'nft')) { + setLookup({ status: 'invalid' }); + return; + } + + let cancelled = false; + setLookup({ status: 'loading' }); + + const updateThumbnail = () => + commands + .getNftThumbnail({ nft_id: debounced }) + .then(({ thumbnail }) => { + if (!cancelled && thumbnail !== null) { + setThumbnail(nftUri('image/png', thumbnail)); + } + }) + .catch(() => undefined); + + commands + .getNftMetadataUpdater({ nft_id: debounced }) + .then(({ metadata_updater_kind: kind, name }) => { + if (cancelled) return; + setLookup( + kind === null + ? { status: 'missing' } + : { status: 'found', name, kind }, + ); + if (kind !== null) updateThumbnail(); + }) + .catch(() => { + if (!cancelled) setLookup({ status: 'missing' }); + }); + + // The image is downloaded after the NFT is first seen, so check again + // once it arrives. + const unlisten = events.syncEvent.listen((event) => { + if (event.payload.type === 'nft_data') updateThumbnail(); + }); + + return () => { + cancelled = true; + unlisten.then((u) => u()); + }; + }, [debounced]); + + // Until the debounce catches up, an edited ID hasn't been checked yet. + if (trimmed !== debounced && trimmed) { + return { lookup: { status: 'loading' }, thumbnail: null }; + } + + return { lookup, thumbnail }; +} From 82b2db6d0ee2b2ee1e2f2177b2db1179b08a6a66 Mon Sep 17 00:00:00 2001 From: Rigidity Date: Sat, 14 Nov 2026 23:44:40 -0500 Subject: [PATCH 2/3] Address review findings on the offer asset rows - Drop the requested NFT's thumbnail, which never loads: the wallet only keeps images for NFTs it holds. Show "NFT found" until the name is known, rather than calling a named NFT unnamed - Store a pasted NFT ID without surrounding whitespace, since the offer would reject it after the preview said it was found - Reuse the row footer for the found preview - Test the lookup's debounce and that a late result is ignored, the preview's states, amounts at the spendable balance, the fee only counting against XCH, and the empty message --- .../selectors/AssetSelector.test.tsx | 65 +++++++++++++++++++ src/components/selectors/AssetSelector.tsx | 8 ++- .../selectors/OfferAssetRow.test.tsx | 51 +++++++++++++++ src/components/selectors/OfferAssetRow.tsx | 31 ++++----- src/hooks/useRequestedNft.test.tsx | 61 +++++++++++++---- src/hooks/useRequestedNft.ts | 36 ++-------- 6 files changed, 192 insertions(+), 60 deletions(-) create mode 100644 src/components/selectors/OfferAssetRow.test.tsx diff --git a/src/components/selectors/AssetSelector.test.tsx b/src/components/selectors/AssetSelector.test.tsx index da1de2ed8..7d0739f91 100644 --- a/src/components/selectors/AssetSelector.test.tsx +++ b/src/components/selectors/AssetSelector.test.tsx @@ -119,3 +119,68 @@ describe('AssetSelector offered tokens', () => { expect(setAssets).toHaveBeenCalledWith(tokens(null, '9.5')); }); }); + +function renderSelector(props: { + assets: Assets; + offering?: boolean; + emptyMessage?: string; +}) { + const setAssets = vi.fn(); + render( + + + , + ); + return setAssets; +} + +const noAssets: Assets = { tokens: [], nfts: [], options: [] }; + +describe('AssetSelector balance edges', () => { + it('accepts an amount equal to the spendable balance', async () => { + renderOffering(tokens('cat1', '500')); + await screen.findByText(/500 CAT spendable/); + expect(screen.queryByText(/More than your spendable balance/)).toBeNull(); + }); + + it('only takes the network fee off XCH', async () => { + renderOffering(tokens('cat1', '500'), '0.5'); + await screen.findByText(/500 CAT spendable/); + expect(screen.queryByText(/More than your spendable balance/)).toBeNull(); + }); +}); + +describe('AssetSelector layout', () => { + it('shows the empty message only while nothing has been added', () => { + renderSelector({ assets: noAssets, emptyMessage: 'Nothing yet' }); + expect(screen.getByText('Nothing yet')).toBeTruthy(); + + cleanup(); + renderSelector({ + assets: { ...noAssets, nfts: [''] }, + emptyMessage: 'Nothing yet', + }); + expect(screen.queryByText('Nothing yet')).toBeNull(); + }); + + it('stores a pasted NFT ID without surrounding whitespace', () => { + const setAssets = renderSelector({ + assets: { ...noAssets, nfts: [''] }, + }); + + fireEvent.change(screen.getByLabelText('NFT ID'), { + target: { value: ' nft1example\n' }, + }); + expect(setAssets).toHaveBeenCalledWith({ + ...noAssets, + nfts: ['nft1example'], + }); + }); +}); diff --git a/src/components/selectors/AssetSelector.tsx b/src/components/selectors/AssetSelector.tsx index 3e6da984b..1c9bd4e0a 100644 --- a/src/components/selectors/AssetSelector.tsx +++ b/src/components/selectors/AssetSelector.tsx @@ -430,7 +430,7 @@ interface RequestedNftRowProps { } function RequestedNftRow({ value, onChange, onRemove }: RequestedNftRowProps) { - const { lookup, thumbnail } = useRequestedNft(value); + const lookup = useRequestedNft(value); return ( } + footer={} >
onChange(e.target.value)} + // Pasted IDs often bring a space or newline, which the offer + // wouldn't accept. + onChange={(e) => onChange(e.target.value.trim())} />
diff --git a/src/components/selectors/OfferAssetRow.test.tsx b/src/components/selectors/OfferAssetRow.test.tsx new file mode 100644 index 000000000..6fff3441b --- /dev/null +++ b/src/components/selectors/OfferAssetRow.test.tsx @@ -0,0 +1,51 @@ +// @vitest-environment jsdom + +import { i18n } from '@lingui/core'; +import { I18nProvider } from '@lingui/react'; +import { cleanup, render, screen } from '@testing-library/react'; +import { afterEach, beforeAll, describe, expect, it } from 'vitest'; +import { NftLookup } from '@/hooks/useRequestedNft'; +import { RequestedNftPreview } from './OfferAssetRow'; + +beforeAll(() => { + i18n.loadAndActivate({ locale: 'en', messages: {} }); +}); + +afterEach(cleanup); + +function renderPreview(lookup: NftLookup) { + return render( + + + , + ); +} + +describe('RequestedNftPreview', () => { + it('shows nothing before an ID is typed', () => { + const { container } = renderPreview({ status: 'idle' }); + expect(container.textContent).toBe(''); + }); + + it('says when the ID is not valid', () => { + renderPreview({ status: 'invalid' }); + expect(screen.getByText("This isn't a valid NFT ID.")).toBeTruthy(); + }); + + it('says when the NFT could not be found', () => { + renderPreview({ status: 'missing' }); + expect(screen.getByText(/Couldn't find this NFT/)).toBeTruthy(); + }); + + it('shows the name and marks a mutable NFT', () => { + renderPreview({ status: 'found', name: 'Cool NFT', kind: 'any' }); + expect(screen.getByText('Cool NFT')).toBeTruthy(); + expect(screen.getByLabelText('Mutable NFT')).toBeTruthy(); + }); + + it("doesn't call a found NFT unnamed before its name is known", () => { + renderPreview({ status: 'found', name: null, kind: 'default' }); + expect(screen.getByText('NFT found')).toBeTruthy(); + expect(screen.queryByLabelText('Mutable NFT')).toBeNull(); + }); +}); diff --git a/src/components/selectors/OfferAssetRow.tsx b/src/components/selectors/OfferAssetRow.tsx index a2f4690db..5e034d3e1 100644 --- a/src/components/selectors/OfferAssetRow.tsx +++ b/src/components/selectors/OfferAssetRow.tsx @@ -1,11 +1,10 @@ import { MutableNftBadge } from '@/components/MutableNftBadge'; import { Button } from '@/components/ui/button'; import { NftLookup } from '@/hooks/useRequestedNft'; -import { nftUri } from '@/lib/nftUri'; import { cn } from '@/lib/utils'; import { t } from '@lingui/core/macro'; import { Trans } from '@lingui/react/macro'; -import { Loader2, TrashIcon } from 'lucide-react'; +import { ImageIcon, Loader2, TrashIcon } from 'lucide-react'; import { ReactNode } from 'react'; export interface OfferAssetRowProps { @@ -54,12 +53,14 @@ export function OfferAssetRow({ export interface OfferAssetRowFooterProps { children: ReactNode; tone?: 'error' | 'warning'; + className?: string; } /** A thin strip along the bottom of an `OfferAssetRow`. */ export function OfferAssetRowFooter({ children, tone, + className, }: OfferAssetRowFooterProps) { return (
{children} @@ -76,14 +78,10 @@ export function OfferAssetRowFooter({ export interface RequestedNftPreviewProps { lookup: NftLookup; - thumbnail: string | null; } /** What the wallet found for a requested NFT ID, shown under its row. */ -export function RequestedNftPreview({ - lookup, - thumbnail, -}: RequestedNftPreviewProps) { +export function RequestedNftPreview({ lookup }: RequestedNftPreviewProps) { switch (lookup.status) { case 'idle': return null; @@ -111,19 +109,22 @@ export function RequestedNftPreview({ ); case 'found': return ( -
- + // There's no thumbnail: the wallet only keeps images for NFTs it + // holds, and a requested NFT usually belongs to someone else. + + +
- {lookup.name ?? t`Unnamed NFT`} + {lookup.name ?? t`NFT found`}
-
+ ); } } diff --git a/src/hooks/useRequestedNft.test.tsx b/src/hooks/useRequestedNft.test.tsx index b6701d58f..6a949c6bd 100644 --- a/src/hooks/useRequestedNft.test.tsx +++ b/src/hooks/useRequestedNft.test.tsx @@ -5,33 +5,29 @@ import { bech32m } from 'bech32'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const getNftMetadataUpdater = vi.fn(); -const getNftThumbnail = vi.fn(); vi.mock('@/bindings', () => ({ commands: { getNftMetadataUpdater: (...args: unknown[]) => getNftMetadataUpdater(...args), - getNftThumbnail: (...args: unknown[]) => getNftThumbnail(...args), }, - events: { syncEvent: { listen: vi.fn(() => Promise.resolve(vi.fn())) } }, })); import { useRequestedNft } from './useRequestedNft'; -const nftId = bech32m.encode( - 'nft', - bech32m.toWords(new Uint8Array(32).fill(7)), -); +const encodeNftId = (byte: number) => + bech32m.encode('nft', bech32m.toWords(new Uint8Array(32).fill(byte))); +const nftId = encodeNftId(7); +const otherNftId = encodeNftId(8); function Probe({ id }: { id: string }) { - const { lookup, thumbnail } = useRequestedNft(id); + const lookup = useRequestedNft(id); return (
{lookup.status} {lookup.status === 'found' ? (lookup.name ?? '') : ''} - {thumbnail ?? ''}
); } @@ -44,7 +40,6 @@ async function settle() { beforeEach(() => { vi.useFakeTimers(); - getNftThumbnail.mockResolvedValue({ thumbnail: null }); }); afterEach(() => { @@ -93,7 +88,6 @@ describe('useRequestedNft', () => { await settle(); expect(screen.getByTestId('status').textContent).toBe('missing'); - expect(getNftThumbnail).not.toHaveBeenCalled(); }); it('treats a failed lookup as missing', async () => { @@ -104,4 +98,49 @@ describe('useRequestedNft', () => { expect(screen.getByTestId('status').textContent).toBe('missing'); }); + + it('waits for typing to stop before looking up', async () => { + getNftMetadataUpdater.mockResolvedValue({ + metadata_updater_kind: 'default', + name: 'Cool NFT', + }); + + // An ID already there when the row appears is looked up straight away, + // so start from an empty field and type. + const { rerender } = render(); + rerender(); + await act(async () => { + await vi.advanceTimersByTimeAsync(200); + }); + rerender(); + await settle(); + + expect(getNftMetadataUpdater).toHaveBeenCalledTimes(1); + expect(getNftMetadataUpdater).toHaveBeenCalledWith({ nft_id: nftId }); + }); + + it('ignores a lookup that finishes after the ID changed', async () => { + let finishFirst: (value: unknown) => void = () => undefined; + getNftMetadataUpdater + .mockReturnValueOnce( + new Promise((resolve) => { + finishFirst = resolve; + }), + ) + .mockResolvedValueOnce({ + metadata_updater_kind: 'default', + name: 'Second NFT', + }); + + const { rerender } = render(); + await settle(); + rerender(); + await settle(); + expect(screen.getByTestId('name').textContent).toBe('Second NFT'); + + await act(async () => { + finishFirst({ metadata_updater_kind: 'default', name: 'First NFT' }); + }); + expect(screen.getByTestId('name').textContent).toBe('Second NFT'); + }); }); diff --git a/src/hooks/useRequestedNft.ts b/src/hooks/useRequestedNft.ts index a0d53eb99..0c603ea76 100644 --- a/src/hooks/useRequestedNft.ts +++ b/src/hooks/useRequestedNft.ts @@ -1,5 +1,4 @@ -import { commands, events, NftUpdaterKind } from '@/bindings'; -import { nftUri } from '@/lib/nftUri'; +import { commands, NftUpdaterKind } from '@/bindings'; import { isValidAddress } from '@/lib/utils'; import { useEffect, useState } from 'react'; import { useDebounce } from './useDebounce'; @@ -11,6 +10,7 @@ export type NftLookup = | { status: 'missing' } | { status: 'found'; + /** Null until the NFT's metadata has been downloaded. */ name: string | null; kind: NftUpdaterKind; }; @@ -20,18 +20,12 @@ export type NftLookup = * right ID before making the offer. The lookup goes to the network, the same * way the offer itself fetches a requested NFT. */ -export function useRequestedNft(nftId: string): { - lookup: NftLookup; - thumbnail: string | null; -} { +export function useRequestedNft(nftId: string): NftLookup { const trimmed = nftId.trim(); const debounced = useDebounce(trimmed, 400); const [lookup, setLookup] = useState({ status: 'idle' }); - const [thumbnail, setThumbnail] = useState(null); useEffect(() => { - setThumbnail(null); - if (!debounced) { setLookup({ status: 'idle' }); return; @@ -45,16 +39,6 @@ export function useRequestedNft(nftId: string): { let cancelled = false; setLookup({ status: 'loading' }); - const updateThumbnail = () => - commands - .getNftThumbnail({ nft_id: debounced }) - .then(({ thumbnail }) => { - if (!cancelled && thumbnail !== null) { - setThumbnail(nftUri('image/png', thumbnail)); - } - }) - .catch(() => undefined); - commands .getNftMetadataUpdater({ nft_id: debounced }) .then(({ metadata_updater_kind: kind, name }) => { @@ -64,28 +48,18 @@ export function useRequestedNft(nftId: string): { ? { status: 'missing' } : { status: 'found', name, kind }, ); - if (kind !== null) updateThumbnail(); }) .catch(() => { if (!cancelled) setLookup({ status: 'missing' }); }); - // The image is downloaded after the NFT is first seen, so check again - // once it arrives. - const unlisten = events.syncEvent.listen((event) => { - if (event.payload.type === 'nft_data') updateThumbnail(); - }); - return () => { cancelled = true; - unlisten.then((u) => u()); }; }, [debounced]); // Until the debounce catches up, an edited ID hasn't been checked yet. - if (trimmed !== debounced && trimmed) { - return { lookup: { status: 'loading' }, thumbnail: null }; - } + if (trimmed !== debounced && trimmed) return { status: 'loading' }; - return { lookup, thumbnail }; + return lookup; } From 4bb145088fb714c38b406eca8864347b0f2402f0 Mon Sep 17 00:00:00 2001 From: Rigidity Date: Sat, 14 Nov 2026 23:53:11 -0500 Subject: [PATCH 3/3] Show an emptied or malformed requested NFT ID straight away Emptying the field or breaking the ID kept the previous NFT's preview up until the debounce caught up. Neither needs a lookup, so show them at once. Also test that requested tokens show no balance or Max. --- .../selectors/AssetSelector.test.tsx | 22 ++++++++++++++++++- src/hooks/useRequestedNft.test.tsx | 20 +++++++++++++++++ src/hooks/useRequestedNft.ts | 10 +++++++-- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/src/components/selectors/AssetSelector.test.tsx b/src/components/selectors/AssetSelector.test.tsx index 7d0739f91..adfad9aa6 100644 --- a/src/components/selectors/AssetSelector.test.tsx +++ b/src/components/selectors/AssetSelector.test.tsx @@ -2,7 +2,13 @@ import { i18n } from '@lingui/core'; import { I18nProvider } from '@lingui/react'; -import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { + act, + cleanup, + fireEvent, + render, + screen, +} from '@testing-library/react'; import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; vi.mock('@/bindings', () => ({ @@ -157,6 +163,20 @@ describe('AssetSelector balance edges', () => { }); }); +describe('AssetSelector requested tokens', () => { + it("doesn't show a balance, Max or a balance error", async () => { + renderSelector({ assets: tokens('cat1', '501') }); + // Give any balance lookup the chance to finish. + await act(async () => undefined); + + expect(screen.queryByText(/spendable/)).toBeNull(); + expect( + screen.queryByRole('button', { name: 'Use maximum spendable balance' }), + ).toBeNull(); + expect(screen.queryByText(/More than your spendable balance/)).toBeNull(); + }); +}); + describe('AssetSelector layout', () => { it('shows the empty message only while nothing has been added', () => { renderSelector({ assets: noAssets, emptyMessage: 'Nothing yet' }); diff --git a/src/hooks/useRequestedNft.test.tsx b/src/hooks/useRequestedNft.test.tsx index 6a949c6bd..a797b9beb 100644 --- a/src/hooks/useRequestedNft.test.tsx +++ b/src/hooks/useRequestedNft.test.tsx @@ -135,6 +135,9 @@ describe('useRequestedNft', () => { const { rerender } = render(); await settle(); rerender(); + // The new ID hasn't been checked yet, so nothing from the old one shows. + expect(screen.getByTestId('status').textContent).toBe('loading'); + expect(screen.getByTestId('name').textContent).toBe(''); await settle(); expect(screen.getByTestId('name').textContent).toBe('Second NFT'); @@ -143,4 +146,21 @@ describe('useRequestedNft', () => { }); expect(screen.getByTestId('name').textContent).toBe('Second NFT'); }); + + it('clears a found NFT as soon as the field is emptied or broken', async () => { + getNftMetadataUpdater.mockResolvedValue({ + metadata_updater_kind: 'default', + name: 'Cool NFT', + }); + + const { rerender } = render(); + await settle(); + expect(screen.getByTestId('status').textContent).toBe('found'); + + rerender(); + expect(screen.getByTestId('status').textContent).toBe('idle'); + + rerender(); + expect(screen.getByTestId('status').textContent).toBe('invalid'); + }); }); diff --git a/src/hooks/useRequestedNft.ts b/src/hooks/useRequestedNft.ts index 0c603ea76..c2fc2e58b 100644 --- a/src/hooks/useRequestedNft.ts +++ b/src/hooks/useRequestedNft.ts @@ -58,8 +58,14 @@ export function useRequestedNft(nftId: string): NftLookup { }; }, [debounced]); - // Until the debounce catches up, an edited ID hasn't been checked yet. - if (trimmed !== debounced && trimmed) return { status: 'loading' }; + // An empty or malformed ID needs no lookup, so it's shown straight away + // rather than after the debounce. A valid ID that's still being typed + // hasn't been checked yet. + if (trimmed !== debounced) { + if (!trimmed) return { status: 'idle' }; + if (!isValidAddress(trimmed, 'nft')) return { status: 'invalid' }; + return { status: 'loading' }; + } return lookup; }