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

Migrate Account component to a functional component and use the new useTransactions hook #3708

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2,890 changes: 1,310 additions & 1,580 deletions packages/desktop-client/src/components/accounts/Account.tsx

Large diffs are not rendered by default.

126 changes: 70 additions & 56 deletions packages/desktop-client/src/components/accounts/Balance.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useRef } from 'react';
import React, { useMemo } from 'react';
import { useTranslation } from 'react-i18next';

import { useHover } from 'usehooks-ts';
import { css } from '@emotion/css';

import { isPreviewId } from 'loot-core/shared/transactions';
import { useCachedSchedules } from 'loot-core/src/client/data-hooks/schedules';
Expand All @@ -12,7 +12,7 @@ import { type AccountEntity } from 'loot-core/types/models';
import { useSelectedItems } from '../../hooks/useSelected';
import { SvgArrowButtonRight1 } from '../../icons/v2';
import { theme } from '../../style';
import { Button } from '../common/Button2';
import { ButtonWithLoading } from '../common/Button2';
import { Text } from '../common/Text';
import { View } from '../common/View';
import { PrivacyFilter } from '../PrivacyFilter';
Expand Down Expand Up @@ -56,10 +56,10 @@ function DetailedBalance({

type SelectedBalanceProps = {
selectedItems: Set<string>;
account?: AccountEntity;
accountId?: AccountEntity['id'];
};

function SelectedBalance({ selectedItems, account }: SelectedBalanceProps) {
function SelectedBalance({ selectedItems, accountId }: SelectedBalanceProps) {
const { t } = useTranslation();

const name = `selected-balance-${[...selectedItems].join('-')}`;
Expand Down Expand Up @@ -104,7 +104,7 @@ function SelectedBalance({ selectedItems, account }: SelectedBalanceProps) {
isExactBalance = false;
}

if (!account || account.id === s._account) {
if (accountId !== s._account) {
scheduleBalance += getScheduledAmount(s._amount);
} else {
scheduleBalance -= getScheduledAmount(s._amount);
Expand All @@ -128,39 +128,53 @@ function SelectedBalance({ selectedItems, account }: SelectedBalanceProps) {
}

type FilteredBalanceProps = {
filteredAmount?: number | null;
transactionsQuery: Query;
};

function FilteredBalance({ filteredAmount }: FilteredBalanceProps) {
function FilteredBalance({ transactionsQuery }: FilteredBalanceProps) {
const { t } = useTranslation();
const filteredBalance = useSheetValue<'balance', 'filtered-balance'>({
name: 'filtered-balance',
query: transactionsQuery.calculate({ $sum: '$amount' }),
value: 0,
});

return (
<DetailedBalance
name={t('Filtered balance:')}
balance={filteredAmount ?? 0}
balance={filteredBalance || 0}
isExactBalance={true}
/>
);
}

type MoreBalancesProps = {
balanceQuery: { name: `balance-query-${string}`; query: Query };
accountId: AccountEntity['id'] | string;
balanceQuery: Query;
};

function MoreBalances({ balanceQuery }: MoreBalancesProps) {
function MoreBalances({ accountId, balanceQuery }: MoreBalancesProps) {
const { t } = useTranslation();

const clearedQuery = useMemo(
() => balanceQuery.filter({ cleared: true }),
[balanceQuery],
);
const cleared = useSheetValue<'balance', `balance-query-${string}-cleared`>({
name: (balanceQuery.name + '-cleared') as `balance-query-${string}-cleared`,
query: balanceQuery.query.filter({ cleared: true }),
name: `balance-query-${accountId}-cleared`,
query: clearedQuery,
});

const unclearedQuery = useMemo(
() => balanceQuery.filter({ cleared: false }),
[balanceQuery],
);
const uncleared = useSheetValue<
'balance',
`balance-query-${string}-uncleared`
>({
name: (balanceQuery.name +
'-uncleared') as `balance-query-${string}-uncleared`,
query: balanceQuery.query.filter({ cleared: false }),
name: `balance-query-${accountId}-uncleared`,
query: unclearedQuery,
});

return (
Expand All @@ -172,25 +186,31 @@ function MoreBalances({ balanceQuery }: MoreBalancesProps) {
}

type BalancesProps = {
balanceQuery: { name: `balance-query-${string}`; query: Query };
accountId?: AccountEntity['id'] | string;
showFilteredBalance: boolean;
transactionsQuery?: Query;
balanceQuery: Query;
showExtraBalances: boolean;
onToggleExtraBalances: () => void;
account?: AccountEntity;
isFiltered: boolean;
filteredAmount?: number | null;
};

export function Balances({
accountId,
balanceQuery,
transactionsQuery,
showFilteredBalance,
showExtraBalances,
onToggleExtraBalances,
account,
isFiltered,
filteredAmount,
}: BalancesProps) {
const selectedItems = useSelectedItems();
const buttonRef = useRef(null);
const isButtonHovered = useHover(buttonRef);
const balanceBinding = useMemo<Binding<'balance', `balance-query-${string}`>>(
() => ({
name: `balance-query-${accountId}`,
query: balanceQuery,
value: 0,
}),
[accountId, balanceQuery],
);

return (
<View
Expand All @@ -201,25 +221,28 @@ export function Balances({
marginLeft: -5,
}}
>
<Button
ref={buttonRef}
<ButtonWithLoading
isLoading={!balanceQuery}
data-testid="account-balance"
variant="bare"
onPress={onToggleExtraBalances}
style={{
className={css({
paddingTop: 1,
paddingBottom: 1,
}}
[`& svg`]: {
width: 10,
height: 10,
marginLeft: 10,
color: theme.pillText,
transform: showExtraBalances ? 'rotateZ(180deg)' : 'rotateZ(0)',
opacity: selectedItems.size > 0 || showExtraBalances ? 1 : 0,
},
[`&[data-hovered] svg`]: {
opacity: 1,
},
})}
>
<CellValue
binding={
{ ...balanceQuery, value: 0 } as Binding<
'balance',
`balance-query-${string}`
>
}
type="financial"
>
<CellValue binding={balanceBinding} type="financial">
{props => (
<CellValueText
{...props}
Expand All @@ -237,26 +260,17 @@ export function Balances({
)}
</CellValue>

<SvgArrowButtonRight1
style={{
width: 10,
height: 10,
marginLeft: 10,
color: theme.pillText,
transform: showExtraBalances ? 'rotateZ(180deg)' : 'rotateZ(0)',
opacity:
isButtonHovered || selectedItems.size > 0 || showExtraBalances
? 1
: 0,
}}
/>
</Button>
{showExtraBalances && <MoreBalances balanceQuery={balanceQuery} />}

<SvgArrowButtonRight1 />
</ButtonWithLoading>
{showExtraBalances && accountId && balanceQuery && (
<MoreBalances accountId={accountId} balanceQuery={balanceQuery} />
)}
{selectedItems.size > 0 && (
Comment on lines +266 to +267
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

Ensure accountId is defined before passing to MoreBalances

In Balances, accountId is passed to MoreBalances, which expects a defined accountId. Since accountId might be undefined, this could lead to runtime errors.

After making accountId required in the previous comment, verify that all instances of Balances are provided with a valid accountId. If accountId can be undefined, consider adding a conditional rendering:

 {showExtraBalances && balanceQuery && accountId && (
   <MoreBalances accountId={accountId} balanceQuery={balanceQuery} />
 )}

However, it's recommended to ensure accountId is always defined if MoreBalances is to be rendered.

📝 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
<MoreBalances accountId={accountId} balanceQuery={balanceQuery} />
)}
{showExtraBalances && balanceQuery && accountId && (
<MoreBalances accountId={accountId} balanceQuery={balanceQuery} />
)}
🧰 Tools
🪛 GitHub Check: typecheck

[failure] 260-260:
Type 'string | undefined' is not assignable to type 'string'.

🪛 GitHub Actions: Test

[error] 260-260: Type 'string | undefined' is not assignable to type 'string'

<SelectedBalance selectedItems={selectedItems} account={account} />
<SelectedBalance selectedItems={selectedItems} accountId={accountId} />
)}
{showFilteredBalance && transactionsQuery && (
<FilteredBalance transactionsQuery={transactionsQuery} />
)}
{isFiltered && <FilteredBalance filteredAmount={filteredAmount} />}
</View>
);
}
Loading
Loading