Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(minifront): #1211: remove swap restrictions #1337

Merged
merged 9 commits into from
Jun 21, 2024
14 changes: 14 additions & 0 deletions .changeset/friendly-horses-hear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
'minifront': minor
'@repo/ui': patch
---

Minifront:

- extend `BalanceSelector` to show not only assets with balances but all available assets
- fix the issues with empty wallets not rendering a swap block correctly
- reduce the height of `BalanceSelecor` and `AssetSelector` to `90dvh`
- autofocus the search inputs in `BalanceSelecor` and `AssetSelector`
- change validations of the swap input to allow entering any possible values

UI: allow passing HTML attributes to the `IconInput` component
7 changes: 4 additions & 3 deletions apps/minifront/src/components/shared/asset-selector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ const useFilteredAssets = ({ assets, value, onChange, filter }: AssetSelectorPro
const [search, setSearch] = useState('');

let filteredAssets = filter ? sortedAssets.filter(filter) : sortedAssets;
filteredAssets = search ? assets.filter(bySearch(search)) : assets;
filteredAssets = search ? assets.filter(filterMetadataBySearch(search)) : assets;

useEffect(
() => switchAssetIfNecessary({ value, onChange, filter, assets: filteredAssets }),
Expand All @@ -63,7 +63,7 @@ const useFilteredAssets = ({ assets, value, onChange, filter }: AssetSelectorPro
return { filteredAssets, search, setSearch };
};

const bySearch = (search: string) => (asset: Metadata) =>
export const filterMetadataBySearch = (search: string) => (asset: Metadata) =>
asset.display.toLocaleLowerCase().includes(search.toLocaleLowerCase()) ||
asset.symbol.toLocaleLowerCase().includes(search.toLocaleLowerCase());

Expand Down Expand Up @@ -118,7 +118,7 @@ export const AssetSelector = ({ assets, onChange, value, filter }: AssetSelector

<Dialog open={isOpen} onOpenChange={setIsOpen}>
<DialogContent layoutId={layoutId}>
<div className='flex max-h-screen flex-col'>
<div className='flex max-h-[90dvh] flex-col'>
<DialogHeader>Select asset</DialogHeader>

<div className='flex flex-col gap-2 overflow-auto p-4'>
Expand All @@ -127,6 +127,7 @@ export const AssetSelector = ({ assets, onChange, value, filter }: AssetSelector
icon={<MagnifyingGlassIcon className='size-5 text-muted-foreground' />}
value={search}
onChange={setSearch}
autoFocus
placeholder='Search assets...'
/>
</Box>
Expand Down
144 changes: 111 additions & 33 deletions apps/minifront/src/components/shared/balance-selector.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { MagnifyingGlassIcon } from '@radix-ui/react-icons';
import { useId, useState } from 'react';
import { useId, useMemo, useState } from 'react';
import { IconInput } from '@repo/ui/components/ui/icon-input';
import { Dialog, DialogClose, DialogContent, DialogHeader } from '@repo/ui/components/ui/dialog';
import { cn } from '@repo/ui/lib/utils';
Expand All @@ -9,19 +9,101 @@ import { getAddressIndex } from '@penumbra-zone/getters/address-view';
import { getDisplayDenomFromView, getSymbolFromValueView } from '@penumbra-zone/getters/value-view';
import { Box } from '@repo/ui/components/ui/box';
import { motion } from 'framer-motion';
import { Metadata } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/asset/v1/asset_pb';
import { filterMetadataBySearch } from './asset-selector';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: importing a helper from inside another component is a little messy.

suggestion: how about putting it in a ./helpers.ts file?

import { getMetadataFromBalancesResponseOptional } from '@penumbra-zone/getters/balances-response';
import { AssetIcon } from '@repo/ui/components/ui/tx/view/asset-icon';
import { emptyBalanceResponse } from '../../utils/empty-balance-response';

const bySearch = (search: string) => (balancesResponse: BalancesResponse) =>
const isMetadata = (asset: BalancesResponse | Metadata): asset is Metadata => {
return Boolean('symbol' in asset && 'name' in asset && 'display' in asset);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: I believe you can use asset.getType() to canonically get the message type.

e.g.,

Suggested change
return Boolean('symbol' in asset && 'name' in asset && 'display' in asset);
return asset.getType() === Metadata

This is provided by the bufbuild tooling, although I can't find docs for getType()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, didn't know about this! Got to modify the suggestion a little to return asset.getType().typeName === Metadata.typeName; to make it work

};

const isBalance = (asset: BalancesResponse | Metadata): asset is BalancesResponse => {
return 'balanceView' in asset && 'accountAddress' in asset;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same note here

};

const filterBalanceBySearch = (search: string) => (balancesResponse: BalancesResponse) =>
getDisplayDenomFromView(balancesResponse.balanceView)
.toLocaleLowerCase()
.includes(search.toLocaleLowerCase()) ||
getSymbolFromValueView(balancesResponse.balanceView)
.toLocaleLowerCase()
.includes(search.toLocaleLowerCase());

const bySearch = (search: string) => (asset: Metadata | BalancesResponse) => {
if (isMetadata(asset)) {
return filterMetadataBySearch(search)(asset);
}
if (isBalance(asset)) return filterBalanceBySearch(search)(asset);
return false;
};

const mergeBalancesAndAssets = (balances: BalancesResponse[] = [], assets: Metadata[] = []) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): add a return type to make it clear what this function does

Suggested change
const mergeBalancesAndAssets = (balances: BalancesResponse[] = [], assets: Metadata[] = []) => {
const mergeBalancesAndAssets = (balances: BalancesResponse[] = [], assets: Metadata[] = []): (BalancesResponse | Metadata)[] => {

const filteredAssets = assets.filter(asset => {
return !balances.some(balance => {
const balanceMetadata = getMetadataFromBalancesResponseOptional(balance);
return balanceMetadata?.equals(asset);
});
});
return [...balances, ...filteredAssets];
};

interface BalanceItemProps {
asset: BalancesResponse | Metadata;
value?: BalancesResponse | Metadata;
onSelect: (value: BalancesResponse | Metadata) => void;
}

const BalanceItem = ({ asset, value, onSelect }: BalanceItemProps) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: could we move this into its own file?

This file is quite long. I'm generally opposed to having two components in the same file unless one is very small. But this one has its own helpers, etc., so the two are a bit tangled up.

I'd move BalanceSelector to shared/balance-selector/index.tsx, and BalanceItem to shared/balance-selector/balance-item.tsx

const account = isBalance(asset) ? getAddressIndex(asset.accountAddress).account : undefined;
const metadata = isMetadata(asset) ? asset : getMetadataFromBalancesResponseOptional(asset);

const isSelected = useMemo(() => {
if (!value) return false;
if (isMetadata(value) && isMetadata(asset)) {
return value.equals(asset);
}
if (isBalance(value) && isBalance(asset)) {
return value.equals(asset);
}
return false;
}, [asset, value]);

return (
<div className='flex flex-col'>
<DialogClose onClick={() => onSelect(asset)}>
<div
className={cn(
'grid grid-cols-4 py-[10px] gap-3 cursor-pointer hover:bg-light-brown hover:px-4 hover:-mx-4 font-bold text-muted-foreground',
isSelected && 'bg-light-brown px-4 -mx-4',
)}
>
{metadata && (
<div className='col-span-2 flex items-center justify-start gap-1'>
<AssetIcon metadata={metadata} />
<p className='truncate'>{metadata.symbol || 'Unknown asset'}</p>
</div>
)}

<div className='flex justify-end'>
{isBalance(asset) && (
<ValueViewComponent showIcon={false} showDenom={false} view={asset.balanceView} />
)}
</div>

<p className='flex justify-center'>{account}</p>
</div>
</DialogClose>
</div>
);
};

interface BalanceSelectorProps {
value: BalancesResponse | undefined;
onChange: (selection: BalancesResponse) => void;
balances: BalancesResponse[];
balances?: BalancesResponse[];
assets?: Metadata[];
}

/**
Expand All @@ -32,12 +114,27 @@ interface BalanceSelectorProps {
* Use `<AssetSelector />` if you want to render assets that aren't tied to any
* balance.
*/
export default function BalanceSelector({ value, balances, onChange }: BalanceSelectorProps) {
export default function BalanceSelector({
value,
balances,
onChange,
assets,
}: BalanceSelectorProps) {
const [search, setSearch] = useState('');
const [isOpen, setIsOpen] = useState(false);
const filteredBalances = search ? balances.filter(bySearch(search)) : balances;
const layoutId = useId();

const allAssets = mergeBalancesAndAssets(balances, assets);
const filteredBalances = search ? allAssets.filter(bySearch(search)) : allAssets;

const onSelect = (asset: BalancesResponse | Metadata) => {
if (!isMetadata(asset)) {
onChange(asset);
return;
}
onChange(emptyBalanceResponse(asset));
};

return (
<>
{!isOpen && (
Expand All @@ -62,46 +159,27 @@ export default function BalanceSelector({ value, balances, onChange }: BalanceSe

<Dialog open={isOpen} onOpenChange={setIsOpen}>
<DialogContent layoutId={layoutId}>
<div className='flex max-h-screen flex-col'>
<div className='flex max-h-[90dvh] flex-col'>
<DialogHeader>Select asset</DialogHeader>
<div className='flex shrink flex-col gap-4 overflow-auto p-4'>
<Box spacing='compact'>
<IconInput
icon={<MagnifyingGlassIcon className='size-5 text-muted-foreground' />}
value={search}
onChange={setSearch}
autoFocus
placeholder='Search assets...'
/>
</Box>
<div className='mt-2 grid grid-cols-4 font-headline text-base font-semibold'>
<p className='flex justify-start'>Account</p>
<p className='col-span-3 flex justify-start'>Asset</p>
<div className='mt-2 grid grid-cols-4 gap-3 font-headline text-base font-semibold'>
<p className='col-span-2 flex justify-start'>Asset</p>
<p className='flex justify-end'>Balance</p>
<p className='flex justify-center'>Account</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the addition of this column, I'm seeing a UI glitch for long balances:

image

</div>
<div className='flex flex-col gap-2'>
{filteredBalances.map((b, i) => {
const index = getAddressIndex(b.accountAddress).account;

return (
<div key={i} className='flex flex-col'>
<DialogClose>
<div
className={cn(
'grid grid-cols-4 py-[10px] cursor-pointer hover:bg-light-brown hover:px-4 hover:-mx-4 font-bold text-muted-foreground',
value?.balanceView?.equals(b.balanceView) &&
value.accountAddress?.equals(b.accountAddress) &&
'bg-light-brown px-4 -mx-4',
)}
onClick={() => onChange(b)}
>
<p className='flex justify-start'>{index}</p>
<div className='col-span-3 flex justify-start'>
<ValueViewComponent view={b.balanceView} />
</div>
</div>
</DialogClose>
</div>
);
})}
{filteredBalances.map((asset, i) => (
<BalanceItem key={i} asset={asset} value={value} onSelect={onSelect} />
))}
</div>
</div>
</div>
Expand Down
39 changes: 15 additions & 24 deletions apps/minifront/src/components/swap/swap-form/token-swap-input.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { BalancesResponse } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/view/v1/view_pb';
import { BalanceValueView } from '@repo/ui/components/ui/balance-value-view';
import { Box } from '@repo/ui/components/ui/box';
import { CandlestickPlot } from '@repo/ui/components/ui/candlestick-plot';
Expand All @@ -13,34 +12,18 @@ import { ArrowRight } from 'lucide-react';
import { useEffect, useState } from 'react';
import { getBlockDate } from '../../../fetchers/block-date';
import { AllSlices } from '../../../state';
import { amountMoreThanBalance } from '../../../state/send';
import { useStoreShallow } from '../../../utils/use-store-shallow';
import { getFormattedAmtFromValueView } from '@penumbra-zone/types/value-view';
import { getAddressIndex } from '@penumbra-zone/getters/address-view';
import {
Metadata,
ValueView,
} from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/asset/v1/asset_pb';
import { AssetSelector } from '../../shared/asset-selector';
import BalanceSelector from '../../shared/balance-selector';
import { Amount } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/num/v1/num_pb';
import { useStatus } from '../../../state/status';
import { hasStakingToken } from '../../../fetchers/staking-token';

const isValidAmount = (amount: string, assetIn?: BalancesResponse) =>
Number(amount) >= 0 && (!assetIn || !amountMoreThanBalance(assetIn, amount));

const getKnownZeroValueView = (metadata?: Metadata) => {
return new ValueView({
valueView: {
case: 'knownAssetId',
value: { amount: new Amount({ lo: 0n }), metadata },
},
});
};
import { zeroValueView } from '../../../utils/zero-value-view';
import { isValidAmount } from '../../../state/swap/instant-swap';

const assetOutBalanceSelector = ({ swap: { balancesResponses, assetIn, assetOut } }: AllSlices) => {
if (!assetIn || !assetOut) return getKnownZeroValueView();
if (!assetIn || !assetOut) return zeroValueView();

const match = balancesResponses.find(balance => {
const balanceViewMetadata = getMetadataFromBalancesResponse(balance);
Expand All @@ -50,7 +33,7 @@ const assetOutBalanceSelector = ({ swap: { balancesResponses, assetIn, assetOut
);
});
const matchedBalance = getBalanceView.optional()(match);
return matchedBalance ?? getKnownZeroValueView(assetOut);
return matchedBalance ?? zeroValueView(assetOut);
};

const tokenSwapInputSelector = (state: AllSlices) => ({
Expand Down Expand Up @@ -129,7 +112,6 @@ export const TokenSwapInput = () => {
step='any'
className={'font-bold leading-10 md:h-8 md:text-xl xl:h-10 xl:text-3xl'}
onChange={e => {
if (!isValidAmount(e.target.value, assetIn)) return;
setAmount(e.target.value);
setshowNonNativeFeeWarning(Number(e.target.value) > 0 && !stakingToken);
}}
Expand All @@ -144,9 +126,18 @@ export const TokenSwapInput = () => {
)}

<div className='flex h-full flex-col gap-2'>
<BalanceSelector value={assetIn} onChange={setAssetIn} balances={balancesResponses} />
<BalanceSelector
value={assetIn}
onChange={setAssetIn}
assets={swappableAssets}
balances={balancesResponses}
/>
{assetIn?.balanceView && (
<BalanceValueView valueView={assetIn.balanceView} onClick={setInputToBalanceMax} />
<BalanceValueView
valueView={assetIn.balanceView}
error={!isValidAmount(amount, assetIn)}
onClick={setInputToBalanceMax}
/>
)}
</div>

Expand Down
14 changes: 9 additions & 5 deletions apps/minifront/src/components/swap/swap-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { Metadata } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/
import { getSwappableBalancesResponses, isSwappable } from './helpers';
import { getAllAssets } from '../../fetchers/assets';
import { getStakingTokenMetadata } from '../../fetchers/registry';
import { emptyBalanceResponse } from '../../utils/empty-balance-response';

export interface UnclaimedSwapsWithMetadata {
swap: SwapRecord;
Expand All @@ -20,11 +21,14 @@ const getAndSetDefaultAssetBalances = async (swappableAssets: Metadata[]) => {
const stakingTokenAssetMetadata = await getStakingTokenMetadata();

// set initial denom in if there is an available balance
if (balancesResponses[0]) {
useStore.getState().swap.setAssetIn(balancesResponses[0]);
useStore.getState().swap.setAssetOut(swappableAssets[0]!);
useStore.getState().swap.setStakingAssetMetadata(stakingTokenAssetMetadata);
}
const initialBalance = balancesResponses[0];
const initialBalanceMetadata = swappableAssets[0]
? emptyBalanceResponse(swappableAssets[0])
: undefined;

useStore.getState().swap.setAssetIn(initialBalance ?? initialBalanceMetadata);
useStore.getState().swap.setAssetOut(initialBalance ? swappableAssets[0] : swappableAssets[1]);
useStore.getState().swap.setStakingAssetMetadata(stakingTokenAssetMetadata);

return balancesResponses;
};
Expand Down
2 changes: 1 addition & 1 deletion apps/minifront/src/state/swap/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ describe('Swap Slice', () => {

test('assetOut can be set', () => {
expect(useStore.getState().swap.assetOut).toBeUndefined();
useStore.getState().swap.setAssetOut(registryAssets[0]!);
useStore.getState().swap.setAssetOut(registryAssets[0]);
expect(useStore.getState().swap.assetOut).toBe(registryAssets[0]);
});

Expand Down
Loading
Loading