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

fix: only use 1 source wallet | memoize components that call balances #255

Merged
merged 4 commits into from
Feb 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
name: Build

on: [push, pull_request]

jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Use Bun
uses: oven-sh/setup-bun@v1
- name: Install dependencies
run: bun install
- name: Build
run: bun run build
2 changes: 0 additions & 2 deletions components/bank/components/__tests__/sendBox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ describe('SendBox', () => {

// Verify cross-chain elements are present
await waitFor(() => {
expect(screen.getByLabelText('from-chain-selector')).toBeInTheDocument();
expect(screen.getByLabelText('to-chain-selector')).toBeInTheDocument();
});
});
Expand All @@ -73,7 +72,6 @@ describe('SendBox', () => {
fireEvent.click(screen.getByLabelText('cross-chain-transfer-tab'));

await waitFor(() => {
expect(screen.getByLabelText('from-chain-selector')).toBeInTheDocument();
expect(screen.getByLabelText('to-chain-selector')).toBeInTheDocument();
});
});
Expand Down
30 changes: 9 additions & 21 deletions components/bank/components/sendBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import IbcSendForm from '../forms/ibcSendForm';
import env from '@/config/env';
import { CombinedBalanceInfo } from '@/utils/types';
import { ChainContext } from '@cosmos-kit/core';
import React from 'react';

