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

V0.5.0 rc1 #583

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,5 @@ yarn-error.log*
fm_*
misc/test/data
misc/test/tls.key

todo.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import React, { useMemo } from 'react';
import { Box, Flex, Text, Square } from '@chakra-ui/react';
import { formatValue, useTranslation } from '@fedimint/utils';
import { MSats } from '@fedimint/types';
import { PieChart } from 'react-minimal-pie-chart';
import { useGatewayContext } from '../../../hooks';
import { useBalanceCalculations } from '../../hooks/useBalanceCalculations';

export const BalancesSummary = React.memo(
function BalancesSummary(): JSX.Element {
const { t } = useTranslation();
const { state } = useGatewayContext();
const balanceAmounts = useBalanceCalculations(state.balances ?? undefined);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle undefined balances more gracefully

The useBalanceCalculations hook receives undefined when state.balances is null, but there's no handling for this case in the component.

Consider adding a loading state or default values:

- const balanceAmounts = useBalanceCalculations(state.balances ?? undefined);
+ const balanceAmounts = useBalanceCalculations(state.balances ?? { ecash: 0, lightning: 0, onchain: 0 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const balanceAmounts = useBalanceCalculations(state.balances ?? undefined);
const balanceAmounts = useBalanceCalculations(state.balances ?? { ecash: 0, lightning: 0, onchain: 0 });


const balanceData = useMemo(
() => [
{
title: 'Ecash',
value: balanceAmounts.ecash,
formattedValue: formatValue(
balanceAmounts.ecash as MSats,
state.unit,
true
),
color: '#FF6384',
},
{
title: 'Lightning',
value: balanceAmounts.lightning,
formattedValue: formatValue(
balanceAmounts.lightning as MSats,
state.unit,
true
),
color: '#36A2EB',
},
{
title: 'Onchain',
value: balanceAmounts.onchain,
formattedValue: formatValue(
balanceAmounts.onchain as MSats,
state.unit,
true
),
Comment on lines +20 to +44
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove redundant type casting

The MSats type casting is redundant as the values from balanceAmounts should already be of the correct type.

- formattedValue: formatValue(balanceAmounts.ecash as MSats, state.unit, true),
+ formattedValue: formatValue(balanceAmounts.ecash, state.unit, true),

Committable suggestion skipped: line range outside the PR's diff.

color: '#FFCE56',
},
],
[balanceAmounts, state.unit]
);

return (
<Box p={6} bg='white' borderRadius='lg' borderWidth='1px' mb={4}>
<Text fontSize='xl' mb={4}>
{t('wallet.balances-summary')}
</Text>
<Flex justify='space-between' align='center'>
<Flex direction='column' flex={1}>
{balanceData.map((item) => (
<Flex key={item.title} mb={2} align='center'>
<Square size='3' bg={item.color} mr={2} />
<Text color='gray.600' flex='1'>
{item.title}:
</Text>
<Text fontWeight='medium'>{item.formattedValue}</Text>
</Flex>
))}
<Flex borderTopWidth={1} mt={3} pt={2}>
<Text fontWeight='semibold' flex='1'>
Total:
</Text>
<Text fontWeight='semibold'>
{formatValue(balanceAmounts.total as MSats, state.unit, true)}
</Text>
</Flex>
</Flex>
<Box w='150px' h='150px' ml={8}>
<PieChart
data={balanceData}
lineWidth={40}
paddingAngle={2}
startAngle={-90}
animate
/>
</Box>
</Flex>
</Box>
);
}
);
59 changes: 59 additions & 0 deletions apps/router/src/gateway-ui/components/walletCard/EcashCard.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import React from 'react';
import { Box, Button, Flex, Text } from '@chakra-ui/react';
import { useTranslation } from '@fedimint/utils';
import { FaArrowDown, FaArrowUp } from 'react-icons/fa';
import { useGatewayContext } from '../../../hooks';
import {
GATEWAY_APP_ACTION_TYPE,
WalletModalAction,
WalletModalType,
} from '../../../types/gateway';

export const EcashCard = React.memo(function EcashCard(): JSX.Element {
const { t } = useTranslation();
const { state, dispatch } = useGatewayContext();

const handleModalOpen = (action: WalletModalAction) => {
const federations = state.gatewayInfo?.federations;
if (!federations?.length) return;

dispatch({
type: GATEWAY_APP_ACTION_TYPE.SET_WALLET_MODAL_STATE,
payload: {
action,
type: WalletModalType.Ecash,
selectedFederation: federations[0],
showSelector: true,
isOpen: true,
},
});
};

return (
<Box p={6} bg='white' borderRadius='lg' borderWidth='1px' mb={4}>
<Flex justify='space-between' align='center' mb={4}>
<Text fontSize='xl'>{t('wallet.ecash')}</Text>
<Flex gap={2}>
<Button
leftIcon={<FaArrowDown />}
colorScheme='blue'
variant='solid'
onClick={() => handleModalOpen(WalletModalAction.Receive)}
>
{t('wallet.receive')}
</Button>
<Button
leftIcon={<FaArrowUp />}
variant='outline'
onClick={() => handleModalOpen(WalletModalAction.Send)}
>
{t('wallet.send')}
</Button>
</Flex>
</Flex>
<Text color='gray.600'>
Send and receive federated ecash between federation members
Copy link
Member

Choose a reason for hiding this comment

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

TODO: add translation

</Text>
</Box>
);
});
Comment on lines +16 to +59
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider refactoring duplicated code into a reusable component

The EcashCard component shares significant code with the LightningCard component. To improve maintainability and reduce duplication, consider abstracting the shared logic into a common component.

Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import React from 'react';
import { Box, Button, Flex, Text } from '@chakra-ui/react';
import { useTranslation } from '@fedimint/utils';
import { FaArrowDown, FaArrowUp } from 'react-icons/fa';
import { useGatewayContext } from '../../../hooks';
import {
GATEWAY_APP_ACTION_TYPE,
WalletModalAction,
WalletModalType,
} from '../../../types/gateway';

export const LightningCard = React.memo(function LightningCard(): JSX.Element {
const { t } = useTranslation();
const { state, dispatch } = useGatewayContext();

const handleModalOpen = (action: WalletModalAction) => {
const federations = state.gatewayInfo?.federations;
if (!federations?.length) return;

dispatch({
type: GATEWAY_APP_ACTION_TYPE.SET_WALLET_MODAL_STATE,
payload: {
action,
type: WalletModalType.Lightning,
selectedFederation: federations[0],
showSelector: true,
isOpen: true,
},
});
};

return (
<Box p={6} bg='white' borderRadius='lg' borderWidth='1px' mb={4}>
<Flex justify='space-between' align='center' mb={4}>
<Text fontSize='xl'>{t('wallet.lightning-payments')}</Text>
<Flex gap={2}>
<Button
leftIcon={<FaArrowDown />}
colorScheme='blue'
variant='solid'
onClick={() => handleModalOpen(WalletModalAction.Receive)}
>
{t('wallet.receive')}
</Button>
<Button
leftIcon={<FaArrowUp />}
variant='outline'
onClick={() => handleModalOpen(WalletModalAction.Send)}
>
{t('wallet.send')}
</Button>
</Flex>
</Flex>
<Text color='gray.600'>
Send and receive Lightning Network payments through the federation
</Text>
</Box>
);
});
Comment on lines +16 to +59
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 9, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider refactoring duplicated code into a reusable component

The LightningCard component has substantial overlap with the EcashCard component in terms of structure and functionality. Refactoring the common code into a shared component can enhance maintainability and reduce redundancy.

Copy link
Member

Choose a reason for hiding this comment

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

TODO ^^

Copy link
Contributor

Choose a reason for hiding this comment

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

@alexlwn123, understood. Would you like me to open a GitHub issue to track this refactoring suggestion?

Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import React from 'react';
import { Box, Button, Flex, Text } from '@chakra-ui/react';
import { useTranslation } from '@fedimint/utils';
import { FaArrowDown, FaArrowUp } from 'react-icons/fa';
import { useGatewayContext } from '../../../hooks';
import {
GATEWAY_APP_ACTION_TYPE,
WalletModalAction,
WalletModalType,
} from '../../../types/gateway';

export const OnchainCard = React.memo(function OnchainCard(): JSX.Element {
const { t } = useTranslation();
const { state, dispatch } = useGatewayContext();

const handleModalOpen = (action: WalletModalAction) => {
const federations = state.gatewayInfo?.federations;
if (!federations?.length) return;

Comment on lines +16 to +19
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 9, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error feedback for missing federations

The function silently returns when no federations are available. Consider showing a toast or error message to inform users.

Copy link
Member

Choose a reason for hiding this comment

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

TODO: ^^^

maybe create a hook that guards the specific operations we're using within the useGatewayContext hook

Copy link
Contributor

Choose a reason for hiding this comment

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

@alexlwn123, that's a great idea. Implementing a hook to guard the operations within useGatewayContext can ensure proper handling when federations are missing. Here's an example of how you might create such a hook:

// In useGatewayContext.tsx
export function useGuardedGatewayContext() {
  const context = useGatewayContext();

  const { state, dispatch } = context;

  if (!state.gatewayInfo?.federations?.length) {
    // Handle the case when no federations are available
    // For example, show an error message or redirect the user
    // dispatch an action or set a state to show an error message
  }

  return { state, dispatch };
}

Then, in your component, you can use the guarded context:

const { state, dispatch } = useGuardedGatewayContext();

This approach ensures that any component using the context will handle missing federations appropriately.

dispatch({
type: GATEWAY_APP_ACTION_TYPE.SET_WALLET_MODAL_STATE,
payload: {
action,
type: WalletModalType.Onchain,
selectedFederation: federations[0],
showSelector: true,
isOpen: true,
},
});
};

return (
<Box p={6} bg='white' borderRadius='lg' borderWidth='1px'>
<Text fontSize='xl' mb={6}>
Onchain
</Text>

{/* Federation Balances Section */}
<Box mb={6}>
<Flex justify='space-between' align='center' mb={4}>
<Text fontSize='md' fontWeight='medium'>
{t('wallet.federation-balances')}
</Text>
<Flex gap={2}>
<Button
leftIcon={<FaArrowDown />}
colorScheme='blue'
variant='solid'
onClick={() => handleModalOpen(WalletModalAction.Receive)}
>
{t('wallet.peg-in')}
</Button>
<Button
leftIcon={<FaArrowUp />}
variant='outline'
onClick={() => handleModalOpen(WalletModalAction.Send)}
>
{t('wallet.peg-out')}
</Button>
</Flex>
</Flex>
<Text color='gray.600'>
Deposit and withdraw bitcoin to/from the federation
</Text>
</Box>

{/* Node's Onchain Wallet Section */}
<Box>
<Flex justify='space-between' align='center' mb={4}>
<Text fontSize='md' fontWeight='medium'>
{t('wallet.node-onchain-wallet')}
</Text>
<Flex gap={2}>
<Button
leftIcon={<FaArrowDown />}
colorScheme='blue'
variant='solid'
>
{t('wallet.receive')}
</Button>
<Button leftIcon={<FaArrowUp />} variant='outline'>
{t('wallet.send')}
</Button>
Comment on lines +75 to +83
Copy link
Member

Choose a reason for hiding this comment

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

TODO: Implement send/receive onchain

</Flex>
Comment on lines +74 to +84
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add missing onClick handlers for node wallet buttons

The receive and send buttons in the node's onchain wallet section are non-functional.

</Flex>
<Text color='gray.600'>
Manage bitcoin in the gateway&apos;s onchain wallet
</Text>
</Box>
</Box>
);
});
Loading
Loading