Skip to content

Commit

Permalink
Filter the <AccountSwitcher /> on the staking page to only allow sw…
Browse files Browse the repository at this point in the history
…itching between relevant accounts (#679)

* Add an accountsSelector to the Staking slice

* Add tests and filter capabilities to AccountSwitcher

* Filter by accounts relevant to staking

* Disable manual account index entry

* Hide the _whole_ button

* Address PR comments
  • Loading branch information
jessepinho authored Mar 6, 2024
1 parent 283817a commit 1a2a191
Show file tree
Hide file tree
Showing 5 changed files with 287 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { ValueViewComponent } from '@penumbra-zone/ui/components/ui/tx/view/valu
import { Stat } from './stat';
import { ValueView } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/asset/v1/asset_pb';
import { STAKING_TOKEN_METADATA } from '@penumbra-zone/constants';
import { stakingSelector } from '../../../../state/staking';
import { accountsSelector, stakingSelector } from '../../../../state/staking';
import { useStore } from '../../../../state';

/**
Expand All @@ -38,12 +38,13 @@ export const Header = () => {
useStore(stakingSelector);
const unstakedTokens = unstakedTokensByAccount.get(account);
const unbondingTokens = unbondingTokensByAccount.get(account);
const accountSwitcherFilter = useStore(accountsSelector);

return (
<Card gradient>
<CardContent>
<div className='flex flex-col gap-2'>
<AccountSwitcher account={account} onChange={setAccount} />
<AccountSwitcher account={account} onChange={setAccount} filter={accountSwitcherFilter} />

<div className='flex justify-center gap-8'>
<Stat label='Available to delegate'>
Expand Down
64 changes: 64 additions & 0 deletions apps/minifront/src/state/staking.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
AddressView,
IdentityKey,
} from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/keys/v1/keys_pb';
import { accountsSelector } from './staking';

const validator1IdentityKey = new IdentityKey({ ik: new Uint8Array([1, 2, 3]) });
const validator1Bech32IdentityKey = bech32IdentityKey(validator1IdentityKey);
Expand Down Expand Up @@ -236,4 +237,67 @@ describe('Staking Slice', () => {
await getState().staking.loadDelegationsForCurrentAccount();
expect(Object.values(getState().staking.votingPowerByValidatorInfo).length).toBe(4);
});

describe('accountsSelector return value', () => {
let result: number[];

beforeEach(() => {
const state = useStore.getState();

useStore.setState({
...state,
staking: {
...state.staking,
unstakedTokensByAccount: new Map([
// Leave unsorted for the sake of the sorting test below.
[1, new ValueView()],
[2, undefined],
[0, new ValueView()],
]),

delegationsByAccount: new Map([
[3, []],
[4, [new ValueView()]],
[5, []],
]),

unbondingTokensByAccount: new Map([
// Leave unsorted for the sake of the sorting test below.
[6, { total: new ValueView(), tokens: [new ValueView()] }],
[5, { total: new ValueView(), tokens: [new ValueView()] }],
]),
},
});

result = accountsSelector(useStore.getState());
});

it('includes accounts that have only unstaked tokens', () => {
expect(result).toContain(0);
});

it('includes accounts that have only delegation tokens', () => {
expect(result).toContain(4);
});

it('includes accounts that have only unbonding tokens', () => {
expect(result).toContain(6);
});

it('includes accounts that have empty delegations but populated unbonding tokens', () => {
expect(result).toContain(5);
});

it('excludes accounts that only have undefined unstaked tokens', () => {
expect(result).not.toContain(2);
});

it('excludes accounts that only have empty delegation tokens', () => {
expect(result).not.toContain(3);
});

it('excludes accounts that have no relevant tokens', () => {
expect(result).not.toContain(7);
});
});
});
27 changes: 27 additions & 0 deletions apps/minifront/src/state/staking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,33 @@ export const createStakingSlice = (): SliceCreator<StakingSlice> => (set, get) =

export const stakingSelector = (state: AllSlices) => state.staking;

/**
* Selector to get all accounts that are relevant to staking (in the form of
* their numeric address index). This includes accounts that:
* 1. have unstaked UM tokens.
* 2. have delegation tokens.
* 3. have unbonding tokens.
*/
export const accountsSelector = (state: AllSlices): number[] => {
const accounts = new Set<number>();

for (const account of state.staking.unstakedTokensByAccount.keys()) {
if (state.staking.unstakedTokensByAccount.get(account)) accounts.add(account);
}

for (const account of state.staking.delegationsByAccount.keys()) {
const delegations = state.staking.delegationsByAccount.get(account);
if (delegations?.length) accounts.add(account);
}

for (const account of state.staking.unbondingTokensByAccount.keys()) {
const unbondingTokens = state.staking.unbondingTokensByAccount.get(account)?.tokens;
if (unbondingTokens?.length) accounts.add(account);
}

return Array.from(accounts.values());
};

const assembleDelegateRequest = ({ account, amount, validatorInfo }: StakingSlice) => {
return new TransactionPlannerRequest({
delegations: [
Expand Down
124 changes: 124 additions & 0 deletions packages/ui/components/ui/account-switcher.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
import { describe, expect, it, vi } from 'vitest';
import { AccountSwitcher } from './account-switcher';
import { fireEvent, render } from '@testing-library/react';

describe('<AccountSwitcher />', () => {
it('renders the current account', () => {
const { getByLabelText } = render(<AccountSwitcher account={123} onChange={vi.fn()} />);

expect(getByLabelText('Account #123')).toHaveValue(123);
});

describe('changing the account via the input field', () => {
it('calls `onChange` with the new value', () => {
const onChange = vi.fn();
const { getByLabelText } = render(<AccountSwitcher account={123} onChange={onChange} />);

expect(onChange).not.toHaveBeenCalled();
fireEvent.change(getByLabelText('Account #123'), { target: { value: 456 } });
expect(onChange).toHaveBeenCalledWith(456);
});

it('has aria-disabled=false', () => {
const { getByLabelText } = render(<AccountSwitcher account={123} onChange={vi.fn()} />);

expect(getByLabelText('Account #123')).toHaveAttribute('aria-disabled', 'false');
});

describe('when a filter has been passed', () => {
it('does not call `onChange` -- i.e., it is effectively disabled', () => {
const onChange = vi.fn();
const { getByLabelText } = render(
<AccountSwitcher account={123} onChange={onChange} filter={[1, 2, 3]} />,
);

fireEvent.change(getByLabelText('Account #123'), { target: { value: 456 } });
expect(onChange).not.toHaveBeenCalled();
});

it('has aria-disabled=true', () => {
const { getByLabelText } = render(
<AccountSwitcher account={123} onChange={vi.fn()} filter={[1, 2, 3]} />,
);

expect(getByLabelText('Account #123')).toHaveAttribute('aria-disabled', 'true');
});
});
});

describe('the previous button', () => {
it('calls `onChange` with one less than the current value', () => {
const onChange = vi.fn();
const { getByLabelText } = render(<AccountSwitcher account={123} onChange={onChange} />);

expect(onChange).not.toHaveBeenCalled();
fireEvent.click(getByLabelText('Previous account'));
expect(onChange).toHaveBeenCalledWith(122);
});

it("does not render when we're at account 0", () => {
const { queryByLabelText } = render(<AccountSwitcher account={0} onChange={vi.fn()} />);

expect(queryByLabelText('Previous account')).toBeNull();
});

describe('when a filter has been passed', () => {
it('calls `onChange` with the next lower account index in the filter', () => {
const onChange = vi.fn();
const { getByLabelText } = render(
<AccountSwitcher account={123} onChange={onChange} filter={[123, 100]} />,
);

expect(onChange).not.toHaveBeenCalled();
fireEvent.click(getByLabelText('Previous account'));
expect(onChange).toHaveBeenCalledWith(100);
});

it("does not render when we're at the lowest account index in the filter", () => {
const { queryByLabelText } = render(
<AccountSwitcher account={100} onChange={vi.fn()} filter={[123, 100]} />,
);

expect(queryByLabelText('Previous account')).toBeNull();
});
});
});

describe('the next button', () => {
it('calls `onChange` with one more than the current value', () => {
const onChange = vi.fn();
const { getByLabelText } = render(<AccountSwitcher account={123} onChange={onChange} />);

expect(onChange).not.toHaveBeenCalled();
fireEvent.click(getByLabelText('Next account'));
expect(onChange).toHaveBeenCalledWith(124);
});

it("does not render when we're at the maximum account index", () => {
const { queryByLabelText } = render(<AccountSwitcher account={2 ** 32} onChange={vi.fn()} />);

expect(queryByLabelText('Next account')).toBeNull();
});

describe('when a filter has been passed', () => {
it('calls `onChange` with the next higher account index in the filter', () => {
const onChange = vi.fn();
const { getByLabelText } = render(
<AccountSwitcher account={100} onChange={onChange} filter={[123, 100]} />,
);

expect(onChange).not.toHaveBeenCalled();
fireEvent.click(getByLabelText('Next account'));
expect(onChange).toHaveBeenCalledWith(123);
});

it("does not render when we're at the highest account index in the filter", () => {
const { queryByLabelText } = render(
<AccountSwitcher account={123} onChange={vi.fn()} filter={[123, 100]} />,
);

expect(queryByLabelText('Next account')).toBeNull();
});
});
});
});
89 changes: 69 additions & 20 deletions packages/ui/components/ui/account-switcher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { ArrowLeftIcon, ArrowRightIcon } from 'lucide-react';
import { cn } from '../../lib/utils';
import { Button } from './button';
import { Input } from './input';
import { useState } from 'react';
import { useMemo, useState } from 'react';

const MAX_INDEX = 2 ** 32;

Expand All @@ -13,42 +13,89 @@ const MAX_INDEX = 2 ** 32;
export const AccountSwitcher = ({
account,
onChange,
filter,
}: {
account: number;
onChange: (account: number) => void;
/**
* An array of address indexes to switch between, if you want to limit the
* options. No need to sort them, as the component will do that for you.
*/
filter?: number[];
}) => {
const [inputCharWidth, setInputCharWidth] = useState(1);

const sortedFilter = useMemo(() => (filter ? [...filter].sort() : undefined), [filter]);

const handleClickPrevious = () => {
if (sortedFilter) {
const previousAccount = sortedFilter[sortedFilter.indexOf(account) - 1];
if (previousAccount !== undefined) onChange(previousAccount);
} else {
onChange(account - 1);
}
};

const handleClickNext = () => {
if (sortedFilter) {
const nextAccount = sortedFilter[sortedFilter.indexOf(account) + 1];
if (nextAccount !== undefined) onChange(nextAccount);
} else {
onChange(account + 1);
}
};

const shouldShowPreviousButton =
account !== 0 && (!sortedFilter || sortedFilter.indexOf(account) > 0);
const shouldShowNextButton =
account !== MAX_INDEX &&
(!sortedFilter || sortedFilter.indexOf(account) < sortedFilter.length - 1);

const handleChange = (value: number) => {
onChange(value);
setInputCharWidth(String(value).length);
};

return (
<div className='flex items-center justify-between'>
<Button variant='ghost' className={cn('hover:bg-inherit', account === 0 && 'cursor-default')}>
{account !== 0 ? (
{shouldShowPreviousButton ? (
<Button
variant='ghost'
className={cn('hover:bg-inherit', account === 0 && 'cursor-default')}
>
<ArrowLeftIcon
onClick={() => {
if (account > 0) handleChange(account - 1);
}}
aria-label='Previous account'
role='button'
onClick={handleClickPrevious}
className='size-6 hover:cursor-pointer'
/>
) : (
<span className='size-6' />
)}
</Button>
</Button>
) : (
<span className='size-6' />
)}
<div className='select-none text-center font-headline text-xl font-semibold leading-[30px]'>
<div className='flex flex-row flex-wrap items-end gap-[6px]'>
<span>Account</span>
<div className='flex items-end gap-0'>
<p>#</p>
<div className='relative w-min min-w-[24px]'>
<Input
aria-label={`Account #${account}`}
aria-disabled={!!filter}
variant='transparent'
type='number'
className='mb-[3px] h-6 py-[2px] font-headline text-xl font-semibold leading-[30px]'
onChange={e => {
/**
* Don't allow manual account number entry when there's a
* filter.
*
* @todo: Change this to only call `handleChange()` when the
* user presses Enter? Then it could validate that the entered
* account index is in the filter.
*/
if (filter) return;

const value = Number(e.target.value);
const valueLength = e.target.value.replace(/^0+/, '').length;

Expand All @@ -62,19 +109,21 @@ export const AccountSwitcher = ({
</div>
</div>
</div>
<Button
variant='ghost'
className={cn('hover:bg-inherit', account === MAX_INDEX && 'cursor-default')}
>
{account < MAX_INDEX ? (
{shouldShowNextButton ? (
<Button
variant='ghost'
className={cn('hover:bg-inherit', account === MAX_INDEX && 'cursor-default')}
>
<ArrowRightIcon
onClick={() => handleChange(account + 1)}
aria-label='Next account'
role='button'
onClick={handleClickNext}
className='size-6 hover:cursor-pointer'
/>
) : (
<span className='size-6' />
)}
</Button>
</Button>
) : (
<span className='size-6' />
)}
</div>
);
};

0 comments on commit 1a2a191

Please sign in to comment.