Convert Mobile Transaction component to TransactionListItem + cleanup #3761
Conversation
actual-github-bot
bot
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller
Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
joel-jeremy
force-pushed
the
ts-migrate-mobile-Transaction-component
branch
from
bad17ba to
5c3798c
Compare
joel-jeremy
changed the title
WalkthroughThis pull request introduces several changes primarily focused on updating dependencies and modifying components within the Possibly related PRs
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-eslint-plugin". (The package "eslint-plugin-eslint-plugin" was not found when loaded as a Node module from the directory "/packages/eslint-plugin-actual".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-eslint-plugin" was referenced from the config file in "packages/eslint-plugin-actual/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
packages/desktop-client/src/components/mobile/transactions/TransactionListItem.tsx (2)
110-116: Simplify nested ternary operators for better readabilityThe nested ternary operators used to determine
specialCategorycan make the code harder to read and maintain. Refactoring to useif...elsestatements enhances clarity.Apply this diff to improve readability:
- const specialCategory = account?.offbudget - ? 'Off Budget' - : transferAcct && !transferAcct.offbudget - ? 'Transfer' - : isParent - ? 'Split' - : null; + let specialCategory = null; + + if (account?.offbudget) { + specialCategory = 'Off Budget'; + } else if (transferAcct && !transferAcct.offbudget) { + specialCategory = 'Transfer'; + } else if (isParent) { + specialCategory = 'Split'; + }
135-160: Extract inline styles to improve performanceInline styling within the
Buttoncomponent creates new style objects on every render, which can impact performance. Consider extracting these styles into a constant or a stylesheet to enhance performance and maintainability.Apply this diff to extract the styles:
+ const buttonStyle = { + userSelect: 'none', + height: ROW_HEIGHT, + width: '100%', + borderRadius: 0, + ...(isSelected + ? { + borderWidth: '0 0 0 4px', + borderColor: theme.mobileTransactionSelected, + borderStyle: 'solid', + } + : { + borderWidth: '0 0 1px 0', + borderColor: theme.tableBorder, + borderStyle: 'solid', + }), + ...(isPreview + ? { + backgroundColor: theme.tableRowHeaderBackground, + } + : { + backgroundColor: theme.tableBackground, + }), + }; <Button - style={{ - userSelect: 'none', - height: ROW_HEIGHT, - width: '100%', - borderRadius: 0, - ...(isSelected - ? { - borderWidth: '0 0 0 4px', - borderColor: theme.mobileTransactionSelected, - borderStyle: 'solid', - } - : { - borderWidth: '0 0 1px 0', - borderColor: theme.tableBorder, - borderStyle: 'solid', - }), - ...(isPreview - ? { - backgroundColor: theme.tableRowHeaderBackground, - } - : { - backgroundColor: theme.tableBackground, - }), - }} + style={buttonStyle} >packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (1)
153-158: Optimize event handlers to prevent unnecessary re-rendersCurrently, new arrow functions are being created for each
TransactionListItem, which can lead to performance issues due to unnecessary re-renders. Bind the event handlers outside the render method or pass the handlers directly if possible.Apply this diff to optimize the event handlers:
<TransactionListItem key={transaction.id} value={transaction} - onPress={trans => onTransactionPress(trans)} - onLongPress={trans => onTransactionPress(trans, true)} + onPress={onTransactionPress} + onLongPress={event => onTransactionPress(event, true)} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (17)
packages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-individual-account-page-and-checks-that-filtering-is-working-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-individual-account-page-and-checks-that-filtering-is-working-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-individual-account-page-and-checks-that-filtering-is-working-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-individual-account-page-and-checks-that-filtering-is-working-7-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-individual-account-page-and-checks-that-filtering-is-working-8-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-individual-account-page-and-checks-that-filtering-is-working-9-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--c18ad-l-redirects-to-the-category-transactions-page-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--e995e-l-redirects-to-the-category-transactions-page-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--ff568-l-redirects-to-the-category-transactions-page-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--11290-l-redirects-to-the-category-transactions-page-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--57d88-l-redirects-to-the-category-transactions-page-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--5d90c-l-redirects-to-the-category-transactions-page-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-7-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-8-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-9-chromium-linux.pngis excluded by!**/*.pngupcoming-release-notes/3761.mdis excluded by!**/*.mdyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
packages/desktop-client/package.json(2 hunks)packages/desktop-client/src/components/mobile/transactions/ListBox.jsx(0 hunks)packages/desktop-client/src/components/mobile/transactions/ListBoxSection.jsx(0 hunks)packages/desktop-client/src/components/mobile/transactions/Option.jsx(0 hunks)packages/desktop-client/src/components/mobile/transactions/Transaction.jsx(0 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx(4 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionListItem.tsx(1 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionListWithBalances.jsx(0 hunks)packages/loot-core/src/shared/months.ts(1 hunks)
💤 Files with no reviewable changes (5)
- packages/desktop-client/src/components/mobile/transactions/ListBox.jsx
- packages/desktop-client/src/components/mobile/transactions/ListBoxSection.jsx
- packages/desktop-client/src/components/mobile/transactions/Option.jsx
- packages/desktop-client/src/components/mobile/transactions/Transaction.jsx
- packages/desktop-client/src/components/mobile/transactions/TransactionListWithBalances.jsx
✅ Files skipped from review due to trivial changes (1)
- packages/loot-core/src/shared/months.ts
🔇 Additional comments (4)
packages/desktop-client/package.json (2)
53-54: LGTM: Minor version updates for react-aria packages.
The minor version bumps for react-aria (^3.35.1) and react-aria-components (^1.4.1) are appropriate for the component conversion work being done.
68-68: Verify react-stately version compatibility.
The significant version change of react-stately from ^3.10.9 to ^3.33.0 could introduce breaking changes. Please ensure this version is compatible with the react-aria-components being used.
✅ Verification successful
The react-stately version update is safe to proceed.
The version change from ^3.10.9 to ^3.33.0 is compatible with the react-aria-components being used. From the yarn.lock analysis:
- [email protected] explicitly lists react-stately@^3.33.0 as a dependency
- All individual @react-stately/* packages are properly aligned with this version
- No conflicting peer dependency requirements were found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential compatibility issues between react-stately and react-aria-components
# Test: Search for react-stately imports and usages to assess impact
rg -A 5 'from.*react-stately'
# Test: Check package.json in node_modules for peer dependency requirements
cat node_modules/react-aria-components/package.json | jq '.peerDependencies["react-stately"]'
Length of output: 207
Script:
#!/bin/bash
# Let's try a different approach to verify react-stately usage and compatibility
# Check direct imports of react-stately in the codebase
rg -l 'from.*react-stately'
# Look for specific react-stately hooks or components being used
rg -l 'use.*State|useSelectState|useComboBoxState|useListState'
# Check if there's a yarn.lock or package-lock.json to see resolved versions
fd -e lock
# If found, let's examine the resolved versions in the lockfile
if [ -f "yarn.lock" ]; then
rg -A 5 'react-stately|react-aria-components' yarn.lock
elif [ -f "package-lock.json" ]; then
rg -A 5 'react-stately|react-aria-components' package-lock.json
fi
Length of output: 35078
packages/desktop-client/src/components/mobile/transactions/TransactionListItem.tsx (1)
64-79: Potential conflict when merging usePress and useLongPress props
Merging pressProps and longPressProps might lead to unintended behavior or event handling conflicts. Consider using the Pressable component from @react-aria/interactions, which natively supports both onPress and onLongPress events. This can simplify the code and ensure consistent behavior across devices.
packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (1)
8-8: Confirm imports from 'react-aria-components'
Ensure that all imported components (ListBox, Section, Header, Collection) are correctly exported from react-aria-components. Incorrect imports can lead to runtime errors if the components are not available.
Run the following script to confirm the availability of these components:
✅ Verification successful
Components from react-aria-components are properly used and imported
The verification confirms that:
react-aria-componentsis correctly listed as a dependency in package.json with version "^1.4.1"- The components (
ListBox,Section,Header,Collection) are actively used in the TransactionList component with proper implementation - Multiple other files in the codebase also use these components successfully
- The code shows proper integration of these components in a working list implementation with sections and headers
The import statement is correct and the components are being used as intended.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that 'react-aria-components' exports the required components.
# Test: List exported members from 'react-aria-components' package.
# Expect: 'ListBox', 'Section', 'Header', and 'Collection' should be listed.
grep -E 'export.*(ListBox|Section|Header|Collection)' node_modules/react-aria-components/dist/index.esm.js
Length of output: 194
Script:
#!/bin/bash
# Let's first check if the package exists in node_modules and its structure
fd -t d "react-aria-components" node_modules
# Then let's check the actual usage of these components in the codebase
ast-grep --pattern 'import { $$$ListBox$$$, $$$Section$$$, $$$Header$$$, $$$Collection$$$ } from "react-aria-components"'
# Also search for actual usage of these components
rg "<(ListBox|Section|Header|Collection)" -l
Length of output: 1147
Script:
#!/bin/bash
# Let's check the package.json to verify the dependency
rg '"react-aria-components"' packages/desktop-client/package.json
# Let's also check how these components are actually used in TransactionList.jsx
cat packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx
Length of output: 16271
packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx
Show resolved
Hide resolved
packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx
Outdated
Show resolved
Hide resolved
joel-jeremy
force-pushed
the
ts-migrate-mobile-Transaction-component
branch
from
ce3be9f to
162a0b9
Compare
joel-jeremy
force-pushed
the
ts-migrate-mobile-Transaction-component
branch
from
162a0b9 to
3756d76
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (3)
73-83: Consider optimizing transaction press handlerThe current implementation could be more efficient by:
- Moving the isPreview check before the callback creation
- Separating long press and normal press into different handlers
+ const isPreviewTransaction = useCallback( + (transaction) => isPreviewId(transaction.id), + [] + ); + + const onTransactionLongPress = useCallback( + (transaction) => { + if (!isPreviewTransaction(transaction)) { + dispatchSelected({ type: 'select', id: transaction.id }); + } + }, + [dispatchSelected, isPreviewTransaction] + ); + - const onTransactionPress = useCallback( - (transaction, isLongPress = false) => { - const isPreview = isPreviewId(transaction.id); - if (!isPreview && (isLongPress || selectedTransactions.size > 0)) { - dispatchSelected({ type: 'select', id: transaction.id }); - } else { - onOpenTransaction(transaction); - } - }, - [dispatchSelected, onOpenTransaction, selectedTransactions.size], - ); + const onTransactionPress = useCallback( + (transaction) => { + if (!isPreviewTransaction(transaction) && selectedTransactions.size > 0) { + dispatchSelected({ type: 'select', id: transaction.id }); + } else { + onOpenTransaction(transaction); + } + }, + [dispatchSelected, onOpenTransaction, selectedTransactions.size, isPreviewTransaction] + );
131-144: Consider extracting header styles to a separate constantThe inline styles in the Header component could be moved to a constant or styled-component for better maintainability and reusability.
+ const headerStyles = { + ...styles.smallText, + backgroundColor: theme.pageBackground, + color: theme.tableHeaderText, + display: 'flex', + justifyContent: 'center', + paddingBottom: 4, + paddingTop: 4, + position: 'sticky', + top: '0', + width: '100%', + zIndex: 10, + }; <Header - style={{ - ...styles.smallText, - backgroundColor: theme.pageBackground, - color: theme.tableHeaderText, - display: 'flex', - justifyContent: 'center', - paddingBottom: 4, - paddingTop: 4, - position: 'sticky', - top: '0', - width: '100%', - zIndex: 10, - }} + style={headerStyles} >
148-153: Consider memoizing the filter callback for better performanceThe filter callback in the Collection component could be memoized to prevent unnecessary re-renders.
+ const filterTransactions = useCallback( + (t) => !isPreviewId(t.id) || !t.is_child, + [] + ); <Collection items={section.transactions.filter( - t => !isPreviewId(t.id) || !t.is_child, + filterTransactions )} addIdAndValue >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (17)
packages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-individual-account-page-and-checks-that-filtering-is-working-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-individual-account-page-and-checks-that-filtering-is-working-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-individual-account-page-and-checks-that-filtering-is-working-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-individual-account-page-and-checks-that-filtering-is-working-7-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-individual-account-page-and-checks-that-filtering-is-working-8-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-individual-account-page-and-checks-that-filtering-is-working-9-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--c18ad-l-redirects-to-the-category-transactions-page-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--e995e-l-redirects-to-the-category-transactions-page-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--ff568-l-redirects-to-the-category-transactions-page-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--11290-l-redirects-to-the-category-transactions-page-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--57d88-l-redirects-to-the-category-transactions-page-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--5d90c-l-redirects-to-the-category-transactions-page-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-7-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-8-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-9-chromium-linux.pngis excluded by!**/*.pngupcoming-release-notes/3761.mdis excluded by!**/*.mdyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
packages/desktop-client/package.json(2 hunks)packages/desktop-client/src/components/mobile/transactions/ListBox.jsx(0 hunks)packages/desktop-client/src/components/mobile/transactions/ListBoxSection.jsx(0 hunks)packages/desktop-client/src/components/mobile/transactions/Option.jsx(0 hunks)packages/desktop-client/src/components/mobile/transactions/Transaction.jsx(0 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx(4 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionListItem.tsx(1 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionListWithBalances.jsx(0 hunks)packages/loot-core/src/shared/months.ts(1 hunks)
💤 Files with no reviewable changes (5)
- packages/desktop-client/src/components/mobile/transactions/ListBox.jsx
- packages/desktop-client/src/components/mobile/transactions/ListBoxSection.jsx
- packages/desktop-client/src/components/mobile/transactions/Option.jsx
- packages/desktop-client/src/components/mobile/transactions/Transaction.jsx
- packages/desktop-client/src/components/mobile/transactions/TransactionListWithBalances.jsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/desktop-client/package.json
- packages/desktop-client/src/components/mobile/transactions/TransactionListItem.tsx
- packages/loot-core/src/shared/months.ts
🔇 Additional comments (2)
packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (2)
8-8: LGTM: Clean transition to react-aria-components
The import changes align well with the PR's objective of converting to react-aria-components, providing better accessibility and maintainability.
Also applies to: 40-40
85-93: Reiterating previous review comment about scroll state tracking
The previous review comment about updating prevScrolledToBottom.current is still valid and should be addressed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (1)
Line range hint
367-371: Address TODO comment regarding schedule functionalityThere's a pending TODO about updating the undo notification message with messageActions when schedule becomes available in mobile.
Would you like me to help create a GitHub issue to track this enhancement for when schedule functionality becomes available in mobile?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx(4 hunks)
🔇 Additional comments (3)
packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (3)
8-8: LGTM: Import changes align with component modernization
The new imports from react-aria-components and the addition of TransactionListItem align well with the PR's objective of modernizing the component structure.
Also applies to: 37-37, 40-40
Line range hint 44-67: LGTM: Clean component structure with improved prop handling
The component signature is now more explicit, and the section data structure is well-organized for transaction grouping.
144-158: LGTM: Improved list rendering with Collection component
The use of Collection component with proper filtering and item handling improves both accessibility and performance. The filtering of preview transactions is handled correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx (1)
320-320: LGTM: Simplified loading state props.The change to use only the main
isLoadingprop simplifies the component's loading state management and improves maintainability.Consider adding TypeScript prop validation to ensure type safety:
+type TransactionListWithBalancesProps = { + isLoading: boolean; + transactions: TransactionEntity[]; + balance: Query; + balanceCleared?: Query; + balanceUncleared?: Query; + onLoadMore?: () => void; + searchPlaceholder: string; + onSearch: (query: string) => void; + onOpenTransaction: (transaction: TransactionEntity) => void; + onRefresh?: () => void; +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx(2 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx (3)
Learnt from: jfdoming
PR: actualbudget/actual#3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In the `AllTransactions` component of `Account.tsx`, the `useLayoutEffect` hook is appropriately used to dispatch an action that closes splits for parent transactions when `prependTransactions` changes, ensuring this occurs synchronously before the component is painted.
Learnt from: jfdoming
PR: actualbudget/actual#3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-11-10T16:45:31.225Z
Learning: In the `AllTransactions` component of `Account.tsx`, the `useLayoutEffect` hook is appropriately used to dispatch an action that closes splits for parent transactions when `prependTransactions` changes, ensuring this occurs synchronously before the component is painted.
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx:261-277
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In React components (TypeScript), avoid using hooks like `useCallback` inside callbacks or nested functions. Hooks must be called at the top level of functional components.
🔇 Additional comments (1)
packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx (1)
242-242: LGTM: Simplified preview transactions loading state.
The change to only destructure data from usePreviewTransactions hook aligns with the cleanup objectives and simplifies the loading state management.
Let's verify that the removal of isLoading from preview transactions doesn't affect other components:
✅ Verification successful
Removal of isLoading from usePreviewTransactions is safe
The verification confirms that no other components in the codebase are using the isLoading property from usePreviewTransactions, making this change safe and non-breaking.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining usage of isLoading from usePreviewTransactions
rg -l "usePreviewTransactions.*isLoading" || echo "No components depend on isLoading from usePreviewTransactions"
Length of output: 176
Removed a bunch of files and replaced it with react-aria-component equivalent components
Did some minor adjustments to the horizontal paddings of transaction list items hence the VRT screenshot changes