export interface IbcChain {
id: string;
Expand All @@ -13,7 +14,7 @@ export interface IbcChain {
chainID: string;
}

export default function SendBox({
export default React.memo(function SendBox({
address,
balances,
isBalancesLoading,
Expand All @@ -23,11 +24,6 @@ export default function SendBox({
isGroup,
admin,
refetchProposals,
osmosisBalances,
isOsmosisBalancesLoading,
refetchOsmosisBalances,
resolveOsmosisRefetch,
chains,
}: {
address: string;
balances: CombinedBalanceInfo[];
Expand All @@ -38,25 +34,20 @@ export default function SendBox({
selectedDenom?: string;
isGroup?: boolean;
admin?: string;
osmosisBalances: CombinedBalanceInfo[];
isOsmosisBalancesLoading: boolean;
refetchOsmosisBalances: () => void;
resolveOsmosisRefetch: () => void;
chains: Record<string, ChainContext>;
}) {
const ibcChains = useMemo<IbcChain[]>(
() => [
{
id: env.chain,
name: 'Manifest',
icon: 'logo.svg',
icon: '/logo.svg',
prefix: 'manifest',
chainID: env.chainId,
},
{
id: env.osmosisChain,
name: 'Osmosis',
icon: 'osmosis.svg',
icon: '/osmosis.svg',
prefix: 'osmo',
chainID: env.osmosisChainId,
},
Expand All @@ -74,6 +65,8 @@ export default function SendBox({
const [selectedFromChain, setSelectedFromChain] = useState<IbcChain>(ibcChains[0]);
const [selectedToChain, setSelectedToChain] = useState<IbcChain>(ibcChains[1]);

const memoizedBalances = useMemo(() => balances, [balances]);

useEffect(() => {
if (selectedFromChain && selectedToChain && selectedFromChain.id === selectedToChain.id) {
// If chains match, switch the destination chain to the other available chain
Expand Down Expand Up @@ -132,25 +125,20 @@ export default function SendBox({
setSelectedToChain={setSelectedToChain}
address={address}
destinationChain={selectedToChain}
balances={balances}
balances={memoizedBalances}
isBalancesLoading={isBalancesLoading}
refetchBalances={refetchBalances}
refetchHistory={refetchHistory}
selectedDenom={selectedDenom}
osmosisBalances={osmosisBalances}
isGroup={isGroup}
admin={admin}
refetchProposals={refetchProposals}
isOsmosisBalancesLoading={isOsmosisBalancesLoading}
refetchOsmosisBalances={refetchOsmosisBalances}
resolveOsmosisRefetch={resolveOsmosisRefetch}
availableToChains={getAvailableToChains}
chains={chains}
/>
) : (
<SendForm
address={address}
balances={balances}
balances={memoizedBalances}
isBalancesLoading={isBalancesLoading}
refetchBalances={refetchBalances}
refetchHistory={refetchHistory}
Expand All @@ -165,4 +153,4 @@ export default function SendBox({
</div>
</div>
);
}
});
23 changes: 5 additions & 18 deletions components/bank/components/tokenList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,9 @@ interface TokenListProps {
admin?: string;
refetchProposals?: () => void;
searchTerm?: string;
osmosisBalances?: CombinedBalanceInfo[] | undefined;
isOsmosisBalancesLoading?: boolean;
refetchOsmosisBalances?: () => void;
resolveOsmosisRefetch?: () => void;
chains: Record<string, ChainContext>;
}

export function TokenList(props: Readonly<TokenListProps>) {
export const TokenList = React.memo(function TokenList(props: Readonly<TokenListProps>) {
const {
balances,
isLoading,
Expand All @@ -36,11 +31,6 @@ export function TokenList(props: Readonly<TokenListProps>) {
admin,
refetchProposals,
searchTerm = '',
osmosisBalances,
isOsmosisBalancesLoading,
refetchOsmosisBalances,
resolveOsmosisRefetch,
chains,
} = props;
const [selectedDenom, setSelectedDenom] = useState<any>(null);
const [isSendModalOpen, setIsSendModalOpen] = useState(false);
Expand Down Expand Up @@ -91,6 +81,8 @@ export function TokenList(props: Readonly<TokenListProps>) {
[totalPages]
);

const memoizedBalances = useMemo(() => props.balances ?? [], [props.balances]);

return (
<div className="w-full mx-auto rounded-[24px] h-full flex flex-col">
<div className="flex-1 overflow-y-auto">
Expand Down Expand Up @@ -228,7 +220,7 @@ export function TokenList(props: Readonly<TokenListProps>) {
modalId="send-modal"
isOpen={isSendModalOpen}
address={address}
balances={balances ?? []}
balances={memoizedBalances}
isBalancesLoading={isLoading}
refetchBalances={refetchBalances}
refetchHistory={refetchHistory}
Expand All @@ -237,12 +229,7 @@ export function TokenList(props: Readonly<TokenListProps>) {
isGroup={isGroup}
admin={admin}
refetchProposals={refetchProposals}
osmosisBalances={osmosisBalances ?? []}
isOsmosisBalancesLoading={isOsmosisBalancesLoading ?? false}
refetchOsmosisBalances={refetchOsmosisBalances ?? (() => {})}
resolveOsmosisRefetch={resolveOsmosisRefetch ?? (() => {})}
chains={chains}
/>
</div>
);
}
});
72 changes: 12 additions & 60 deletions components/bank/forms/__tests__/ibcSendForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@ mock.module('next/router', () => ({
}),
}));

// Add this mock before the tests
mock.module('next/image', () => ({
default: (props: any) => {
// eslint-disable-next-line @next/next/no-img-element
return <img {...props} alt={props.alt || ''} />;
},
__esModule: true,
}));

function renderWithProps(props = {}) {
const defaultChains = [
{
Expand Down Expand Up @@ -87,7 +96,6 @@ describe('IbcSendForm Component', () => {
const { findForm } = renderWithProps();
const form = await findForm();
expect(form).toBeInTheDocument();
expect(screen.getByLabelText('from-chain-selector')).toBeInTheDocument();
expect(screen.getByLabelText('to-chain-selector')).toBeInTheDocument();
});

Expand All @@ -114,25 +122,10 @@ describe('IbcSendForm Component', () => {

test('updates chain selector correctly', () => {
renderWithProps();

const fromChainSelector = screen.getByLabelText('from-chain-selector');
fireEvent.click(fromChainSelector);

// Get all Manifest options and select the enabled one
const manifestOptions = screen.getAllByRole('option', { name: 'Manifest' });
const enabledManifestOption = manifestOptions.find(
option => !option.className.includes('opacity-50')
);
fireEvent.click(enabledManifestOption!);

expect(screen.getByLabelText('from-chain-selector')).toHaveTextContent('Manifest');

const toChainSelector = screen.getByLabelText('to-chain-selector');
fireEvent.click(toChainSelector);

const osmosisOption = screen.getAllByRole('option', { name: 'Osmosis' });
fireEvent.click(osmosisOption[0]);

expect(screen.getByLabelText('to-chain-selector')).toHaveTextContent('Osmosis');
});

Expand Down Expand Up @@ -167,69 +160,28 @@ describe('IbcSendForm Component', () => {

test('handles chain selection correctly', async () => {
renderWithProps();

const fromChainSelector = screen.getByLabelText('from-chain-selector');
fireEvent.click(fromChainSelector);

const manifestOptions = screen.getAllByRole('option', { name: 'Manifest' });
const enabledManifestOption = manifestOptions.find(
option => !option.className.includes('opacity-50')
);
fireEvent.click(enabledManifestOption!);

expect(screen.getByLabelText('from-chain-selector')).toHaveTextContent('Manifest');

const toChainSelector = screen.getByLabelText('to-chain-selector');
fireEvent.click(toChainSelector);

const osmosisOption = screen.getAllByRole('option', { name: 'Osmosis' });
fireEvent.click(osmosisOption[0]);

expect(screen.getByLabelText('to-chain-selector')).toHaveTextContent('Osmosis');
});

test('prevents selecting same chain for source and destination', async () => {
// cant select from chain anymore hardcoded to manifest
test.skip('prevents selecting same chain for source and destination', async () => {
const { findForm } = renderWithProps();
await findForm(); // Wait for form to be mounted

const fromChainSelector = screen.getByLabelText('from-chain-selector');
await act(async () => {
fireEvent.click(fromChainSelector);
});

// Wait for dropdown content to be visible
await act(async () => {
const manifestOptions = await screen.findAllByRole('option', {
name: 'Manifest',
hidden: true,
});

const enabledManifestOption = manifestOptions.find(
option => !option.className.includes('opacity-50')
);
fireEvent.click(enabledManifestOption!);
});

expect(screen.getByLabelText('from-chain-selector')).toHaveTextContent('Manifest');

await findForm();
const toChainSelector = screen.getByLabelText('to-chain-selector');
await act(async () => {
fireEvent.click(toChainSelector);
});

// Wait for dropdown content to be visible
await act(async () => {
const toChainOptions = await screen.findAllByRole('option', {
hidden: true,
});

// Find the Manifest option in the to-chain dropdown
const manifestInToChain = toChainOptions.find(option => {
const link = option.querySelector('a');
return link && link.textContent?.includes('Manifest');
});

// Check that the Manifest option has the disabled styling and attributes
expect(manifestInToChain?.querySelector('a')).toHaveStyle({ pointerEvents: 'none' });
expect(manifestInToChain?.querySelector('a')).toHaveClass('opacity-50');
});
Expand Down
Loading