Mobile VRTs #3506
Mobile VRTs #3506
Conversation
WalkthroughThe pull request introduces end-to-end tests for a mobile budgeting application using Playwright, focusing on "Envelope" and "Tracking" budget types. It enhances the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
✅ 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 No assets were 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 No files were changed 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
mobile-vrts-1
branch
from
760988f to
d4297fc
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (18)
packages/desktop-client/e2e/page-models/settings-page.js (2)
10-18: LGTM! Consider adding error handling and result verification.The
useBudgetTypemethod is well-structured and follows good practices. It effectively enables the experimental feature and switches to the specified budget type.Consider the following improvements:
- Add error handling in case the button is not found or the click action fails.
- Implement a verification step to confirm that the budget type was successfully changed.
- Consider checking if the 'Budget mode toggle' feature is already enabled before enabling it again.
Here's a suggested implementation:
async useBudgetType(budgetType) { try { if (!(await this.isExperimentalFeatureEnabled('Budget mode toggle'))) { await this.enableExperimentalFeature('Budget mode toggle'); } const switchBudgetTypeButton = this.page.getByRole('button', { name: `Switch to ${budgetType} budgeting`, }); await switchBudgetTypeButton.click(); // Verify the change (implement according to your UI) const newBudgetTypeIndicator = this.page.getByTestId('current-budget-type'); await expect(newBudgetTypeIndicator).toHaveText(budgetType); } catch (error) { console.error(`Failed to switch to ${budgetType} budgeting:`, error); throw error; } } async isExperimentalFeatureEnabled(featureName) { // Implement this method to check if a feature is already enabled }This implementation adds error handling, verifies the result, and checks if the feature is already enabled before enabling it again.
20-32: LGTM! Consider enhancing error handling and feature toggle logic.The modifications to
enableExperimentalFeatureimprove its robustness by using roles and test IDs for element selection. This is a good practice and makes the code more reliable.Consider the following improvements:
- Add error handling in case any elements are not found or click actions fail.
- Implement a verification step to confirm that the feature was successfully enabled.
- Check the current state of the checkbox before clicking to ensure we're enabling (not disabling) the feature.
Here's a suggested implementation:
async enableExperimentalFeature(featureName) { try { const advancedSettingsButton = this.page.getByTestId('advanced-settings'); await advancedSettingsButton.click(); const experimentalSettingsButton = this.page.getByTestId('experimental-settings'); await experimentalSettingsButton.click(); const featureCheckbox = this.page.getByRole('checkbox', { name: featureName }); // Check if the feature is already enabled const isChecked = await featureCheckbox.isChecked(); if (!isChecked) { await featureCheckbox.click(); } // Verify the change await expect(featureCheckbox).toBeChecked(); } catch (error) { console.error(`Failed to enable experimental feature ${featureName}:`, error); throw error; } }This implementation adds error handling, verifies the result, and only toggles the checkbox if the feature is not already enabled.
packages/desktop-client/e2e/settings.mobile.test.js (2)
6-22: LGTM: Test suite setup is comprehensive.The setup for the mobile settings test suite is well-structured. Setting a specific viewport size ensures consistency across tests, and creating a test file before each test is a good practice for isolation.
Consider extracting the viewport size into constants at the top of the file for easier maintenance:
const MOBILE_VIEWPORT = { width: 350, height: 600 }; // Then in the beforeEach hook: await page.setViewportSize(MOBILE_VIEWPORT);
28-42: LGTM: Comprehensive test case with room for improvement.The test case effectively covers navigation, visual regression, and export functionality. The use of async/await and the flexible regex pattern for filename verification are good practices.
Consider the following improvements:
- Add a step to clean up the downloaded file after the test to prevent accumulation of test artifacts.
- Split the test into smaller, more focused test cases. For example:
- One test for navigation and visual regression
- Another test for the export functionality
This would improve test readability and maintainability. Here's a suggested refactor:
test('navigates to settings page and matches theme screenshots', async () => { const settingsPage = await navigation.goToSettingsPage(); await expect(page).toMatchThemeScreenshots(); }); test('exports data with correct filename format', async () => { const settingsPage = await navigation.goToSettingsPage(); const downloadPromise = page.waitForEvent('download'); await settingsPage.exportData(); const download = await downloadPromise; expect(await download.suggestedFilename()).toMatch(/^\d{4}-\d{2}-\d{2}-.*.zip$/); // Clean up downloaded file await download.delete(); });This refactoring separates concerns and makes each test responsible for a single aspect of functionality.
packages/desktop-client/e2e/accounts.mobile.test.js (3)
6-26: LGTM: Test setup is comprehensive, with a suggestion for improvement.The test setup is well-structured, properly initializing the test environment and ensuring cleanup after each test. The mobile viewport size is appropriately set.
Consider parameterizing the tests to run with multiple screen sizes to ensure responsiveness across different mobile devices. This could be achieved using Playwright's test.describe.configure() or by creating a custom test fixture.
Example:
const viewports = [ { width: 350, height: 600 }, { width: 375, height: 667 }, { width: 414, height: 736 } ]; for (const viewport of viewports) { test.describe(`Mobile Accounts (${viewport.width}x${viewport.height})`, () => { test.use({ viewport }); // ... existing tests }); }
28-36: LGTM: Good test coverage with room for improvement.The test case effectively checks the basic functionality of the accounts page, including account name, balance, and visual appearance.
Consider the following improvements:
- Use data-testid attributes for more robust element selection instead of relying on text content.
- Avoid hard-coding expected values. Instead, consider using a test data fixture or a mock API response to set and verify account details.
Example:
test('opens the accounts page and asserts on balances', async ({ testData }) => { const accountsPage = await navigation.goToAccountsPage(); const account = await accountsPage.getNthAccount(1); await expect(account.nameElement).toHaveText(testData.accounts[0].name); await expect(account.balanceElement).toHaveText(testData.accounts[0].balance); await expect(page).toMatchThemeScreenshots(); });
38-56: LGTM: Comprehensive test coverage with suggestions for enhancement.The test case effectively covers the functionality of the individual account page, including filtering and search features. The multiple assertions and visual comparisons at different stages provide good coverage.
Consider the following improvements:
- Use data-testid attributes for more robust element selection.
- Parameterize the search tests to cover more scenarios easily.
- Use a test data fixture for expected values instead of hard-coding them.
Example:
const searchScenarios = [ { searchTerm: 'nothing should be found', expectedCount: 0, messageVisible: true }, { searchTerm: 'Kroger', expectedCount: { greaterThan: 0 }, messageVisible: false } ]; for (const scenario of searchScenarios) { test(`search functionality: ${scenario.searchTerm}`, async ({ testData }) => { // ... setup code ... await accountPage.searchByText(scenario.searchTerm); if (typeof scenario.expectedCount === 'number') { await expect(accountPage.transactions).toHaveCount(scenario.expectedCount); } else if (scenario.expectedCount.greaterThan !== undefined) { await expect(await accountPage.getTransactionCount()).toBeGreaterThan(scenario.expectedCount.greaterThan); } await expect(accountPage.noTransactionsMessage).toBeVisible({ visible: scenario.messageVisible }); await expect(page).toMatchThemeScreenshots(); }); }This approach makes it easier to add more search scenarios and reduces code duplication.
packages/desktop-client/e2e/transactions.mobile.test.js (2)
Line range hint
1-93: Consider expanding test coverage for mobile functionality.While the remaining tests for transaction creation are valuable, the removal of several other tests has significantly reduced the overall coverage of mobile functionality. This change may leave important aspects of the mobile experience untested, potentially increasing the risk of undetected issues.
Consider the following suggestions to improve test coverage:
- Reinstate tests for critical mobile views (e.g., budget page, accounts page).
- Add tests for mobile-specific UI elements and interactions.
- Ensure that mobile navigation is thoroughly tested.
- Include tests for responsive design and different screen sizes.
- If the removed tests are no longer relevant, document the reason for their removal and consider replacing them with more appropriate tests for the current mobile functionality.
To help prioritize which tests to add or reinstate, I can assist in analyzing the most critical user flows for the mobile experience. Would you like me to create a GitHub issue to track the task of expanding mobile test coverage?
Line range hint
27-93: Consider refactoring transaction creation steps.The two remaining tests for transaction creation share similar steps. To improve maintainability and reduce duplication, consider refactoring the common transaction creation steps into a shared helper function or method in the
TransactionEntryPagepage object.For example:
// In TransactionEntryPage class async fillTransactionDetails(amount, payee, category, account = null) { await this.amountField.fill(amount); await this.header.click(); // Cancel active edit await this.fillField(this.page.getByTestId('payee-field'), payee); await this.fillField(this.page.getByTestId('category-field'), category); if (account) { await this.fillField(this.page.getByTestId('account-field'), account); } } // In test await transactionEntryPage.fillTransactionDetails('12.34', 'Kroger', 'Clothing', 'Ally Savings');This refactoring would make the tests more concise and easier to maintain.
packages/desktop-client/src/components/modals/EnvelopeBudgetMenuModal.tsx (1)
Line range hint
30-117: Consider the following improvements for better maintainability and separation of concerns:
Standardize styling approach:
The component uses a mix of inline styles and imported styles. Consider moving all styles to a separate stylesheet or using a consistent styling approach throughout the component for better maintainability.Simplify state management:
TheuseEffecthook setsamountFocusedtotrueon mount. If this is the desired initial state, you could simply initialize the state totrue:const [amountFocused, setAmountFocused] = useState(true);This would eliminate the need for the
useEffecthook.Move amount conversion logic:
The_onUpdateBudgetfunction converts the amount to an integer before calling theonUpdateBudgetprop. Consider moving this conversion to the parent component to maintain a clearer separation of concerns:const _onUpdateBudget = (amount: number) => { onUpdateBudget?.(amount); };Then handle the conversion in the parent component where
onUpdateBudgetis defined.packages/desktop-client/src/components/modals/TrackingBudgetMenuModal.tsx (4)
102-102: Approve the addition of data-testid attributeThe addition of the
data-testidattribute is a good practice for testing purposes, especially for visual regression tests (VRTs) mentioned in the PR title. This will make it easier to select and verify the budget amount input in automated tests.Consider using a more specific test ID to avoid potential conflicts in larger test suites. For example:
-data-testid="budget-amount" +data-testid="tracking-budget-modal-amount-input"This more descriptive ID provides context about where this element is located (in the tracking budget modal) and what it represents (the amount input).
Line range hint
46-48: Consider adding error handling for budget updatesThe
_onUpdateBudgetfunction currently doesn't have any error handling. Consider wrapping theonUpdateBudgetcall in a try-catch block to handle potential errors gracefully.Here's a suggested implementation:
const _onUpdateBudget = (amount: number) => { try { onUpdateBudget?.(amountToInteger(amount)); } catch (error) { console.error('Failed to update budget:', error); // Consider showing an error message to the user } };
Line range hint
65-67: Enhance accessibility for the modalTo improve accessibility, consider adding
aria-labelledbyto the modal andidto the title. This will explicitly associate the title with the modal for screen readers.Here's a suggested implementation:
-<Modal name="tracking-budget-menu"> +<Modal name="tracking-budget-menu" aria-labelledby="tracking-budget-modal-title"> {({ state: { close } }) => ( <> <ModalHeader - title={<ModalTitle title={category.name} shrinkOnOverflow />} + title={<ModalTitle id="tracking-budget-modal-title" title={category.name} shrinkOnOverflow />} rightContent={<ModalCloseButton onPress={close} />} />
Line range hint
50-52: Optimize useEffect for focus managementThe current useEffect hook sets the amount input to focused on every render. Consider adding a dependency array to ensure it only runs once when the component mounts.
Here's a suggested implementation:
useEffect(() => { setAmountFocused(true); -}); +}, []);packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (1)
111-111: Approved: Consistent capitalization in aria-labelThe update of the
aria-labelfrom "transaction list" to "Transaction list" improves consistency in the UI text. This change, while minor, contributes to a more polished user interface.To ensure consistency, it would be beneficial to review other
aria-labelattributes throughout the application and apply similar capitalization rules where appropriate.packages/desktop-client/e2e/page-models/mobile-budget-page.js (1)
27-31: Use consistent element scoping intoggleVisibleColumns()In the
toggleVisibleColumns()method, you retrieve the 'Budgeted' button usingthis.budgetTableHeader, but the 'Spent' button is retrieved usingthis.page. For consistency and proper element scoping, consider retrieving the 'Spent' button fromthis.budgetTableHeaderas well.Suggested change:
- const spentHeaderButton = this.page.getByRole('button', { name: 'Spent' }); + const spentHeaderButton = this.budgetTableHeader.getByRole('button', { name: 'Spent' });packages/desktop-client/e2e/budget.mobile.test.js (1)
241-241: Typo in test description: "Overspensing" should be "Overspending"There's a typographical error in the test description at line 241. Please correct "Overspensing" to "Overspending".
packages/desktop-client/src/components/budget/BalanceWithCarryover.tsx (1)
114-120: Nitpick: Simplify Logic ingetDifferenceToGoalThe
getDifferenceToGoalfunction could be simplified for better readability by assigning the current value to a variable before performing the subtraction.Consider refactoring as follows:
const getDifferenceToGoal = useCallback( (balanceValue: number) => { - return longGoalValue === 1 - ? balanceValue - goalValue - : budgetedValue - goalValue; + const currentValue = longGoalValue === 1 ? balanceValue : budgetedValue; + return currentValue - goalValue; }, [balanceValue, budgetedValue, goalValue, longGoalValue], );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
🔇 Files ignored due to path filters (103)
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-4-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-5-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-6-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/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-the-accounts-page-and-asserts-on-balances-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-the-accounts-page-and-asserts-on-balances-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-the-accounts-page-and-asserts-on-balances-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--14404-in-the-page-header-opens-the-month-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--321fd-ed-amount-opens-the-budget-summary-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--4bb70-ed-amount-opens-the-budget-summary-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--94a85-ed-amount-opens-the-budget-summary-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--bed18-in-the-page-header-opens-the-month-menu-modal-1-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--ceb3a-in-the-page-header-opens-the-month-menu-modal-2-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-Envelope-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-loads-the-budget-page-with-budgeted-amounts-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-loads-the-budget-page-with-budgeted-amounts-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-loads-the-budget-page-with-budgeted-amounts-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-updates-the-budgeted-amount-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-updates-the-budgeted-amount-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-updates-the-budgeted-amount-3-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--42062-in-the-page-header-opens-the-month-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--49fb6-in-the-page-header-opens-the-month-menu-modal-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/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--730ef-ng-amount-opens-the-budget-summary-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--b1562-in-the-page-header-opens-the-month-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--c3fb8-ng-amount-opens-the-budget-summary-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--d9705-ng-amount-opens-the-budget-summary-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-loads-the-budget-page-with-budgeted-amounts-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-loads-the-budget-page-with-budgeted-amounts-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-loads-the-budget-page-with-budgeted-amounts-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-updates-the-budgeted-amount-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-updates-the-budgeted-amount-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-updates-the-budgeted-amount-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.test.js-snapshots/Budget-renders-the-summary-information-available-funds-overspent-budgeted-and-for-next-month-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.test.js-snapshots/Budget-renders-the-summary-information-available-funds-overspent-budgeted-and-for-next-month-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-checks-that-settings-page-can-be-opened-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-checks-that-settings-page-can-be-opened-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-creates-a-transaction-from-accounts-id-page-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-creates-a-transaction-from-accounts-id-page-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-creates-a-transaction-from-accounts-id-page-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-creates-a-transaction-via-footer-button-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-opens-the-accounts-page-and-asserts-on-balances-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-opens-the-accounts-page-and-asserts-on-balances-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-opens-the-accounts-page-and-asserts-on-balances-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/onboarding.test.js-snapshots/Onboarding-checks-the-page-visuals-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/rules.test.js-snapshots/Rules-checks-the-page-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/rules.test.js-snapshots/Rules-creates-a-split-transaction-rule-and-makes-sure-it-is-applied-when-creating-a-transaction-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/schedules.test.js-snapshots/Schedules-creates-a-new-schedule-posts-the-transaction-and-later-completes-it-7-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/schedules.test.js-snapshots/Schedules-creates-a-new-schedule-posts-the-transaction-and-later-completes-it-8-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-6-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-6-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!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-6-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-filters-transactions-by-date-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-filters-transactions-by-date-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-filters-transactions-by-date-6-chromium-linux.pngis excluded by!**/*.pngupcoming-release-notes/3492.mdis excluded by!**/*.mdyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
- packages/desktop-client/e2e/accounts.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/budget.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/page-models/mobile-account-page.js (1 hunks)
- packages/desktop-client/e2e/page-models/mobile-budget-page.js (1 hunks)
- packages/desktop-client/e2e/page-models/settings-page.js (1 hunks)
- packages/desktop-client/e2e/settings.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/transactions.mobile.test.js (1 hunks)
- packages/desktop-client/src/components/budget/BalanceWithCarryover.tsx (4 hunks)
- packages/desktop-client/src/components/budget/tracking/TrackingBudgetComponents.tsx (1 hunks)
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx (8 hunks)
- packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (2 hunks)
- packages/desktop-client/src/components/modals/EnvelopeBalanceMenuModal.tsx (1 hunks)
- packages/desktop-client/src/components/modals/EnvelopeBudgetMenuModal.tsx (1 hunks)
- packages/desktop-client/src/components/modals/TrackingBalanceMenuModal.tsx (1 hunks)
- packages/desktop-client/src/components/modals/TrackingBudgetMenuModal.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/desktop-client/e2e/page-models/mobile-account-page.js
🧰 Additional context used
📓 Learnings (1)
packages/desktop-client/src/components/budget/BalanceWithCarryover.tsx (1)
Learnt from: joel-jeremy PR: actualbudget/actual#3492 File: packages/desktop-client/src/components/budget/BalanceWithCarryover.tsx:123-136 Timestamp: 2024-09-25T21:00:54.979Z Learning: glamor's `css()` function returns a `StyleAttribute`, not a string.
🔇 Additional comments not posted (23)
packages/desktop-client/e2e/settings.mobile.test.js (2)
1-5: LGTM: Imports and setup look good.The necessary modules are imported, and the use of custom page models (ConfigurationPage and MobileNavigation) is a good practice for maintaining clean and reusable test code.
24-26: Verify if manual page closure is necessary.Closing the page after each test is generally a good practice for cleaning up resources. However, Playwright might automatically handle this between tests.
Please confirm if this manual teardown is necessary by checking the Playwright documentation or running tests with and without this hook to see if there's any difference in resource usage or test isolation.
packages/desktop-client/e2e/accounts.mobile.test.js (2)
1-5: LGTM: Imports are appropriate and follow good practices.The imports are well-structured, utilizing Playwright's test framework and implementing page object models for better maintainability.
1-57: Overall: Well-structured mobile tests with room for enhancement.This new file introduces comprehensive mobile account tests using Playwright, which is a positive addition to the project. The tests cover important functionality of both the accounts page and individual account pages, including balance checking, filtering, and search features.
Positive aspects:
- Use of page object models for better maintainability.
- Comprehensive test setup and teardown.
- Good use of visual snapshot comparisons.
- Coverage of different scenarios in search functionality.
Main points for improvement:
- Parameterize tests to cover multiple screen sizes for better responsiveness testing.
- Use data-testid attributes for more robust element selection.
- Implement test data fixtures to avoid hard-coding expected values.
- Further parameterize tests, especially for search scenarios, to make adding new test cases easier.
These enhancements will make the tests more robust, easier to maintain, and more comprehensive in their coverage of mobile functionality.
packages/desktop-client/e2e/transactions.mobile.test.js (1)
6-6: LGTM: More specific test suite description.The change from "Mobile" to "Mobile Transactions" provides a more accurate description of the test suite's focus. This improvement in specificity is beneficial for understanding the scope of the tests at a glance.
packages/desktop-client/src/components/modals/TrackingBalanceMenuModal.tsx (2)
67-67: Clarification needed: Relation to Mobile VRTsWhile the prop name change from
disabledtoisDisabledis a good improvement, it's not immediately clear how this change relates to the PR objective of "Mobile VRTs" (Visual Regression Tests). Could you please provide more context on how this change fits into the larger scope of mobile visual regression testing?To help understand the context, let's check for any mobile-specific code or VRT-related comments in this file:
#!/bin/bash # Search for mobile-related code or comments rg --type typescript --type tsx -i '(mobile|vrt|visual regression test)' packages/desktop-client/src/components/modals/TrackingBalanceMenuModal.tsx
67-67: Prop name update:disabledtoisDisabledThe change from
disabledtoisDisabledfor theBalanceWithCarryovercomponent prop is a good improvement. This update aligns with React's naming convention for boolean props, which typically use the "is" prefix for clarity.To ensure consistency across the codebase, let's verify if this change affects other components or if there are any remaining instances of the
disabledprop forBalanceWithCarryover:✅ Verification successful
Re-verifying prop name updates in
BalanceWithCarryoverTo ensure that all instances of the
disabledprop have been updated toisDisabledin theBalanceWithCarryovercomponent, please run the following scripts:
Prop name update verified: all instances updated to
isDisabledAll instances of the
disabledprop in theBalanceWithCarryovercomponent have been successfully updated toisDisabled. No remainingdisabledusages were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining usage of 'disabled' prop in BalanceWithCarryover rg --type typescript --type tsx 'BalanceWithCarryover.*disabled\s*[=:]' # Search for the updated 'isDisabled' prop usage rg --type typescript --type tsx 'BalanceWithCarryover.*isDisabled\s*[=:]'Length of output: 374
Script:
#!/bin/bash # Search for any remaining usage of 'disabled' prop in BalanceWithCarryover rg --glob '*.ts*' 'BalanceWithCarryover.*disabled\s*[=:]' # Search for the updated 'isDisabled' prop usage rg --glob '*.ts*' 'BalanceWithCarryover.*isDisabled\s*[=:]'Length of output: 121
packages/desktop-client/src/components/modals/EnvelopeBalanceMenuModal.tsx (1)
69-69: Approve prop name change and suggest verificationThe change from
disabledtoisDisabledimproves code clarity and follows React's recommended naming convention for boolean props. This is a positive change that enhances code readability.To ensure consistency across the codebase, please verify:
- The
BalanceWithCarryovercomponent definition has been updated to useisDisabledinstead ofdisabled.- All other usages of
BalanceWithCarryoverin the codebase have been updated similarly.Run the following script to check for any remaining instances of
disabledprop inBalanceWithCarryover:This script will help identify any instances that might need updating for consistency.
packages/desktop-client/src/components/modals/EnvelopeBudgetMenuModal.tsx (1)
102-102: LGTM: Addition of data-testid for testing purposesThe addition of the
data-testid="budget-amount"prop to theFocusableAmountInputcomponent is a good practice for enabling easier and more reliable automated testing. This change allows test scripts to uniquely identify this input element without relying on more brittle selectors.packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (1)
94-94: Excellent addition of aria-label for improved accessibilityAdding the
aria-label="Loading..."to the View component during the loading state is a great improvement for accessibility. This change ensures that screen reader users are informed about the current state of the component, enhancing the overall user experience.packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx (9)
282-287: Improved testability for BudgetCell componentThe
data-testidattribute has been added to theCellValuecomponent withinBudgetCell. This change enhances the testability of the component by providing a unique identifier for each budgeted cell.
303-303: Added data-testid to budget cell buttonA
data-testidattribute has been added to the button within theBudgetCellcomponent. This improves the ability to target this specific button in automated tests.
604-608: Enhanced testability for spent amount in ExpenseCategoryThe
data-testidattribute has been added to theCellValuecomponent for the spent amount in theExpenseCategorycomponent. This change improves the ability to target and test this specific element.
616-616: Added data-testid to spent amount buttonA
data-testidattribute has been added to the button displaying the spent amount in theExpenseCategorycomponent. This enhancement allows for more precise targeting in automated tests.
647-647: Improved testability for BalanceWithCarryover componentThe
data-testidattribute has been added to theBalanceWithCarryovercomponent within theExpenseCategory. This change enhances the ability to target and test this specific element.
680-680: Added data-testid to balance buttonA
data-testidattribute has been added to the button displaying the balance in theExpenseCategorycomponent. This improvement allows for more precise targeting in automated tests.
1702-1702: Enhanced testability for BudgetTableHeaderA
data-testidattribute has been added to theBudgetTableHeadercomponent. This change improves the ability to target and test this specific component in automated tests.
1924-1924: Improved accessibility for MonthSelector buttons
aria-labelattributes have been added to the previous and next month buttons in theMonthSelectorcomponent. This enhancement improves accessibility by providing clear labels for screen readers.Also applies to: 1953-1953
1946-1946: Added data-testid to month buttonA
data-testidattribute has been added to the month button in theMonthSelectorcomponent. This change enhances the testability of this specific element.packages/desktop-client/e2e/budget.mobile.test.js (1)
1-253: Overall, comprehensive and well-structured testsThe end-to-end tests are thorough and cover a wide range of functionalities in the mobile budgeting application. The use of Playwright is appropriate, and the test cases are clear and readable.
packages/desktop-client/src/components/budget/BalanceWithCarryover.tsx (3)
Line range hint
103-109: Verify: Confirm Correctness ofgetBalanceAmountStyleParametersEnsure that the parameters passed to
getBalanceAmountStyleare accurate, especially considering the conditions whenlongGoalValueis1. Verify that the function behaves correctly across different scenarios oflongGoalValue,goalValue, andbudgetedValue.
201-207: Verify: EnsurechildrenProp Matches New SignatureThe
childrenprop now accepts a functionChildrenWithClassNamewhich expects aclassNameprop. Verify that any components passingchildrentoBalanceWithCarryoverare updated to handle the new signature and are correctly applying theclassName.Run the following script to locate usages of
BalanceWithCarryoverwithchildren:✅ Verification successful
No usages of
BalanceWithCarryoverwith thechildrenprop were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find components using `BalanceWithCarryover` with a `children` prop. # Test: Search for `BalanceWithCarryover` components with `children` prop. # Expect: `children` functions accept and apply `className`. rg --type typescript '<BalanceWithCarryover[^>]*children=' -A 10Length of output: 101
Script:
#!/bin/bash # Description: Find components using `BalanceWithCarryover` with a `children` prop. # Test: Search for `BalanceWithCarryover` components with `children` prop. # Expect: `children` functions accept and apply `className`. rg '<BalanceWithCarryover[^>]*children=' -g '*.tsx' -g '*.ts' -A 10Length of output: 69
Script:
#!/bin/bash # Description: Find components using `BalanceWithCarryover` with a `children` prop that expects a `className`. # Using ast-grep to match the `children` prop with a function parameter that includes `className`. ast-grep --lang typescript --pattern 'BalanceWithCarryover($(props).children(className: $_))'Length of output: 95
82-82: Verify: Update All References ofdisabledtoisDisabledThe prop
disabledhas been renamed toisDisabledinBalanceWithCarryoverProps. Please ensure that all references todisabledwithin this component and any components that consumeBalanceWithCarryoverare updated accordingly.Run the following script to check for any remaining instances of
disabled:
| @@ -398,7 +398,7 @@ export const CategoryMonth = memo(function CategoryMonth({ | |||
| onClick={() => !category.is_income && setBalanceMenuOpen(true)} | |||
| > | |||
| <BalanceWithCarryover | |||
| disabled={category.is_income} | |||
| isDisabled={category.is_income} | |||
There was a problem hiding this comment.
💡 Codebase verification
Missing isDisabled Prop in Several BalanceWithCarryover Usages
The isDisabled prop is missing in the following usages of the BalanceWithCarryover component:
packages/desktop-client/src/components/modals/EnvelopeBalanceMenuModal.tsxpackages/desktop-client/src/components/modals/TrackingBalanceMenuModal.tsxpackages/desktop-client/src/components/budget/tracking/TrackingBudgetComponents.tsxpackages/desktop-client/src/components/budget/envelope/EnvelopeBudgetComponents.tsx
Please update these instances to include the isDisabled prop to ensure consistent behavior and adherence to the updated prop naming convention.
🔗 Analysis chain
Approve prop name change and suggest verification
The change from disabled to isDisabled improves adherence to React naming conventions for boolean props. This is a positive change that enhances code readability and maintainability.
To ensure consistency across the codebase, please verify the following:
- Update the
BalanceWithCarryovercomponent definition to useisDisabledinstead ofdisabled. - Check for other usages of
BalanceWithCarryoverand update them accordingly.
Run the following script to identify potential areas that need updating:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for BalanceWithCarryover component definition and usages
echo "BalanceWithCarryover component definition:"
rg --type typescript "export (const|function) BalanceWithCarryover" -A 10
echo "\nBalanceWithCarryover component usages:"
rg --type typescript "<BalanceWithCarryover" -A 5
Length of output: 383
Script:
#!/bin/bash
# Search for BalanceWithCarryover component definition and usages
echo "BalanceWithCarryover component definition:"
rg "export (const|function) BalanceWithCarryover" --glob "*.ts" --glob "*.tsx" -A 10
echo "\nBalanceWithCarryover component usages:"
rg "<BalanceWithCarryover" --glob "*.ts" --glob "*.tsx" -A 5
Length of output: 4554
Script:
#!/bin/bash
# Search for BalanceWithCarryover component usages without the isDisabled prop
echo "BalanceWithCarryover usages missing isDisabled prop:"
rg '<BalanceWithCarryover(?!(.|\n)*isDisabled)' --glob '*.ts' --glob '*.tsx' -C 2
Length of output: 465
Script:
#!/bin/bash
# Search for BalanceWithCarryover component usages without the isDisabled prop
echo "BalanceWithCarryover usages missing isDisabled prop:"
rg "<BalanceWithCarryover" --glob "*.ts" --glob "*.tsx" -C 2 | grep -v "isDisabled"
Length of output: 2255
|
|
||
| this.budgetTableHeader = page.getByTestId('budget-table-header'); | ||
| this.budgetTable = page.getByTestId('budget-table'); | ||
| this.navigation = new MobileNavigation(page); |
There was a problem hiding this comment.
Unused property this.navigation
The property this.navigation is instantiated in the constructor but is not used elsewhere in the class. If it's unnecessary, consider removing it to keep the code clean and maintainable.
| if ((await overspentButton.count()) > 0) { | ||
| await overspentButton.click(); | ||
| return; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle cases where no buttons are found in openTrackingBudgetSummaryMenu()
Similarly, if none of the 'Saved', 'Projected Savings', or 'Overspent' buttons are found, the method exits silently. Adding error handling will help identify issues during execution.
Suggested change:
}
+ throw new Error('No Tracking Budget Summary buttons could be located.');Committable suggestion was skipped due to low confidence.
| `balance-${categoryName}-button`, | ||
| ); | ||
| await balanceButton.click(); | ||
| } |
There was a problem hiding this comment.
Check for the existence of balanceButton before clicking
In the openBalanceMenu() method, you directly click on balanceButton without verifying its existence or visibility. This could lead to exceptions if the button is not present. Consider adding a check before attempting to click.
Suggested change:
+ if ((await balanceButton.count()) > 0 && (await balanceButton.isVisible())) {
await balanceButton.click();
+ } else {
+ throw new Error(`Balance button for category ${categoryName} could not be located.`);
+ }Committable suggestion was skipped due to low confidence.
| await overbudgetedButton.click(); | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle cases where no buttons are found in openEnvelopeBudgetSummaryMenu()
If neither the 'To Budget' nor 'Overbudgeted' buttons are found, the method exits without any indication. To improve error handling, consider throwing an error when no buttons are located.
Suggested change:
}
+ throw new Error('No Envelope Budget Summary buttons could be located.');Committable suggestion was skipped due to low confidence.
| test.beforeEach(async ({ browser }) => { | ||
| page = await browser.newPage(); | ||
| navigation = new MobileNavigation(page); | ||
| configurationPage = new ConfigurationPage(page); | ||
|
|
||
| await page.setViewportSize({ | ||
| width: 350, | ||
| height: 600, | ||
| }); | ||
| await page.goto('/'); | ||
| await configurationPage.createTestFile(); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor beforeEach hooks to reduce duplication
The beforeEach hooks in both the 'Mobile Budget [Envelope]' and 'Mobile Budget [Tracking]' test suites are nearly identical. Consider moving the common setup code to a higher scope or creating a shared helper function to reduce code duplication and improve maintainability.
Also applies to: 131-142
| test.afterEach(async () => { | ||
| await page.close(); | ||
| }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor afterEach hooks to reduce duplication
Similarly, the afterEach hooks in both test suites are the same. Abstracting this common teardown code can help avoid redundancy.
Also applies to: 144-147
| test('loads the budget page with budgeted amounts', async () => { | ||
| const budgetPage = await navigation.goToBudgetPage(); | ||
|
|
||
| await expect(budgetPage.categoryNames).toHaveText([ | ||
| 'Food', | ||
| 'Restaurants', | ||
| 'Entertainment', | ||
| 'Clothing', | ||
| 'General', | ||
| 'Gift', | ||
| 'Medical', | ||
| 'Savings', | ||
| 'Cell', | ||
| 'Internet', | ||
| 'Mortgage', | ||
| 'Water', | ||
| 'Power', | ||
| 'Starting Balances', | ||
| 'Misc', | ||
| 'Income', | ||
| ]); | ||
| await expect(page).toMatchThemeScreenshots(); | ||
| }); | ||
|
|
||
| test('checks that clicking the budgeted cell opens the budget menu modal', async () => { | ||
| const budgetPage = await navigation.goToBudgetPage(); | ||
| await expect(budgetPage.budgetTable).toBeVisible(); | ||
|
|
||
| await budgetPage.openBudgetMenu('Food'); | ||
|
|
||
| const budgetMenuModal = page.getByTestId('envelope-budget-menu-modal'); | ||
| await expect(budgetMenuModal).toBeVisible(); | ||
| await expect(page).toMatchThemeScreenshots(); | ||
| }); | ||
|
|
||
| test('checks that clicking spent cell redirects to the category transactions page', async () => { | ||
| const budgetPage = await navigation.goToBudgetPage(); | ||
| await expect(budgetPage.budgetTable).toBeVisible(); | ||
|
|
||
| const accountPage = await budgetPage.openSpentPage('Food'); | ||
|
|
||
| await expect(accountPage.transactionList).toBeVisible(); | ||
| await expect(page).toMatchThemeScreenshots(); | ||
| }); | ||
|
|
||
| test('checks that clicking the balance button opens the balance menu modal', async () => { | ||
| const budgetPage = await navigation.goToBudgetPage(); | ||
| await expect(budgetPage.budgetTable).toBeVisible(); | ||
|
|
||
| await budgetPage.openBalanceMenu('Food'); | ||
|
|
||
| const balanceMenuModal = page.getByTestId('envelope-balance-menu-modal'); | ||
| await expect(balanceMenuModal).toBeVisible(); | ||
| await expect(page).toMatchThemeScreenshots(); | ||
| }); | ||
|
|
||
| test('checks that clicking the month in the page header opens the month menu modal', async () => { | ||
| const budgetPage = await navigation.goToBudgetPage(); | ||
| await expect(budgetPage.budgetTable).toBeVisible(); | ||
|
|
||
| await budgetPage.openMonthMenu(); | ||
|
|
||
| const monthMenuModal = page.getByTestId('envelope-budget-month-menu-modal'); | ||
| await expect(monthMenuModal).toBeVisible(); | ||
| await expect(page).toMatchThemeScreenshots(); | ||
| }); | ||
|
|
||
| test('updates the budgeted amount', async () => { | ||
| const budgetPage = await navigation.goToBudgetPage(); | ||
| await expect(budgetPage.budgetTable).toBeVisible(); | ||
|
|
||
| // Set to 100.00 | ||
| await budgetPage.setBudget('Food', 10000); | ||
|
|
||
| const budgetedButton = await budgetPage.getBudgetedButton('Food'); | ||
|
|
||
| await expect(budgetedButton).toHaveText('100.00'); | ||
|
|
||
| await expect(page).toMatchThemeScreenshots(); | ||
| }); | ||
|
|
||
| test('checks that clicking the To Budget/Overbudgeted amount opens the budget summary menu modal', async () => { | ||
| const budgetPage = await navigation.goToBudgetPage(); | ||
|
|
||
| await budgetPage.openEnvelopeBudgetSummaryMenu(); | ||
|
|
||
| const summaryModal = page.getByTestId('envelope-budget-summary-modal'); | ||
| await expect(summaryModal).toBeVisible(); | ||
| await expect(page).toMatchThemeScreenshots(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Parameterize tests to eliminate duplicated test cases
The test cases in the 'Mobile Budget [Envelope]' and 'Mobile Budget [Tracking]' suites are very similar, differing mainly by the budget type. Consider parameterizing these tests to run for both budget types. This will reduce duplication and simplify future maintenance.
You can achieve this by iterating over the budget types and wrapping the shared tests within a loop. For example:
const budgetTypes = ['envelope', 'tracking'];
budgetTypes.forEach(budgetType => {
test.describe(`Mobile Budget [${budgetType}]`, () => {
// Shared variables
let page;
let navigation;
let configurationPage;
test.beforeEach(async ({ browser }) => {
page = await browser.newPage();
navigation = new MobileNavigation(page);
configurationPage = new ConfigurationPage(page);
await page.setViewportSize({
width: 350,
height: 600,
});
await page.goto('/');
await configurationPage.createTestFile();
if (budgetType === 'tracking') {
// Set budget type to tracking
const settingsPage = await navigation.goToSettingsPage();
await settingsPage.useBudgetType('tracking');
}
});
test.afterEach(async () => {
await page.close();
});
// Shared tests
test('loads the budget page with budgeted amounts', async () => {
const budgetPage = await navigation.goToBudgetPage();
// ... rest of the test
});
// Additional tests...
});
});This approach reduces code duplication and makes it easier to add or modify tests for all budget types.
Also applies to: 148-253
| placement="bottom" | ||
| triggerProps={{ | ||
| delay: 750, | ||
| isDisabled: !isGoalTemplatesEnabled || goalValue == null, |
There was a problem hiding this comment.
Issue: Potential Unintended Tooltip Disabling
In the Tooltip component's triggerProps, the isDisabled condition is:
isDisabled: !isGoalTemplatesEnabled || goalValue == null,Using goalValue == null might incorrectly disable the tooltip when goalValue is 0. To accurately check for null or undefined, consider using goalValue === null || goalValue === undefined.
Apply this diff to fix the condition:
- isDisabled: !isGoalTemplatesEnabled || goalValue == null,
+ isDisabled: !isGoalTemplatesEnabled || goalValue === null || goalValue === undefined,| String( | ||
| css({ | ||
| ...getBalanceAmountStyle(balanceValue), | ||
| overflow: 'hidden', | ||
| textOverflow: 'ellipsis', | ||
| textAlign: 'right', | ||
| ...(!isDisabled && { | ||
| cursor: 'pointer', | ||
| }), | ||
| ':hover': { textDecoration: 'underline' }, | ||
| }), | ||
| ), |
There was a problem hiding this comment.
Issue: Potential Type Inconsistency with css() Function Usage
According to the retrieved learning, Glamor's css() function returns a StyleAttribute, not a string. Wrapping it with String() may result in [object Object] being assigned to the className, which is not the intended effect.
To fix this, you should use the css() function directly without wrapping it in String(), as it returns a class name that can be used directly.
Apply this diff to correct the code:
- (balanceValue: number) =>
- String(
- css({
- ...getBalanceAmountStyle(balanceValue),
- overflow: 'hidden',
- textOverflow: 'ellipsis',
- textAlign: 'right',
- ...(!isDisabled && {
- cursor: 'pointer',
- }),
- ':hover': { textDecoration: 'underline' },
- }),
- ),
+ (balanceValue: number) =>
+ css({
+ ...getBalanceAmountStyle(balanceValue),
+ overflow: 'hidden',
+ textOverflow: 'ellipsis',
+ textAlign: 'right',
+ ...(!isDisabled && {
+ cursor: 'pointer',
+ }),
+ ':hover': { textDecoration: 'underline' },
+ }),📝 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.
| String( | |
| css({ | |
| ...getBalanceAmountStyle(balanceValue), | |
| overflow: 'hidden', | |
| textOverflow: 'ellipsis', | |
| textAlign: 'right', | |
| ...(!isDisabled && { | |
| cursor: 'pointer', | |
| }), | |
| ':hover': { textDecoration: 'underline' }, | |
| }), | |
| ), | |
| (balanceValue: number) => | |
| css({ | |
| ...getBalanceAmountStyle(balanceValue), | |
| overflow: 'hidden', | |
| textOverflow: 'ellipsis', | |
| textAlign: 'right', | |
| ...(!isDisabled && { | |
| cursor: 'pointer', | |
| }), | |
| ':hover': { textDecoration: 'underline' }, | |
| }), |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
packages/desktop-client/e2e/page-models/mobile-budget-page.js (1)
1-158: Overall assessment: Well-implemented with minor improvement suggestions.The
MobileBudgetPageclass is well-structured and implements a comprehensive set of methods for interacting with various elements on a mobile budget page. The code aligns well with the PR objectives of enhancing mobile-related features.Key strengths:
- Clear separation of concerns between methods.
- Good error handling in most methods.
- Effective implementation of visibility and existence checks for UI elements.
Suggested improvements:
- Add recursion limits to
getBudgetedButtonandgetSpentButtonmethods to prevent potential infinite loops.- Implement error handling in
openBalanceMenuandopenMonthMenumethods.These minor enhancements will further improve the robustness and reliability of the code. Great job on implementing a comprehensive set of features for the mobile budget page!
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
🔇 Files ignored due to path filters (1)
upcoming-release-notes/3506.mdis excluded by!**/*.md
📒 Files selected for processing (2)
- packages/desktop-client/e2e/budget.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/page-models/mobile-budget-page.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/desktop-client/e2e/budget.mobile.test.js
🔇 Additional comments not posted (7)
packages/desktop-client/e2e/page-models/mobile-budget-page.js (7)
1-14: LGTM: Import and constructor look good.The import statement and constructor are well-structured. All necessary properties are initialized, and there are no unused properties.
16-34: LGTM: Well-implemented column visibility toggle.The
toggleVisibleColumnsmethod effectively handles different scenarios for toggling column visibility. It includes proper error handling when neither 'Budgeted' nor 'Spent' buttons are found.
57-68: LGTM:openBudgetMenuandsetBudgetmethods are well-implemented.These methods are concise, focused, and correctly implement their intended functionality. They make good use of the
getBudgetedButtonmethod to locate the necessary elements.
89-94: LGTM:openSpentPagemethod is well-implemented.The
openSpentPagemethod is concise, focused, and correctly implements its intended functionality. It makes good use of thegetSpentButtonmethod and properly returns a new instance ofMobileAccountPage.
108-128: LGTM:openEnvelopeBudgetSummaryMenuis well-implemented.The
openEnvelopeBudgetSummaryMenumethod is robust and well-structured. It effectively handles different scenarios, includes proper visibility checks, and implements error handling as suggested in a previous review. Good job on addressing the past feedback!
130-158: LGTM:openTrackingBudgetSummaryMenuis well-implemented.The
openTrackingBudgetSummaryMenumethod is robust and well-structured. It effectively handles different scenarios, includes proper existence checks usingcount(), and implements error handling as suggested in a previous review. Excellent job on addressing the past feedback and creating a comprehensive method!
96-101:⚠️ Potential issueAdd error handling to
openBalanceMenu.As suggested in a previous review, it's important to check for the existence and visibility of the balance button before attempting to click it. This will prevent potential exceptions if the button is not present or not visible.
Consider implementing the following change:
async openBalanceMenu(categoryName) { const balanceButton = this.page.getByTestId(`balance-${categoryName}-button`); if ((await balanceButton.count()) > 0 && (await balanceButton.isVisible())) { await balanceButton.click(); } else { throw new Error(`Balance button for category ${categoryName} could not be located or is not visible.`); } }This modification adds a check for the button's existence and visibility before clicking, improving the method's robustness.
| async openMonthMenu() { | ||
| const monthButton = this.page.getByTestId('page-header-month-button'); | ||
| await monthButton.click(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to openMonthMenu.
To improve robustness, consider adding a check for the existence and visibility of the month button before attempting to click it. This will prevent potential exceptions if the button is not present or not visible.
Here's a suggested improvement:
async openMonthMenu() {
const monthButton = this.page.getByTestId('page-header-month-button');
if ((await monthButton.count()) > 0 && (await monthButton.isVisible())) {
await monthButton.click();
} else {
throw new Error('Month button could not be located or is not visible.');
}
}This modification adds a check for the button's existence and visibility before clicking, improving the method's reliability.
joel-jeremy
force-pushed
the
mobile-vrts-1
branch
from
73d8689 to
41b9393
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (5)
packages/desktop-client/e2e/page-models/settings-page.js (2)
10-18: LGTM! Consider adding error handling and verification.The
useBudgetTypemethod is well-structured and logically sound. It effectively enables the experimental feature and switches to the specified budget type. However, consider the following improvements:
- Add error handling in case the button is not found or the click action fails.
- Implement a verification step to confirm that the budget type was successfully changed.
- Consider checking if the "Budget mode toggle" feature is already enabled before enabling it again.
Here's a suggested implementation with these improvements:
async useBudgetType(budgetType) { await this.enableExperimentalFeature('Budget mode toggle'); const switchBudgetTypeButton = this.page.getByRole('button', { name: `Switch to ${budgetType} budgeting`, }); if (await switchBudgetTypeButton.isVisible()) { await switchBudgetTypeButton.click(); // Verify the change (adjust the selector as needed) const budgetTypeIndicator = this.page.getByText(`Current: ${budgetType} budgeting`); await expect(budgetTypeIndicator).toBeVisible({ timeout: 5000 }); } else { throw new Error(`Button to switch to ${budgetType} budgeting not found`); } }This implementation adds error handling, verifies the change, and uses a timeout for the verification step.
20-32: LGTM! Consider adding error handling and improving toggle logic.The modifications to the
enableExperimentalFeaturemethod improve its robustness by using more specific selectors. However, consider the following enhancements:
- Add error handling in case any of the elements are not found.
- Implement a verification step to confirm that the feature was successfully enabled.
- Check the current state of the checkbox before clicking to ensure the feature is always enabled, not toggled.
Here's a suggested implementation with these improvements:
async enableExperimentalFeature(featureName) { const advancedSettingsButton = this.page.getByTestId('advanced-settings'); await advancedSettingsButton.click(); const experimentalSettingsButton = this.page.getByTestId('experimental-settings'); await experimentalSettingsButton.click(); const featureCheckbox = this.page.getByRole('checkbox', { name: featureName }); try { await featureCheckbox.waitFor({ state: 'visible', timeout: 5000 }); const isChecked = await featureCheckbox.isChecked(); if (!isChecked) { await featureCheckbox.check(); // Verify the change await expect(featureCheckbox).toBeChecked({ timeout: 5000 }); } } catch (error) { throw new Error(`Failed to enable experimental feature "${featureName}": ${error.message}`); } }This implementation adds error handling, verifies the change, and only checks the checkbox if it's not already checked.
packages/desktop-client/e2e/transactions.mobile.test.js (2)
Line range hint
28-61: Comprehensive test for transaction creation via footer buttonThis test thoroughly covers the process of creating a transaction using the footer button, including UI validation, data entry, and final transaction creation. The use of visual regression testing with
toMatchThemeScreenshots()is a good practice for catching unexpected UI changes.Consider adding a comment explaining the purpose of clicking the header after filling the amount field:
await transactionEntryPage.amountField.fill('12.34'); -// Click anywhere to cancel active edit. +// Click the header to dismiss the keyboard and ensure the amount is properly set await transactionEntryPage.header.click();
Line range hint
63-91: Enhance consistency and coverage in the second transaction creation testWhile this test covers an important scenario of creating a transaction from a specific account page, there are opportunities to improve its consistency with the first test and enhance its coverage:
- Add visual regression testing using
toMatchThemeScreenshots()at key points, similar to the first test.- Include an assertion to verify the account field is pre-filled when creating a transaction from the account page.
- Add assertions for the transaction entry page header and final transaction details, matching the thoroughness of the first test.
Consider applying these changes to improve the test:
const transactionEntryPage = await accountPage.clickCreateTransaction(); await expect(transactionEntryPage.header).toHaveText('New Transaction'); +await expect(page).toMatchThemeScreenshots(); + +// Verify the account field is pre-filled +await expect(page.getByTestId('account-field')).toHaveValue('Ally Savings'); await transactionEntryPage.amountField.fill('12.34'); // Click anywhere to cancel active edit. await transactionEntryPage.header.click(); await transactionEntryPage.fillField( page.getByTestId('payee-field'), 'Kroger', ); await transactionEntryPage.fillField( page.getByTestId('category-field'), 'Clothing', ); +await expect(page).toMatchThemeScreenshots(); + await transactionEntryPage.createTransaction(); await expect(accountPage.transactions.nth(0)).toHaveText( 'KrogerClothing-12.34', ); +await expect(page).toMatchThemeScreenshots();These changes will make the test more robust and consistent with the first test.
packages/desktop-client/src/components/common/Modal.tsx (1)
Line range hint
405-450: Consider enhancing accessibility for editable titlesWhile the addition of
aria-labelattributes is a great improvement, we could further enhance accessibility for editable titles. Consider the following suggestions:
- Add an
aria-editable="true"attribute when the title is editable.- Use
aria-live="polite"to announce changes when the title is updated.- Include instructions for screen reader users on how to edit the title.
Here's an example of how you might implement these suggestions:
<TextOneLine aria-label="Modal title" aria-editable={isEditable ? "true" : undefined} aria-live="polite" onClick={onTitleClick} style={{ fontSize: 25, fontWeight: 700, textAlign: 'center', ...(isEditable && styles.underlinedText), ...style, }} > {title} {isEditable && <span className="sr-only">(Click to edit)</span>} </TextOneLine>These changes would provide more context and better usability for screen reader users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (107)
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-4-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-5-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-6-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/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-the-accounts-page-and-asserts-on-balances-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-the-accounts-page-and-asserts-on-balances-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-the-accounts-page-and-asserts-on-balances-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--14404-in-the-page-header-opens-the-month-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--321fd-ed-amount-opens-the-budget-summary-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--4bb70-ed-amount-opens-the-budget-summary-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--94a85-ed-amount-opens-the-budget-summary-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--bed18-in-the-page-header-opens-the-month-menu-modal-1-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--ceb3a-in-the-page-header-opens-the-month-menu-modal-2-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-Envelope-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-loads-the-budget-page-with-budgeted-amounts-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-loads-the-budget-page-with-budgeted-amounts-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-loads-the-budget-page-with-budgeted-amounts-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-updates-the-budgeted-amount-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-updates-the-budgeted-amount-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-updates-the-budgeted-amount-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--0ba04-nt-amount-opens-the-budget-summary-menu-modal-1-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--1ce6d-nt-amount-opens-the-budget-summary-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--42062-in-the-page-header-opens-the-month-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--49fb6-in-the-page-header-opens-the-month-menu-modal-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/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--b1562-in-the-page-header-opens-the-month-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--f224f-nt-amount-opens-the-budget-summary-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-loads-the-budget-page-with-budgeted-amounts-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-loads-the-budget-page-with-budgeted-amounts-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-loads-the-budget-page-with-budgeted-amounts-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-updates-the-budgeted-amount-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-updates-the-budgeted-amount-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-updates-the-budgeted-amount-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.test.js-snapshots/Budget-renders-the-summary-information-available-funds-overspent-budgeted-and-for-next-month-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.test.js-snapshots/Budget-renders-the-summary-information-available-funds-overspent-budgeted-and-for-next-month-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-checks-that-settings-page-can-be-opened-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-checks-that-settings-page-can-be-opened-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-creates-a-transaction-from-accounts-id-page-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-creates-a-transaction-from-accounts-id-page-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-creates-a-transaction-from-accounts-id-page-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-creates-a-transaction-via-footer-button-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-loads-the-budget-page-with-budgeted-amounts-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-loads-the-budget-page-with-budgeted-amounts-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-loads-the-budget-page-with-budgeted-amounts-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-opens-the-accounts-page-and-asserts-on-balances-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-opens-the-accounts-page-and-asserts-on-balances-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-opens-the-accounts-page-and-asserts-on-balances-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/onboarding.test.js-snapshots/Onboarding-checks-the-page-visuals-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/rules.test.js-snapshots/Rules-checks-the-page-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/rules.test.js-snapshots/Rules-creates-a-split-transaction-rule-and-makes-sure-it-is-applied-when-creating-a-transaction-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/schedules.test.js-snapshots/Schedules-creates-a-new-schedule-posts-the-transaction-and-later-completes-it-7-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/schedules.test.js-snapshots/Schedules-creates-a-new-schedule-posts-the-transaction-and-later-completes-it-8-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-6-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-6-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!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-6-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-filters-transactions-by-date-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-filters-transactions-by-date-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-filters-transactions-by-date-6-chromium-linux.pngis excluded by!**/*.pngupcoming-release-notes/3492.mdis excluded by!**/*.mdupcoming-release-notes/3506.mdis excluded by!**/*.mdyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
- packages/desktop-client/e2e/accounts.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/budget.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/page-models/mobile-account-page.js (1 hunks)
- packages/desktop-client/e2e/page-models/mobile-budget-page.js (1 hunks)
- packages/desktop-client/e2e/page-models/settings-page.js (1 hunks)
- packages/desktop-client/e2e/settings.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/transactions.mobile.test.js (1 hunks)
- packages/desktop-client/src/components/budget/BalanceWithCarryover.tsx (4 hunks)
- packages/desktop-client/src/components/budget/tracking/TrackingBudgetComponents.tsx (1 hunks)
- packages/desktop-client/src/components/common/Modal.tsx (2 hunks)
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx (8 hunks)
- packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (2 hunks)
- packages/desktop-client/src/components/modals/EnvelopeBalanceMenuModal.tsx (1 hunks)
- packages/desktop-client/src/components/modals/EnvelopeBudgetMenuModal.tsx (1 hunks)
- packages/desktop-client/src/components/modals/TrackingBalanceMenuModal.tsx (1 hunks)
- packages/desktop-client/src/components/modals/TrackingBudgetMenuModal.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- packages/desktop-client/e2e/accounts.mobile.test.js
- packages/desktop-client/e2e/budget.mobile.test.js
- packages/desktop-client/e2e/page-models/mobile-account-page.js
- packages/desktop-client/e2e/settings.mobile.test.js
- packages/desktop-client/src/components/budget/BalanceWithCarryover.tsx
- packages/desktop-client/src/components/budget/tracking/TrackingBudgetComponents.tsx
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx
- packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx
- packages/desktop-client/src/components/modals/EnvelopeBalanceMenuModal.tsx
- packages/desktop-client/src/components/modals/EnvelopeBudgetMenuModal.tsx
- packages/desktop-client/src/components/modals/TrackingBalanceMenuModal.tsx
- packages/desktop-client/src/components/modals/TrackingBudgetMenuModal.tsx
🔇 Additional comments (12)
packages/desktop-client/e2e/transactions.mobile.test.js (2)
6-6: Improved test suite descriptionThe updated description "Mobile Transactions" is more specific and accurately reflects the focus of the remaining tests. This change enhances clarity and aligns with best practices for test naming.
Line range hint
1-91: Concern: Significant reduction in mobile test coverageThe removal of several tests has substantially reduced the coverage of mobile functionality. Important features that are no longer being tested include:
- Budget page loading and category display
- Accounts page balances
- Individual account page transaction filtering and searching
- Settings page access and data export functionality
These removals could potentially lead to unnoticed regressions in critical mobile features.
To address this concern, please consider one of the following actions:
- Reinstate the removed tests in this file to maintain comprehensive mobile coverage.
- If these tests have been moved elsewhere, ensure they are still being run in a mobile context.
- If the features are no longer relevant or have been significantly changed, update the test suite accordingly while maintaining adequate coverage.
To verify the current state of mobile testing, you can run the following script:
This script will help identify if the removed tests have been relocated or if similar mobile-specific tests exist elsewhere in the codebase.
packages/desktop-client/e2e/page-models/mobile-budget-page.js (9)
4-47: Remove unused propertythis.navigationA previous review comment mentioned an unused property
this.navigation. This property is not present in the current code, which is good. However, to ensure code cleanliness, double-check if this property is used elsewhere in the class. If not, you can safely ignore this comment.
49-61: LGTM: Well-implemented toggle functionalityThe
toggleVisibleColumnsmethod is well-implemented. It correctly handles the toggling between "Budgeted" and "Spent" columns, and includes proper error handling if neither button is found. This approach ensures robust functionality and clear error reporting.
84-87: LGTM: Correct implementation ofopenBudgetMenuThe
openBudgetMenumethod correctly uses thegetBudgetedButtonmethod to locate and click the budget button for a specific category. The implementation is concise and effective.
116-121: LGTM: Correct implementation ofopenSpentPageThe
openSpentPagemethod correctly uses thegetSpentButtonmethod to locate and click the spent button for a specific category. Returning a new instance ofMobileAccountPageis a good practice for page object models, allowing for seamless navigation between pages.
134-150: LGTM: Well-implementedgetEnvelopeBudgetSummaryButtonmethodThe
getEnvelopeBudgetSummaryButtonmethod is well-implemented. It correctly checks for the visibility of both "To Budget" and "Overbudgeted" buttons, and includes proper error handling. The use of an optional parameterthrowIfNotFoundprovides flexibility in how errors are handled, which is a good practice.
152-155: LGTM: Correct implementation ofopenEnvelopeBudgetSummaryMenuThe
openEnvelopeBudgetSummaryMenumethod correctly uses thegetEnvelopeBudgetSummaryButtonmethod to locate and click the appropriate button. This implementation is concise and leverages the error handling already present in thegetEnvelopeBudgetSummaryButtonmethod.
157-177: LGTM: Well-implementedgetTrackingBudgetSummaryButtonmethodThe
getTrackingBudgetSummaryButtonmethod is well-implemented. It correctly checks for the visibility of "Saved", "Projected Savings", and "Overspent" buttons, and includes proper error handling. The use of the optional parameterthrowIfNotFoundis consistent with thegetEnvelopeBudgetSummaryButtonmethod, which is a good practice for maintaining code consistency.
179-182: LGTM: Correct implementation ofopenTrackingBudgetSummaryMenuThe
openTrackingBudgetSummaryMenumethod correctly uses thegetTrackingBudgetSummaryButtonmethod to locate and click the appropriate button. This implementation is concise and leverages the error handling already present in thegetTrackingBudgetSummaryButtonmethod.
1-182: Overall assessment: Significant improvements with some areas for refinementThe changes to the
MobileBudgetPageclass significantly enhance its functionality, providing a comprehensive interface for interacting with various elements of the mobile budget page. The new methods are well-structured and follow good practices for page object models.Main points for improvement:
- Implement recursion limits in
getBudgetedButtonandgetSpentButtonto prevent potential infinite loops.- Add input validation in the
setBudgetmethod.- Enhance error handling in
openBalanceMenuandopenMonthMenuby checking for button existence and visibility before clicking.These refinements will further improve the robustness and reliability of the class. Overall, the changes represent a substantial improvement to the mobile budget page interactions.
packages/desktop-client/src/components/common/Modal.tsx (1)
421-421: Excellent accessibility improvement!The addition of
aria-label="Modal title"to both theAutoTextSizeandTextOneLinecomponents significantly enhances the accessibility of the modal. This change allows screen readers to properly identify and announce the modal title, improving the user experience for individuals using assistive technologies.Also applies to: 437-437
| async getBudgetedButton(categoryName) { | ||
| let budgetedButton = this.page.getByTestId( | ||
| `budgeted-${categoryName}-button`, | ||
| ); | ||
|
|
||
| if (await budgetedButton.isVisible()) { | ||
| return budgetedButton; | ||
| } | ||
|
|
||
| await this.toggleVisibleColumns(); | ||
| budgetedButton = await this.getBudgetedButton(categoryName); | ||
|
|
||
| if (await budgetedButton.isVisible()) { | ||
| return budgetedButton; | ||
| } | ||
|
|
||
| throw new Error( | ||
| `Budget button for category ${categoryName} could not be located on the page`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement recursion limit in getBudgetedButton
While the method logic is sound, there's a risk of infinite recursion if the button is never found. Consider implementing a recursion limit as suggested in a previous review:
async getBudgetedButton(categoryName, attempts = 0) {
const maxAttempts = 2;
let budgetedButton = this.page.getByTestId(`budgeted-${categoryName}-button`);
if (await budgetedButton.isVisible()) {
return budgetedButton;
}
if (attempts < maxAttempts) {
await this.toggleVisibleColumns();
return this.getBudgetedButton(categoryName, attempts + 1);
}
throw new Error(`Budget button for category ${categoryName} could not be located on the page`);
}This modification limits the number of recursion attempts and prevents potential infinite loops.
| async getSpentButton(categoryName) { | ||
| let spentButton = this.page.getByTestId(`spent-${categoryName}-button`); | ||
|
|
||
| if ((await spentButton.count()) > 0 && (await spentButton.isVisible())) { | ||
| return spentButton; | ||
| } | ||
|
|
||
| await this.toggleVisibleColumns(); | ||
| spentButton = await this.getSpentButton(categoryName); | ||
|
|
||
| if ((await spentButton.count()) > 0) { | ||
| return spentButton; | ||
| } | ||
|
|
||
| throw new Error( | ||
| `Spent button for category ${categoryName} could not be located on the page`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement recursion limit in getSpentButton
Similar to getBudgetedButton, consider implementing a recursion limit in getSpentButton to prevent potential infinite loops:
async getSpentButton(categoryName, attempts = 0) {
const maxAttempts = 2;
let spentButton = this.page.getByTestId(`spent-${categoryName}-button`);
if ((await spentButton.count()) > 0 && (await spentButton.isVisible())) {
return spentButton;
}
if (attempts < maxAttempts) {
await this.toggleVisibleColumns();
return this.getSpentButton(categoryName, attempts + 1);
}
throw new Error(`Spent button for category ${categoryName} could not be located on the page`);
}This modification limits the number of recursion attempts while maintaining the existing checks for button count and visibility.
| async openBalanceMenu(categoryName) { | ||
| const balanceButton = this.page.getByTestId( | ||
| `balance-${categoryName}-button`, | ||
| ); | ||
| await balanceButton.click(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add existence and visibility checks in openBalanceMenu
As suggested in a previous review, consider adding checks for the existence and visibility of the balance button before clicking it. This will prevent potential exceptions if the button is not present or not visible.
Suggested implementation:
async openBalanceMenu(categoryName) {
const balanceButton = this.page.getByTestId(`balance-${categoryName}-button`);
if ((await balanceButton.count()) > 0 && (await balanceButton.isVisible())) {
await balanceButton.click();
} else {
throw new Error(`Balance button for category ${categoryName} could not be located or is not visible.`);
}
}This modification adds a check for the button's existence and visibility before clicking, improving the method's reliability.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (3)
packages/desktop-client/e2e/page-models/mobile-budget-page.js (1)
209-234: LGTM: Consistent implementation of tracking budget summary methodsThe
getTrackingBudgetSummaryButtonandopenTrackingBudgetSummaryMenumethods are well-implemented and consistent with their envelope counterparts:
- Good error handling in
getTrackingBudgetSummaryButton.- Consistent use of the
throwIfNotFoundoption.- Proper use of the getter method in
openTrackingBudgetSummaryMenu.For complete consistency, consider updating the error message in
getTrackingBudgetSummaryButtonto match the style ofgetEnvelopeBudgetSummaryButton:throw new Error( 'Saved/Projected Savings/Overspent button could not be located on the page' );This minor change will ensure full consistency across similar methods.
packages/desktop-client/src/components/common/Modal.tsx (2)
422-422: Great accessibility improvement!Adding the
aria-label="Modal title"to theAutoTextSizecomponent enhances accessibility by providing clear context for screen reader users. This is particularly useful when the title text might be dynamically resized.For consistency, consider using a prop for the aria-label value, allowing it to be customized if needed:
aria-label={`${title} modal title`}This would make the label more specific and potentially more useful for users navigating between multiple modals.
438-438: Excellent accessibility enhancement!Adding the
aria-label="Modal title"to theTextOneLinecomponent improves accessibility by providing clear context for screen reader users. This is particularly useful for single-line titles that don't shrink on overflow.As suggested in the previous comment, consider using a prop for the aria-label value to allow for customization:
aria-label={`${title} modal title`}This would make the label more specific and maintain consistency with the
AutoTextSizecomponent implementation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (26)
packages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-the-accounts-page-and-asserts-on-balances-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--6ab37-roup-name-opens-the-category-group-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--94a79-roup-name-opens-the-category-group-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--bbde3-roup-name-opens-the-category-group-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-category-name-opens-the-category-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-category-name-opens-the-category-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-category-name-opens-the-category-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--5f098-roup-name-opens-the-category-group-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--929be-roup-name-opens-the-category-group-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--dc927-roup-name-opens-the-category-group-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-category-name-opens-the-category-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-category-name-opens-the-category-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-category-name-opens-the-category-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-summary-button-shows-the-summary-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/rules.test.js-snapshots/Rules-creates-a-rule-and-makes-sure-it-is-applied-when-creating-a-transaction-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/schedules.test.js-snapshots/Schedules-creates-a-new-schedule-posts-the-transaction-and-later-completes-it-8-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-test-transaction-1-chromium-linux.pngis excluded by!**/*.png
📒 Files selected for processing (4)
- packages/desktop-client/e2e/budget.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/page-models/mobile-budget-page.js (1 hunks)
- packages/desktop-client/src/components/common/Modal.tsx (3 hunks)
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx (15 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/desktop-client/e2e/budget.mobile.test.js
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx
🔇 Additional comments (9)
packages/desktop-client/e2e/page-models/mobile-budget-page.js (7)
1-3: LGTM: Import and class declaration are appropriate.The import of
MobileAccountPageand the export ofMobileBudgetPageclass are correctly implemented.
186-207: LGTM: Well-structured envelope budget summary methodsThe
getEnvelopeBudgetSummaryButtonandopenEnvelopeBudgetSummaryMenumethods are well-implemented:
- Good error handling in
getEnvelopeBudgetSummaryButton.- Flexible approach with the
throwIfNotFoundoption.- Proper use of the getter method in
openEnvelopeBudgetSummaryMenu.These methods demonstrate good practices in error handling and flexibility.
1-234: Overall: Well-implementedMobileBudgetPageclass with room for minor improvementsThe
MobileBudgetPageclass provides a comprehensive set of methods for interacting with a mobile budget page. Key strengths include:
- Consistent use of Playwright's page object model.
- Well-structured methods with clear purposes.
- Good error handling in many methods.
Areas for improvement:
- Enhance error handling in some methods, particularly for button visibility and existence checks.
- Implement recursion limits in methods like
getBudgetedButtonandgetSpentButton.- Add input validation for methods like
setBudget.- Improve code organization in the constructor by grouping related properties.
Overall, this is a solid implementation that, with these minor improvements, will be more robust and maintainable.
113-132: 🛠️ Refactor suggestionImplement recursion limit in
getBudgetedButtonWhile the method logic is sound, there's a risk of infinite recursion if the button is never found. Consider implementing a recursion limit:
async getBudgetedButton(categoryName, attempts = 0) { const maxAttempts = 2; let budgetedButton = this.budgetTable.getByTestId(`budgeted-${categoryName}-button`); if (await budgetedButton.isVisible()) { return budgetedButton; } if (attempts < maxAttempts) { await this.toggleVisibleColumns(); return this.getBudgetedButton(categoryName, attempts + 1); } throw new Error(`Budget button for category ${categoryName} could not be located on the page`); }This modification limits the number of recursion attempts and prevents potential infinite loops.
139-145: 🛠️ Refactor suggestionAdd input validation to
setBudgetThe
setBudgetmethod correctly sets a new budget amount. However, consider adding input validation for thenewAmountparameter to ensure it's a valid number before attempting to set it.Example:
async setBudget(categoryName, newAmount) { if (isNaN(newAmount) || newAmount < 0) { throw new Error('Invalid budget amount'); } const budgetedButton = await this.getBudgetedButton(categoryName); await budgetedButton.click(); await this.page.keyboard.type(String(newAmount)); await this.page.keyboard.press('Enter'); }
147-173: 🛠️ Refactor suggestionApply consistent improvements to
getSpentButtonThe
getSpentButtonmethod uses similar logic togetBudgetedButton. To maintain consistency and prevent potential infinite recursion:
- Implement a recursion limit in
getSpentButtonsimilar to the suggestion forgetBudgetedButton.- Consider adding error handling in
openSpentPageto catch potential errors when creating the newMobileAccountPageinstance.Example for
getSpentButton:async getSpentButton(categoryName, attempts = 0) { const maxAttempts = 2; let spentButton = this.budgetTable.getByTestId(`spent-${categoryName}-button`); if (await spentButton.isVisible()) { return spentButton; } if (attempts < maxAttempts) { await this.toggleVisibleColumns(); return this.getSpentButton(categoryName, attempts + 1); } throw new Error(`Spent button for category ${categoryName} could not be located on the page`); }This modification aligns the implementation with
getBudgetedButtonand prevents potential infinite loops.
175-184: 🛠️ Refactor suggestionAdd error handling to
openBalanceMenuandopenMonthMenuBoth methods currently assume the buttons will always be present and clickable. To improve robustness:
- Add checks for button existence and visibility before clicking.
- Implement error handling if the buttons are not found or not clickable.
Example for
openBalanceMenu:async openBalanceMenu(categoryName) { const balanceButton = this.budgetTable.getByTestId(`balance-${categoryName}-button`); if (await balanceButton.isVisible()) { await balanceButton.click(); } else { throw new Error(`Balance button for category ${categoryName} could not be located or is not visible.`); } }Apply similar error handling to
openMonthMenu.packages/desktop-client/src/components/common/Modal.tsx (2)
321-321: Excellent addition for accessibility!Adding the
aria-label="Modal logo"to theSvgLogocomponent significantly improves accessibility. This label provides context for screen reader users, helping them understand the purpose of this visual element.
Line range hint
1-478: Summary: Excellent accessibility improvements throughout the Modal componentThe changes made to this file consistently enhance the accessibility of the Modal component by adding appropriate
aria-labelattributes to key elements. These additions will significantly improve the experience for users relying on screen readers.To further improve the implementation:
- Consider using dynamic aria-labels that incorporate the actual title text for more specific and useful labels.
- Ensure consistency across all uses of aria-labels within the Modal component.
Overall, these changes represent a positive step towards a more accessible user interface. Great work!
| async openBudgetPageMenu() { | ||
| await this.budgetPageMenuButton.click(); | ||
| } | ||
|
|
||
| async getCategoryGroupNameForRow(idx) { | ||
| return this.categoryGroupNames.nth(idx).textContent(); | ||
| } | ||
|
|
||
| getCategoryGroupButton(categoryGroupName) { | ||
| return this.categoryGroupRows.getByRole('button', { | ||
| name: categoryGroupName, | ||
| }); | ||
| } | ||
|
|
||
| async openCategoryGroupMenu(categoryGroupName) { | ||
| const categoryGroupButton = | ||
| await this.getCategoryGroupButton(categoryGroupName); | ||
| await categoryGroupButton.click(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to category group methods.
The methods are concise and use Playwright's locators effectively. However, consider adding error handling to improve robustness:
Example for openCategoryGroupMenu:
async openCategoryGroupMenu(categoryGroupName) {
const categoryGroupButton = await this.getCategoryGroupButton(categoryGroupName);
if (!categoryGroupButton) {
throw new Error(`Category group '${categoryGroupName}' not found`);
}
await categoryGroupButton.click();
}Apply similar error handling to other methods where appropriate.
| async getCategoryNameForRow(idx) { | ||
| return this.categoryNames.nth(idx).textContent(); | ||
| } | ||
|
|
||
| getCategoryButton(categoryName) { | ||
| return this.categoryRows.getByRole('button', { | ||
| name: categoryName, | ||
| }); | ||
| } | ||
|
|
||
| async openCategoryMenu(categoryName) { | ||
| const categoryButton = await this.getCategoryButton(categoryName); | ||
| await categoryButton.click(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure consistent error handling across category methods.
The category methods are well-structured and consistent with the category group methods. To maintain this consistency and improve robustness:
- Add error handling to
openCategoryMenusimilar to the suggestion foropenCategoryGroupMenu. - Consider adding a check in
getCategoryNameForRowto ensure the index is valid.
Example for openCategoryMenu:
async openCategoryMenu(categoryName) {
const categoryButton = await this.getCategoryButton(categoryName);
if (!categoryButton) {
throw new Error(`Category '${categoryName}' not found`);
}
await categoryButton.click();
}
joel-jeremy
force-pushed
the
mobile-vrts-1
branch
from
e1d7849 to
03d4e3f
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (2)
packages/desktop-client/e2e/page-models/settings-page.js (2)
10-18: Approve the newuseBudgetTypemethod with suggestions for improvement.The new
useBudgetTypemethod is a good addition for managing budget types. However, consider the following improvements for increased robustness:
- Add error handling in case the specified
budgetTypedoesn't exist.- Consider checking if the 'Budget mode toggle' is already enabled before enabling it again.
- Add verification that the budget type was successfully changed after clicking the button.
Here's a suggested improvement:
async useBudgetType(budgetType) { await this.enableExperimentalFeature('Budget mode toggle'); const switchBudgetTypeButton = this.page.getByRole('button', { name: `Switch to ${budgetType} budgeting`, }); if (await switchBudgetTypeButton.isVisible()) { await switchBudgetTypeButton.click(); // Verify the change was successful (adjust the selector as needed) const newBudgetTypeIndicator = this.page.getByText(`Current: ${budgetType} budgeting`); await newBudgetTypeIndicator.waitFor({ state: 'visible', timeout: 5000 }); } else { throw new Error(`Budget type "${budgetType}" not found or not available.`); } }This version includes error handling and verification of the budget type change.
20-32: Approve the changes toenableExperimentalFeaturewith suggestions for improvement.The modifications to the
enableExperimentalFeaturemethod improve element selection reliability. However, consider the following enhancements:
- Add error handling in case elements are not found.
- Check if the feature is already enabled before clicking the checkbox.
- Add verification that the feature was successfully enabled after clicking.
Here's a suggested improvement:
async enableExperimentalFeature(featureName) { const advancedSettingsButton = this.page.getByTestId('advanced-settings'); await advancedSettingsButton.click(); const experimentalSettingsButton = this.page.getByTestId('experimental-settings'); await experimentalSettingsButton.click(); const featureCheckbox = this.page.getByRole('checkbox', { name: featureName }); try { await featureCheckbox.waitFor({ state: 'visible', timeout: 5000 }); if (!(await featureCheckbox.isChecked())) { await featureCheckbox.click(); // Verify the change was successful await featureCheckbox.waitFor({ state: 'checked', timeout: 5000 }); } else { console.log(`Feature "${featureName}" is already enabled.`); } } catch (error) { throw new Error(`Failed to enable feature "${featureName}": ${error.message}`); } }This version includes error handling, checks if the feature is already enabled, and verifies the change was successful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (124)
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-4-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-5-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-6-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/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-the-accounts-page-and-asserts-on-balances-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-the-accounts-page-and-asserts-on-balances-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-the-accounts-page-and-asserts-on-balances-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--14404-in-the-page-header-opens-the-month-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--321fd-ed-amount-opens-the-budget-summary-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--4bb70-ed-amount-opens-the-budget-summary-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--6ab37-roup-name-opens-the-category-group-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--94a79-roup-name-opens-the-category-group-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--94a85-ed-amount-opens-the-budget-summary-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--bbde3-roup-name-opens-the-category-group-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--bed18-in-the-page-header-opens-the-month-menu-modal-1-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--ceb3a-in-the-page-header-opens-the-month-menu-modal-2-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-Envelope-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-category-name-opens-the-category-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-category-name-opens-the-category-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-category-name-opens-the-category-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-loads-the-budget-page-with-budgeted-amounts-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-loads-the-budget-page-with-budgeted-amounts-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-loads-the-budget-page-with-budgeted-amounts-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-updates-the-budgeted-amount-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-updates-the-budgeted-amount-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-updates-the-budgeted-amount-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--0ba04-nt-amount-opens-the-budget-summary-menu-modal-1-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--1ce6d-nt-amount-opens-the-budget-summary-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--42062-in-the-page-header-opens-the-month-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--49fb6-in-the-page-header-opens-the-month-menu-modal-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/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--5f098-roup-name-opens-the-category-group-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--929be-roup-name-opens-the-category-group-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--b1562-in-the-page-header-opens-the-month-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--dc927-roup-name-opens-the-category-group-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--f224f-nt-amount-opens-the-budget-summary-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-category-name-opens-the-category-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-category-name-opens-the-category-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-category-name-opens-the-category-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-loads-the-budget-page-with-budgeted-amounts-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-loads-the-budget-page-with-budgeted-amounts-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-loads-the-budget-page-with-budgeted-amounts-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-updates-the-budgeted-amount-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-updates-the-budgeted-amount-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-updates-the-budgeted-amount-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.test.js-snapshots/Budget-renders-the-summary-information-available-funds-overspent-budgeted-and-for-next-month-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.test.js-snapshots/Budget-renders-the-summary-information-available-funds-overspent-budgeted-and-for-next-month-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-checks-that-settings-page-can-be-opened-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-creates-a-transaction-from-accounts-id-page-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-creates-a-transaction-from-accounts-id-page-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-creates-a-transaction-via-footer-button-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-loads-the-budget-page-with-budgeted-amounts-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-loads-the-budget-page-with-budgeted-amounts-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-loads-the-budget-page-with-budgeted-amounts-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-opens-the-accounts-page-and-asserts-on-balances-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-opens-the-accounts-page-and-asserts-on-balances-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-opens-the-accounts-page-and-asserts-on-balances-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-summary-button-shows-the-summary-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/rules.test.js-snapshots/Rules-checks-the-page-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/rules.test.js-snapshots/Rules-creates-a-rule-and-makes-sure-it-is-applied-when-creating-a-transaction-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/rules.test.js-snapshots/Rules-creates-a-split-transaction-rule-and-makes-sure-it-is-applied-when-creating-a-transaction-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/schedules.test.js-snapshots/Schedules-creates-a-new-schedule-posts-the-transaction-and-later-completes-it-7-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/schedules.test.js-snapshots/Schedules-creates-a-new-schedule-posts-the-transaction-and-later-completes-it-8-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-6-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-6-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!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-test-transaction-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-6-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-filters-transactions-by-date-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-filters-transactions-by-date-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-filters-transactions-by-date-6-chromium-linux.pngis excluded by!**/*.pngupcoming-release-notes/3506.mdis excluded by!**/*.mdyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
- packages/desktop-client/e2e/accounts.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/budget.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/page-models/mobile-account-page.js (1 hunks)
- packages/desktop-client/e2e/page-models/mobile-budget-page.js (1 hunks)
- packages/desktop-client/e2e/page-models/settings-page.js (1 hunks)
- packages/desktop-client/e2e/settings.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/transactions.mobile.test.js (1 hunks)
- packages/desktop-client/playwright.config.js (1 hunks)
- packages/desktop-client/src/components/common/Modal.tsx (3 hunks)
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx (15 hunks)
- packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (2 hunks)
- packages/desktop-client/src/components/modals/EnvelopeBudgetMenuModal.tsx (1 hunks)
- packages/desktop-client/src/components/modals/TrackingBudgetMenuModal.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/desktop-client/e2e/accounts.mobile.test.js
- packages/desktop-client/e2e/budget.mobile.test.js
- packages/desktop-client/e2e/page-models/mobile-account-page.js
- packages/desktop-client/e2e/settings.mobile.test.js
- packages/desktop-client/e2e/transactions.mobile.test.js
- packages/desktop-client/playwright.config.js
- packages/desktop-client/src/components/common/Modal.tsx
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx
- packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx
- packages/desktop-client/src/components/modals/EnvelopeBudgetMenuModal.tsx
- packages/desktop-client/src/components/modals/TrackingBudgetMenuModal.tsx
🔇 Additional comments (1)
packages/desktop-client/e2e/page-models/mobile-budget-page.js (1)
1-234: Overall assessment: Well-structured with room for improvementsThe
MobileBudgetPageclass provides a comprehensive set of methods for interacting with a mobile budget page in end-to-end tests. The code is generally well-structured and uses Playwright's Page object model effectively.Key strengths:
- Comprehensive coverage of UI elements and interactions.
- Good use of Playwright's locators and methods.
- Logical organization of methods by functionality.
Areas for improvement:
- Refactor the constructor for better organization and readability.
- Enhance error handling in various methods, especially for edge cases.
- Implement safeguards against potential infinite loops in recursive methods.
- Add input validation for methods that accept user input.
Addressing these points will further improve the robustness and maintainability of the code.
| async getCategoryGroupNameForRow(idx) { | ||
| return this.categoryGroupNames.nth(idx).textContent(); | ||
| } | ||
|
|
||
| getCategoryGroupButton(categoryGroupName) { | ||
| return this.categoryGroupRows.getByRole('button', { | ||
| name: categoryGroupName, | ||
| }); | ||
| } | ||
|
|
||
| async openCategoryGroupMenu(categoryGroupName) { | ||
| const categoryGroupButton = | ||
| await this.getCategoryGroupButton(categoryGroupName); | ||
| await categoryGroupButton.click(); | ||
| } | ||
|
|
||
| async getCategoryNameForRow(idx) { | ||
| return this.categoryNames.nth(idx).textContent(); | ||
| } | ||
|
|
||
| getCategoryButton(categoryName) { | ||
| return this.categoryRows.getByRole('button', { | ||
| name: categoryName, | ||
| }); | ||
| } | ||
|
|
||
| async openCategoryMenu(categoryName) { | ||
| const categoryButton = await this.getCategoryButton(categoryName); | ||
| await categoryButton.click(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling in category and group-related methods.
The methods are well-structured, but could benefit from improved error handling:
- In
getCategoryGroupNameForRowandgetCategoryNameForRow, check if the index is valid before accessing the element. - In
openCategoryGroupMenuandopenCategoryMenu, handle cases where the button is not found.
Example improvements:
async getCategoryGroupNameForRow(idx) {
const count = await this.categoryGroupNames.count();
if (idx >= count) {
throw new Error(`Invalid index: ${idx}. Only ${count} category groups available.`);
}
return this.categoryGroupNames.nth(idx).textContent();
}
async openCategoryMenu(categoryName) {
const categoryButton = await this.getCategoryButton(categoryName);
if (!categoryButton) {
throw new Error(`Category '${categoryName}' not found`);
}
await categoryButton.click();
}Apply similar error handling to other methods where appropriate.
| async openBalanceMenu(categoryName) { | ||
| const balanceButton = this.budgetTable.getByTestId( | ||
| `balance-${categoryName}-button`, | ||
| ); | ||
| await balanceButton.click(); | ||
| } | ||
|
|
||
| async openMonthMenu() { | ||
| await this.selectedBudgetMonthButton.click(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling in navigation methods.
The openBalanceMenu and openMonthMenu methods could benefit from improved error handling. Consider checking if the buttons exist and are visible before clicking:
async openBalanceMenu(categoryName) {
const balanceButton = this.budgetTable.getByTestId(`balance-${categoryName}-button`);
if (await balanceButton.isVisible()) {
await balanceButton.click();
} else {
throw new Error(`Balance button for category ${categoryName} not found or not visible`);
}
}
async openMonthMenu() {
if (await this.selectedBudgetMonthButton.isVisible()) {
await this.selectedBudgetMonthButton.click();
} else {
throw new Error('Selected budget month button not found or not visible');
}
}These changes will make the methods more robust by handling cases where the buttons might not be available.
joel-jeremy
force-pushed
the
mobile-vrts-1
branch
from
03d4e3f to
53ccc5e
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (2)
packages/desktop-client/e2e/page-models/settings-page.js (2)
10-18: Approve the new method with suggestions for improvement.The new
useBudgetTypemethod is a valuable addition for managing budget types. However, consider enhancing its robustness:
- Add error handling in case the 'Budget mode toggle' feature or the switch button is not found.
- Implement a verification step to confirm the budget type switch was successful.
- Consider adding a timeout or retry mechanism for the button click action.
Here's a suggested improvement:
async useBudgetType(budgetType) { try { await this.enableExperimentalFeature('Budget mode toggle'); const switchBudgetTypeButton = this.page.getByRole('button', { name: `Switch to ${budgetType} budgeting`, }); await switchBudgetTypeButton.click(); // Verify the switch was successful (adjust the selector as needed) await this.page.waitForSelector(`[data-testid="current-budget-type-${budgetType}"]`, { timeout: 5000 }); } catch (error) { console.error(`Failed to switch to ${budgetType} budgeting:`, error); throw error; } }This implementation adds error handling and a verification step to ensure the budget type switch was successful.
20-32: Approve the changes with suggestions for improvement.The modifications to
enableExperimentalFeaturemethod improve accessibility and selector specificity. However, consider the following enhancements:
- Implement error handling for cases where elements are not found or click actions fail.
- Add a verification step to confirm the experimental feature was successfully enabled.
- Consider adding timeouts or retry mechanisms for the click actions.
Here's a suggested improvement:
async enableExperimentalFeature(featureName) { try { const advancedSettingsButton = this.page.getByTestId('advanced-settings'); await advancedSettingsButton.click(); const experimentalSettingsButton = this.page.getByTestId('experimental-settings'); await experimentalSettingsButton.click(); const featureCheckbox = this.page.getByRole('checkbox', { name: featureName }); await featureCheckbox.click(); // Verify the feature was enabled (adjust the selector as needed) await this.page.waitForSelector(`[data-testid="enabled-feature-${featureName}"]`, { timeout: 5000 }); } catch (error) { console.error(`Failed to enable experimental feature ${featureName}:`, error); throw error; } }This implementation adds error handling and a verification step to ensure the experimental feature was successfully enabled.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (123)
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-4-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-5-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-6-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/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-the-accounts-page-and-asserts-on-balances-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-the-accounts-page-and-asserts-on-balances-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-the-accounts-page-and-asserts-on-balances-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--14404-in-the-page-header-opens-the-month-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--321fd-ed-amount-opens-the-budget-summary-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--4bb70-ed-amount-opens-the-budget-summary-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--6ab37-roup-name-opens-the-category-group-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--94a79-roup-name-opens-the-category-group-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--94a85-ed-amount-opens-the-budget-summary-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--bbde3-roup-name-opens-the-category-group-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--bed18-in-the-page-header-opens-the-month-menu-modal-1-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--ceb3a-in-the-page-header-opens-the-month-menu-modal-2-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-Envelope-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-category-name-opens-the-category-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-category-name-opens-the-category-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-category-name-opens-the-category-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-loads-the-budget-page-with-budgeted-amounts-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-loads-the-budget-page-with-budgeted-amounts-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-loads-the-budget-page-with-budgeted-amounts-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-updates-the-budgeted-amount-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-updates-the-budgeted-amount-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-updates-the-budgeted-amount-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--0ba04-nt-amount-opens-the-budget-summary-menu-modal-1-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--1ce6d-nt-amount-opens-the-budget-summary-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--42062-in-the-page-header-opens-the-month-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--49fb6-in-the-page-header-opens-the-month-menu-modal-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/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--5f098-roup-name-opens-the-category-group-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--929be-roup-name-opens-the-category-group-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--b1562-in-the-page-header-opens-the-month-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--dc927-roup-name-opens-the-category-group-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--f224f-nt-amount-opens-the-budget-summary-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-category-name-opens-the-category-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-category-name-opens-the-category-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-category-name-opens-the-category-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-loads-the-budget-page-with-budgeted-amounts-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-loads-the-budget-page-with-budgeted-amounts-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-loads-the-budget-page-with-budgeted-amounts-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-updates-the-budgeted-amount-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-updates-the-budgeted-amount-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-updates-the-budgeted-amount-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.test.js-snapshots/Budget-renders-the-summary-information-available-funds-overspent-budgeted-and-for-next-month-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.test.js-snapshots/Budget-renders-the-summary-information-available-funds-overspent-budgeted-and-for-next-month-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-checks-that-settings-page-can-be-opened-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-creates-a-transaction-from-accounts-id-page-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-creates-a-transaction-via-footer-button-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-loads-the-budget-page-with-budgeted-amounts-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-loads-the-budget-page-with-budgeted-amounts-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-loads-the-budget-page-with-budgeted-amounts-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-opens-the-accounts-page-and-asserts-on-balances-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-opens-the-accounts-page-and-asserts-on-balances-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-opens-the-accounts-page-and-asserts-on-balances-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-summary-button-shows-the-summary-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/rules.test.js-snapshots/Rules-checks-the-page-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/rules.test.js-snapshots/Rules-creates-a-rule-and-makes-sure-it-is-applied-when-creating-a-transaction-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/rules.test.js-snapshots/Rules-creates-a-split-transaction-rule-and-makes-sure-it-is-applied-when-creating-a-transaction-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/schedules.test.js-snapshots/Schedules-creates-a-new-schedule-posts-the-transaction-and-later-completes-it-7-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/schedules.test.js-snapshots/Schedules-creates-a-new-schedule-posts-the-transaction-and-later-completes-it-8-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-6-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-6-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!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-test-transaction-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-6-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-filters-transactions-by-date-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-filters-transactions-by-date-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-filters-transactions-by-date-6-chromium-linux.pngis excluded by!**/*.pngupcoming-release-notes/3506.mdis excluded by!**/*.mdyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
- packages/desktop-client/e2e/accounts.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/budget.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/page-models/mobile-account-page.js (1 hunks)
- packages/desktop-client/e2e/page-models/mobile-budget-page.js (1 hunks)
- packages/desktop-client/e2e/page-models/settings-page.js (1 hunks)
- packages/desktop-client/e2e/settings.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/transactions.mobile.test.js (1 hunks)
- packages/desktop-client/playwright.config.js (1 hunks)
- packages/desktop-client/src/components/common/Modal.tsx (3 hunks)
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx (15 hunks)
- packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (2 hunks)
- packages/desktop-client/src/components/modals/EnvelopeBudgetMenuModal.tsx (1 hunks)
- packages/desktop-client/src/components/modals/TrackingBudgetMenuModal.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/desktop-client/e2e/accounts.mobile.test.js
- packages/desktop-client/e2e/budget.mobile.test.js
- packages/desktop-client/e2e/page-models/mobile-account-page.js
- packages/desktop-client/e2e/settings.mobile.test.js
- packages/desktop-client/e2e/transactions.mobile.test.js
- packages/desktop-client/playwright.config.js
- packages/desktop-client/src/components/common/Modal.tsx
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx
- packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx
- packages/desktop-client/src/components/modals/EnvelopeBudgetMenuModal.tsx
- packages/desktop-client/src/components/modals/TrackingBudgetMenuModal.tsx
🔇 Additional comments (1)
packages/desktop-client/e2e/page-models/mobile-budget-page.js (1)
1-234: Overall assessment: Well-structured class with room for robustness improvementsThe
MobileBudgetPageclass provides a comprehensive set of methods for interacting with various elements of a mobile budget page. The code is generally well-structured and follows good practices for page object models in Playwright.Key strengths:
- Comprehensive coverage of UI interactions
- Clear method naming conventions
- Use of Playwright's locator patterns for element selection
Areas for improvement:
- Refactor the constructor for better readability and maintainability
- Enhance error handling in several methods
- Add input validation where appropriate
- Prevent potential infinite recursion in methods like
getBudgetedButtonImplementing the suggested improvements will make the class more robust and easier to maintain. Consider also adding JSDoc comments to describe the purpose and parameters of each method, which would improve the overall documentation of the class.
| constructor(page) { | ||
| this.page = page; | ||
|
|
||
| this.categoryNames = page | ||
| this.categoryRows = page | ||
| .getByTestId('budget-groups') | ||
| .getByTestId('category-name'); | ||
| .getByTestId('category-row'); | ||
|
|
||
| this.categoryNames = this.categoryRows.getByTestId('category-name'); | ||
|
|
||
| this.categoryGroupRows = page | ||
| .getByTestId('budget-groups') | ||
| .getByTestId('category-group-row'); | ||
|
|
||
| this.categoryGroupNames = this.categoryGroupRows.getByTestId( | ||
| 'category-group-name', | ||
| ); | ||
|
|
||
| this.heading = page.getByRole('heading'); | ||
| this.selectedBudgetMonthButton = this.heading.getByLabel( | ||
| 'Selected budget month', | ||
| ); | ||
| this.budgetPageMenuButton = page.getByLabel('Budget page menu'); | ||
|
|
||
| this.budgetTableHeader = page.getByTestId('budget-table-header'); | ||
|
|
||
| // Envelope budget summary buttons | ||
| this.toBudgetButton = this.budgetTableHeader.getByRole('button', { | ||
| name: 'To Budget', | ||
| }); | ||
| this.overbudgetedButton = this.budgetTableHeader.getByRole('button', { | ||
| name: 'Overbudgeted', | ||
| }); | ||
|
|
||
| // Tracking budget summary buttons | ||
| this.savedButton = this.budgetTableHeader.getByRole('button', { | ||
| name: 'Saved', | ||
| }); | ||
| this.projectedSavingsButton = this.budgetTableHeader.getByRole('button', { | ||
| name: 'Projected Savings', | ||
| }); | ||
| this.overspentButton = this.budgetTableHeader.getByRole('button', { | ||
| name: 'Overspent', | ||
| }); | ||
|
|
||
| this.budgetedHeaderButton = this.budgetTableHeader.getByRole('button', { | ||
| name: 'Budgeted', | ||
| }); | ||
| this.spentHeaderButton = this.budgetTableHeader.getByRole('button', { | ||
| name: 'Spent', | ||
| }); | ||
|
|
||
| this.budgetTable = page.getByTestId('budget-table'); | ||
| this.budgetType = | ||
| this.getEnvelopeBudgetSummaryButton({ | ||
| throwIfNotFound: false, | ||
| }) !== null | ||
| ? 'Envelope' | ||
| : 'Tracking'; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider refactoring the constructor for improved readability
The constructor is comprehensive but quite long. Consider refactoring it to improve readability and maintainability:
- Group related properties (e.g., all buttons, all row-related properties).
- Extract the budget type determination logic into a separate method.
- Use private fields (prefixed with #) for internal properties not meant to be accessed outside the class.
Example refactoring:
constructor(page) {
this.page = page;
this.#initializeRowProperties();
this.#initializeButtonProperties();
this.#initializeHeaderProperties();
this.budgetType = this.#determineBudgetType();
}
#initializeRowProperties() {
// Initialize this.categoryRows, this.categoryNames, etc.
}
#initializeButtonProperties() {
// Initialize this.toBudgetButton, this.overbudgetedButton, etc.
}
#initializeHeaderProperties() {
// Initialize this.heading, this.selectedBudgetMonthButton, etc.
}
#determineBudgetType() {
return this.getEnvelopeBudgetSummaryButton({ throwIfNotFound: false }) !== null
? 'Envelope'
: 'Tracking';
}This refactoring will make the constructor more concise and easier to maintain.
| async getBudgetedButton(categoryName) { | ||
| let budgetedButton = this.budgetTable.getByTestId( | ||
| `budgeted-${categoryName}-button`, | ||
| ); | ||
|
|
||
| if (await budgetedButton.isVisible()) { | ||
| return budgetedButton; | ||
| } | ||
|
|
||
| await this.toggleVisibleColumns(); | ||
| budgetedButton = await this.getBudgetedButton(categoryName); | ||
|
|
||
| if (await budgetedButton.isVisible()) { | ||
| return budgetedButton; | ||
| } | ||
|
|
||
| throw new Error( | ||
| `Budget button for category ${categoryName} could not be located on the page`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent potential infinite recursion in getBudgetedButton method
The getBudgetedButton method could potentially lead to infinite recursion. Consider adding a maximum number of attempts:
async getBudgetedButton(categoryName, attempts = 0) {
const maxAttempts = 2;
let budgetedButton = this.budgetTable.getByTestId(`budgeted-${categoryName}-button`);
if (await budgetedButton.isVisible()) {
return budgetedButton;
}
if (attempts < maxAttempts) {
await this.toggleVisibleColumns();
return this.getBudgetedButton(categoryName, attempts + 1);
}
throw new Error(`Budget button for category ${categoryName} could not be located on the page`);
}This change limits the number of recursion attempts and prevents potential infinite loops.
| async setBudget(categoryName, newAmount) { | ||
| const budgetedButton = await this.getBudgetedButton(categoryName); | ||
| await budgetedButton.click(); | ||
|
|
||
| await this.page.keyboard.type(String(newAmount)); | ||
| await this.page.keyboard.press('Enter'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation to setBudget method
The setBudget method should validate the newAmount parameter to ensure it's a valid number:
async setBudget(categoryName, newAmount) {
if (typeof newAmount !== 'number' || isNaN(newAmount) || newAmount < 0) {
throw new Error('Invalid budget amount. Must be a non-negative number.');
}
const budgetedButton = await this.getBudgetedButton(categoryName);
await budgetedButton.click();
await this.page.keyboard.type(String(newAmount));
await this.page.keyboard.press('Enter');
}This change ensures that only valid budget amounts are set.
| async openBalanceMenu(categoryName) { | ||
| const balanceButton = this.budgetTable.getByTestId( | ||
| `balance-${categoryName}-button`, | ||
| ); | ||
| await balanceButton.click(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to openBalanceMenu method
The openBalanceMenu method should handle cases where the balance button is not found:
async openBalanceMenu(categoryName) {
const balanceButton = this.budgetTable.getByTestId(`balance-${categoryName}-button`);
if (await balanceButton.isVisible()) {
await balanceButton.click();
} else {
throw new Error(`Balance button for category ${categoryName} not found or not visible`);
}
}This change makes the method more robust by handling cases where the button might not be available.
| async openMonthMenu() { | ||
| await this.selectedBudgetMonthButton.click(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to openMonthMenu method
The openMonthMenu method should handle cases where the month button is not found:
async openMonthMenu() {
if (await this.selectedBudgetMonthButton.isVisible()) {
await this.selectedBudgetMonthButton.click();
} else {
throw new Error('Selected budget month button not found or not visible');
}
}This change makes the method more robust by handling cases where the button might not be available.
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (23)
packages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--589a6-the-page-header-shows-the-next-month-s-budget-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--6dbdb-page-header-shows-the-previous-month-s-budget-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--7bd8f-the-page-header-shows-the-next-month-s-budget-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--884ac-the-page-header-shows-the-next-month-s-budget-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--96ebb-page-header-shows-the-previous-month-s-budget-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--9e6aa-in-the-page-header-opens-the-budget-page-menu-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--d270d-in-the-page-header-opens-the-budget-page-menu-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--d7184-page-header-shows-the-previous-month-s-budget-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--fdd57-in-the-page-header-opens-the-budget-page-menu-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--0dfe7-page-header-shows-the-previous-month-s-budget-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--1ce6d-nt-amount-opens-the-budget-summary-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--7c353-the-page-header-shows-the-next-month-s-budget-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--a3783-in-the-page-header-opens-the-budget-page-menu-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--a8b5e-in-the-page-header-opens-the-budget-page-menu-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--cfb69-page-header-shows-the-previous-month-s-budget-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--d5af6-the-page-header-shows-the-next-month-s-budget-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--f2198-the-page-header-shows-the-next-month-s-budget-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--f7fa3-page-header-shows-the-previous-month-s-budget-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--f8a19-in-the-page-header-opens-the-budget-page-menu-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-6-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-2-chromium-linux.pngis excluded by!**/*.png
📒 Files selected for processing (3)
- packages/desktop-client/e2e/budget.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/page-models/mobile-budget-page.js (1 hunks)
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx (16 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/desktop-client/e2e/budget.mobile.test.js
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx
🔇 Additional comments (1)
packages/desktop-client/e2e/page-models/mobile-budget-page.js (1)
1-269: Overall, well-structured class with room for minor improvementsThe
MobileBudgetPageclass is well-organized and provides comprehensive functionality for interacting with a mobile budget page. The separation of concerns and the use of Playwright's locators are commendable.Main areas for improvement:
- Extracting the budget type determination to a separate method.
- Adding timeout mechanisms to methods that might loop, like
toggleVisibleColumns.- Reducing code duplication in similar methods, such as
getBudgetCellButtonandgetSpentCellButton.- Enhancing error handling in methods like
openBalanceMenu.- Improving error messages for better debugging.
These improvements will further enhance the robustness and maintainability of the code.
| async toggleVisibleColumns() { | ||
| if (await this.budgetedHeaderButton.isVisible()) { | ||
| await this.budgetedHeaderButton.click(); | ||
| return; | ||
| } | ||
|
|
||
| if (await this.spentHeaderButton.isVisible()) { | ||
| await this.spentHeaderButton.click(); | ||
| return; | ||
| } | ||
|
|
||
| throw new Error('Budgeted/Spent columns could not be located on the page'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a timeout mechanism to toggleVisibleColumns
The toggleVisibleColumns method is well-structured, but it could benefit from a timeout mechanism to prevent potential infinite loops.
Consider adding a maximum number of attempts:
async toggleVisibleColumns(maxAttempts = 3) {
for (let i = 0; i < maxAttempts; i++) {
if (await this.budgetedHeaderButton.isVisible()) {
await this.budgetedHeaderButton.click();
return;
}
if (await this.spentHeaderButton.isVisible()) {
await this.spentHeaderButton.click();
return;
}
await this.page.waitForTimeout(1000); // Wait for 1 second before retrying
}
throw new Error('Budgeted/Spent columns could not be located on the page');
}This change adds a retry mechanism with a maximum number of attempts, making the method more robust.
| async getBudgetCellButton(categoryName) { | ||
| let budgetedButton = this.budgetTable.getByRole('button', { | ||
| name: `Open budget menu for ${categoryName} category`, | ||
| }); | ||
|
|
||
| if (await budgetedButton.isVisible()) { | ||
| return budgetedButton; | ||
| } | ||
|
|
||
| await this.toggleVisibleColumns(); | ||
| budgetedButton = await this.getBudgetCellButton(categoryName); | ||
|
|
||
| if (await budgetedButton.isVisible()) { | ||
| return budgetedButton; | ||
| } | ||
|
|
||
| throw new Error( | ||
| `Budget button for category ${categoryName} could not be located on the page`, | ||
| ); | ||
| } | ||
|
|
||
| async openBudgetMenu(categoryName) { | ||
| const budgetedButton = await this.getBudgetCellButton(categoryName); | ||
| await budgetedButton.click(); | ||
| } | ||
|
|
||
| async setBudget(categoryName, newAmount) { | ||
| const budgetedButton = await this.getBudgetCellButton(categoryName); | ||
| await budgetedButton.click(); | ||
|
|
||
| await this.page.keyboard.type(String(newAmount)); | ||
| await this.page.keyboard.press('Enter'); | ||
| } | ||
|
|
||
| async getSpentCellButton(categoryName) { | ||
| let spentButton = this.budgetTable.getByRole('button', { | ||
| name: `Show transactions for ${categoryName} category`, | ||
| }); | ||
|
|
||
| if (await spentButton.isVisible()) { | ||
| return spentButton; | ||
| } | ||
|
|
||
| await this.toggleVisibleColumns(); | ||
| spentButton = await this.getSpentCellButton(categoryName); | ||
|
|
||
| if (await spentButton.isVisible()) { | ||
| return spentButton; | ||
| } | ||
|
|
||
| throw new Error( | ||
| `Spent button for category ${categoryName} could not be located on the page`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce code duplication in getBudgetCellButton and getSpentCellButton
The getBudgetCellButton and getSpentCellButton methods have similar structures and could benefit from a shared helper method to reduce code duplication.
Consider creating a helper method:
async getCellButton(buttonType, categoryName) {
const buttonSelector = buttonType === 'budget'
? `Open budget menu for ${categoryName} category`
: `Show transactions for ${categoryName} category`;
let button = this.budgetTable.getByRole('button', { name: buttonSelector });
if (await button.isVisible()) {
return button;
}
await this.toggleVisibleColumns();
button = this.budgetTable.getByRole('button', { name: buttonSelector });
if (await button.isVisible()) {
return button;
}
throw new Error(
`${buttonType.charAt(0).toUpperCase() + buttonType.slice(1)} button for category ${categoryName} could not be located on the page`
);
}
async getBudgetCellButton(categoryName) {
return this.getCellButton('budget', categoryName);
}
async getSpentCellButton(categoryName) {
return this.getCellButton('spent', categoryName);
}This refactoring reduces code duplication and improves maintainability.
| async openBalanceMenu(categoryName) { | ||
| const balanceButton = this.budgetTable.getByRole('button', { | ||
| name: `Open balance menu for ${categoryName} category`, | ||
| }); | ||
| await balanceButton.click(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to openBalanceMenu
The openBalanceMenu method currently doesn't check if the balance button exists or is visible before clicking, which could lead to potential exceptions.
Consider adding error handling:
async openBalanceMenu(categoryName) {
const balanceButton = this.budgetTable.getByRole('button', {
name: `Open balance menu for ${categoryName} category`,
});
if (await balanceButton.isVisible()) {
await balanceButton.click();
} else {
throw new Error(`Balance button for category ${categoryName} not found or not visible`);
}
}This change makes the method more robust by handling cases where the button might not be available.
| async getEnvelopeBudgetSummaryButton({ throwIfNotFound = true } = {}) { | ||
| if (await this.toBudgetButton.isVisible()) { | ||
| return this.toBudgetButton; | ||
| } | ||
|
|
||
| if (await this.overbudgetedButton.isVisible()) { | ||
| return this.overbudgetedButton; | ||
| } | ||
|
|
||
| if (!throwIfNotFound) { | ||
| return null; | ||
| } | ||
|
|
||
| throw new Error( | ||
| 'To Budget/Overbudgeted button could not be located on the page', | ||
| ); | ||
| } | ||
|
|
||
| async openEnvelopeBudgetSummaryMenu() { | ||
| const budgetSummaryButton = await this.getEnvelopeBudgetSummaryButton(); | ||
| await budgetSummaryButton.click(); | ||
| } | ||
|
|
||
| async getTrackingBudgetSummaryButton({ throwIfNotFound = true } = {}) { | ||
| if (await this.savedButton.isVisible()) { | ||
| return this.savedButton; | ||
| } | ||
|
|
||
| if (await this.projectedSavingsButton.isVisible()) { | ||
| return this.projectedSavingsButton; | ||
| } | ||
|
|
||
| if (await this.overspentButton.isVisible()) { | ||
| return this.overspentButton; | ||
| } | ||
|
|
||
| if (!throwIfNotFound) { | ||
| return null; | ||
| } | ||
|
|
||
| throw new Error( | ||
| 'Saved/Projected Savings/Overspent button could not be located on the page', | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error messages in budget summary button methods
The getEnvelopeBudgetSummaryButton and getTrackingBudgetSummaryButton methods are well-structured, but their error messages could be more descriptive.
Consider updating the error messages to include all possible button names:
async getEnvelopeBudgetSummaryButton({ throwIfNotFound = true } = {}) {
// ... existing code ...
if (!throwIfNotFound) {
return null;
}
throw new Error(
'Neither "To Budget" nor "Overbudgeted" button could be located on the page'
);
}
async getTrackingBudgetSummaryButton({ throwIfNotFound = true } = {}) {
// ... existing code ...
if (!throwIfNotFound) {
return null;
}
throw new Error(
'None of "Saved", "Projected Savings", or "Overspent" buttons could be located on the page'
);
}These more specific error messages will make debugging easier if issues arise.
| constructor(page) { | ||
| this.page = page; | ||
|
|
||
| this.categoryNames = page | ||
| this.initializePageHeaderLocators(page); | ||
| this.initializeBudgetTableLocators(page); | ||
|
|
||
| this.budgetType = | ||
| this.getEnvelopeBudgetSummaryButton({ | ||
| throwIfNotFound: false, | ||
| }) !== null | ||
| ? 'Envelope' | ||
| : 'Tracking'; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting budget type determination to a separate method
The constructor is well-organized with separate initialization methods. However, the budget type determination logic could be moved to a dedicated method for improved readability and maintainability.
Consider refactoring the constructor as follows:
constructor(page) {
this.page = page;
this.initializePageHeaderLocators(page);
this.initializeBudgetTableLocators(page);
- this.budgetType =
- this.getEnvelopeBudgetSummaryButton({
- throwIfNotFound: false,
- }) !== null
- ? 'Envelope'
- : 'Tracking';
+ this.budgetType = this.determineBudgetType();
}
+determineBudgetType() {
+ return this.getEnvelopeBudgetSummaryButton({
+ throwIfNotFound: false,
+ }) !== null
+ ? 'Envelope'
+ : 'Tracking';
+}This change improves the separation of concerns and makes the constructor more concise.
📝 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.
| constructor(page) { | |
| this.page = page; | |
| this.categoryNames = page | |
| this.initializePageHeaderLocators(page); | |
| this.initializeBudgetTableLocators(page); | |
| this.budgetType = | |
| this.getEnvelopeBudgetSummaryButton({ | |
| throwIfNotFound: false, | |
| }) !== null | |
| ? 'Envelope' | |
| : 'Tracking'; | |
| } | |
| constructor(page) { | |
| this.page = page; | |
| this.initializePageHeaderLocators(page); | |
| this.initializeBudgetTableLocators(page); | |
| this.budgetType = this.determineBudgetType(); | |
| } | |
| determineBudgetType() { | |
| return this.getEnvelopeBudgetSummaryButton({ | |
| throwIfNotFound: false, | |
| }) !== null | |
| ? 'Envelope' | |
| : 'Tracking'; | |
| } |
There was a problem hiding this comment.
Overall really good changes! Just a couple of small comments from me.
Apart from that, two more things:
- can you look through the bot comments? Some of them are quite good ideas
- the VRT test is failing in this PR - need to make sure it's not failing AND not flaking (we should try not to introduce new flakiness)
| @@ -418,6 +419,7 @@ export function ModalTitle({ | |||
| {shrinkOnOverflow ? ( | |||
| <AutoTextSize | |||
| as={Text} | |||
| aria-label="Modal title" | |||
There was a problem hiding this comment.
🔨 warning: we should not be setting aria labels to elements that already have perfectly legible titles (in this case title). Instead you might consider using roles: aria-role="heading" and then lookup via getByRole('heading')
| @@ -433,6 +435,7 @@ export function ModalTitle({ | |||
| </AutoTextSize> | |||
| ) : ( | |||
| <TextOneLine | |||
| aria-label="Modal title" | |||
| @@ -1968,12 +1986,15 @@ function MonthSelector({ | |||
| onPress={() => { | |||
| onOpenMonthMenu?.(month); | |||
| }} | |||
| aria-label="Selected budget month" | |||
There was a problem hiding this comment.
🔨 warning: this one is problematic too.
actual-github-bot
bot
added
⚠️ Changes requested
🔍 Ready for review
and removed
🔍 Ready for review
⚠️ Changes requested
labels
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- packages/desktop-client/e2e/page-models/mobile-budget-page.js (1 hunks)
🔇 Additional comments (14)
packages/desktop-client/e2e/page-models/mobile-budget-page.js (14)
7-10: Improved constructor organization and budget type determination.The constructor has been refactored to use separate initialization methods for page header and budget table locators. The budget type determination has been moved to a separate method, improving code organization and readability.
13-19: New method for determining budget type.The
determineBudgetTypemethod provides a clear way to distinguish between 'Envelope' and 'Tracking' budget types based on the presence of specific buttons. This improves code readability and maintainability.
21-65: Comprehensive initialization of budget table locators.The
initializeBudgetTableLocatorsmethod provides a clear and organized way to set up all the necessary locators for the budget table. This includes buttons for different budget types, header buttons, and various row and name selectors.
67-81: Organized initialization of page header locators.The
initializePageHeaderLocatorsmethod sets up locators for the page header elements, including navigation buttons and the budget page menu button. This separation of concerns improves code organization.
83-85: New method for waiting for budget table.The
waitForBudgetTablemethod provides a simple way to ensure the budget table is loaded before proceeding with other actions. This can help prevent race conditions in tests.
87-101: Improved column visibility toggling with retry mechanism.The
toggleVisibleColumnsmethod now includes a retry mechanism with a maximum number of attempts. This makes the method more robust and helps prevent potential infinite loops.
103-105: New method for getting the selected month.The
getSelectedMonthmethod provides a simple way to retrieve the currently selected month, which can be useful for various test scenarios.
107-109: New method for opening the budget page menu.The
openBudgetPageMenumethod encapsulates the action of clicking the budget page menu button, improving code readability and reusability.
144-166: New generic method for getting cell buttons.The
getCellButtonmethod provides a generic way to get both budget and spent cell buttons. It includes error handling and attempts to toggle visible columns if the button is not initially visible. This reduces code duplication and improves maintainability.
168-174: Simplified methods for getting specific cell buttons.The
getBudgetCellButtonandgetSpentCellButtonmethods now use the genericgetCellButtonmethod, reducing code duplication and improving maintainability.
196-208: Improved error handling in openBalanceMenu method.The
openBalanceMenumethod now checks if the balance button is visible before clicking it and throws a descriptive error if it's not found or visible. This improves the robustness of the method.
222-238: Comprehensive method for getting envelope budget summary button.The
getEnvelopeBudgetSummaryButtonmethod checks for multiple possible buttons and includes an option to return null instead of throwing an error. This flexibility can be useful in different test scenarios.
245-265: Comprehensive method for getting tracking budget summary button.Similar to the envelope budget method,
getTrackingBudgetSummaryButtonchecks for multiple possible buttons and includes an option to return null instead of throwing an error. This consistency in approach is commendable.
267-269: New method for opening tracking budget summary menu.The
openTrackingBudgetSummaryMenumethod provides a simple way to open the tracking budget summary menu, improving code readability and reusability.
| async setBudget(categoryName, newAmount) { | ||
| const budgetedButton = await this.getBudgetCellButton(categoryName); | ||
| await budgetedButton.click(); | ||
|
|
||
| await this.page.keyboard.type(String(newAmount)); | ||
| await this.page.keyboard.press('Enter'); | ||
| } |
There was a problem hiding this comment.
Method for setting budget lacks input validation.
The setBudget method doesn't validate the newAmount parameter. Consider adding a check to ensure newAmount is a valid, non-negative number.
Suggested improvement:
async setBudget(categoryName, newAmount) {
if (typeof newAmount !== 'number' || isNaN(newAmount) || newAmount < 0) {
throw new Error('Invalid budget amount. Must be a non-negative number.');
}
const budgetedButton = await this.getBudgetCellButton(categoryName);
await budgetedButton.click();
await this.page.keyboard.type(String(newAmount));
await this.page.keyboard.press('Enter');
}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
packages/desktop-client/e2e/page-models/mobile-budget-page.js (5)
6-7: Consider using a more specific date format constant name.The constant
MONTH_HEADER_DATE_FORMATis well-defined, but its name could be more specific to its usage in this class.Consider renaming it to
BUDGET_MONTH_HEADER_DATE_FORMATto make its purpose clearer within the context of theMobileBudgetPageclass.
17-23: Consider adding error handling todetermineBudgetType().The
determineBudgetType()method assumes that if the envelope budget summary button is not found, it must be a tracking budget. This might not always be the case if there's an error in loading the page.Consider adding a fallback or error handling:
determineBudgetType() { const envelopeButton = this.getEnvelopeBudgetSummaryButton({ throwIfNotFound: false, }); const trackingButton = this.getTrackingBudgetSummaryButton({ throwIfNotFound: false, }); if (envelopeButton !== null) return 'Envelope'; if (trackingButton !== null) return 'Tracking'; throw new Error('Unable to determine budget type'); }
71-88: Consider using a constant for the month format.In the
initializePageHeaderLocatorsmethod, you're using theMONTH_HEADER_DATE_FORMATconstant, which is good. However, the format is repeated in the method call.Consider using the constant directly:
this.selectedBudgetMonthButton = this.heading.getByRole('button', { name: monthUtils.format( await this.getSelectedMonth(), this.MONTH_HEADER_DATE_FORMAT, ), });
94-108: Consider adding a return value totoggleVisibleColumns.The
toggleVisibleColumnsmethod could be more informative by returning which button was clicked.Consider modifying the method to return the clicked button:
async toggleVisibleColumns(maxAttempts = 3) { for (let i = 0; i < maxAttempts; i++) { if (await this.budgetedHeaderButton.isVisible()) { await this.budgetedHeaderButton.click(); return 'Budgeted'; } if (await this.spentHeaderButton.isVisible()) { await this.spentHeaderButton.click(); return 'Spent'; } await this.page.waitForTimeout(1000); } throw new Error('Budgeted/Spent columns could not be located on the page'); }
151-173: Consider simplifying thegetCellButtonmethod.The
getCellButtonmethod is well-structured but could be simplified slightly.Consider this refactoring:
async getCellButton(buttonType, categoryName) { const buttonSelector = `${buttonType === 'Budgeted' ? 'Open budget menu for' : 'Show transactions for'} ${categoryName} category`; for (let attempt = 0; attempt < 2; attempt++) { const button = this.budgetTable.getByRole('button', { name: buttonSelector }); if (await button.isVisible()) return button; if (attempt === 0) await this.toggleVisibleColumns(); } throw new Error(`${buttonType} button for category ${categoryName} could not be located on the page`); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- packages/desktop-client/e2e/budget.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/page-models/mobile-budget-page.js (1 hunks)
- packages/desktop-client/src/components/common/Modal.tsx (2 hunks)
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx (16 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/desktop-client/e2e/budget.mobile.test.js
- packages/desktop-client/src/components/common/Modal.tsx
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx
🔇 Additional comments (10)
packages/desktop-client/e2e/page-models/mobile-budget-page.js (10)
1-4: LGTM: Imports look good.The imports are appropriate for the functionality of this class. The
monthUtilsimport will be useful for date formatting, and theMobileAccountPageimport suggests proper separation of concerns between different page models.
11-15: LGTM: Constructor refactoring improves readability.The constructor has been refactored to use separate initialization methods, which improves readability and maintainability. The
determineBudgetType()call at the end is a good way to set thebudgetTypeproperty.
25-69: LGTM: Well-structured initialization of budget table locators.The
initializeBudgetTableLocatorsmethod is well-organized and clearly defines all the necessary locators for interacting with the budget table. The use of descriptive variable names makes the code easy to understand.
90-92: LGTM: Simple and clear wait method.The
waitForBudgetTablemethod is straightforward and serves its purpose well.
110-112: LGTM: Efficient method to get the selected month.The
getSelectedMonthmethod is concise and uses the appropriate attribute to retrieve the selected month.
114-116: LGTM: Well-structured methods for interacting with category groups and categories.These methods (
openBudgetPageMenu,getCategoryGroupNameForRow,getCategoryGroupButton,openCategoryGroupMenu,getCategoryNameForRow,getCategoryButton,openCategoryMenu) are concise and follow a consistent pattern, which is good for maintainability.Also applies to: 118-120, 122-127, 129-133, 135-137, 139-144, 146-149
175-181: LGTM: Well-implemented methods for interacting with budget cells.The methods
getBudgetCellButton,getSpentCellButton,openBudgetMenu,setBudget, andopenSpentPageare concise and make good use of thegetCellButtonmethod.Also applies to: 183-186, 188-194, 196-201
203-215: LGTM: Improved error handling inopenBalanceMenu.The
openBalanceMenumethod now includes proper error handling when the balance button is not found or not visible.
217-227: LGTM: Simple and clear navigation methods.The methods
goToPreviousMonth,openMonthMenu, andgoToNextMonthare straightforward and serve their purpose well.
229-245: LGTM: Well-implemented budget summary button methods.The methods
getEnvelopeBudgetSummaryButton,openEnvelopeBudgetSummaryMenu,getTrackingBudgetSummaryButton, andopenTrackingBudgetSummaryMenuare well-structured and include proper error handling.Also applies to: 247-250, 252-272, 274-276
There was a problem hiding this comment.
Code LGTM, but the budget.mobile.test seems to be flaky: https://github.com/actualbudget/actual/actions/runs/11098819972/job/30832313768
joel-jeremy
force-pushed
the
mobile-vrts-1
branch
from
ae622a3 to
5617f56
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
packages/desktop-client/e2e/page-models/mobile-budget-page.js (2)
130-144: Add input validation togetCategoryNameForRowand approve other methods.
- The
getCategoryNameForRowmethod should include input validation similar to the suggestion forgetCategoryGroupNameForRow:async getCategoryNameForRow(idx) { const count = await this.categoryNames.count(); if (idx < 0 || idx >= count) { throw new Error(`Invalid index: ${idx}. Valid range is 0 to ${count - 1}.`); } return this.categoryNames.nth(idx).textContent(); }
- The
getCategoryButtonandopenCategoryMenumethods are well-implemented and consistent with their category group counterparts.
178-189: Add input validation tosetBudgetand approveopenBudgetMenu.
The
openBudgetMenumethod is well-implemented and straightforward.The
setBudgetmethod should include input validation for thenewAmountparameter:async setBudget(categoryName, newAmount) { if (typeof newAmount !== 'number' || isNaN(newAmount) || newAmount < 0) { throw new Error('Invalid budget amount. Must be a non-negative number.'); } const budgetedButton = await this.getBudgetCellButton(categoryName); await budgetedButton.click(); await this.page.keyboard.type(String(newAmount)); await this.page.keyboard.press('Enter'); }This will prevent potential errors when an invalid amount is provided.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (141)
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-4-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-5-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-6-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/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-the-accounts-page-and-asserts-on-balances-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-the-accounts-page-and-asserts-on-balances-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.mobile.test.js-snapshots/Mobile-Accounts-opens-the-accounts-page-and-asserts-on-balances-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/accounts.test.js-snapshots/Accounts-closes-an-account-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--14404-in-the-page-header-opens-the-month-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--321fd-ed-amount-opens-the-budget-summary-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--4bb70-ed-amount-opens-the-budget-summary-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--589a6-the-page-header-shows-the-next-month-s-budget-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--6ab37-roup-name-opens-the-category-group-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--6dbdb-page-header-shows-the-previous-month-s-budget-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--7bd8f-the-page-header-shows-the-next-month-s-budget-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--884ac-the-page-header-shows-the-next-month-s-budget-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--94a79-roup-name-opens-the-category-group-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--94a85-ed-amount-opens-the-budget-summary-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--96ebb-page-header-shows-the-previous-month-s-budget-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--9e6aa-in-the-page-header-opens-the-budget-page-menu-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--bbde3-roup-name-opens-the-category-group-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--bed18-in-the-page-header-opens-the-month-menu-modal-1-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--ceb3a-in-the-page-header-opens-the-month-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--d270d-in-the-page-header-opens-the-budget-page-menu-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking--d7184-page-header-shows-the-previous-month-s-budget-2-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--fdd57-in-the-page-header-opens-the-budget-page-menu-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-Envelope-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-category-name-opens-the-category-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-category-name-opens-the-category-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-checks-that-clicking-the-category-name-opens-the-category-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-loads-the-budget-page-with-budgeted-amounts-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-loads-the-budget-page-with-budgeted-amounts-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-loads-the-budget-page-with-budgeted-amounts-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-updates-the-budgeted-amount-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-updates-the-budgeted-amount-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Envelope-updates-the-budgeted-amount-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--0ba04-nt-amount-opens-the-budget-summary-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--0dfe7-page-header-shows-the-previous-month-s-budget-1-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--1ce6d-nt-amount-opens-the-budget-summary-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--42062-in-the-page-header-opens-the-month-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--49fb6-in-the-page-header-opens-the-month-menu-modal-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/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--5f098-roup-name-opens-the-category-group-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--7c353-the-page-header-shows-the-next-month-s-budget-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--929be-roup-name-opens-the-category-group-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--a3783-in-the-page-header-opens-the-budget-page-menu-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--a8b5e-in-the-page-header-opens-the-budget-page-menu-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--b1562-in-the-page-header-opens-the-month-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--cfb69-page-header-shows-the-previous-month-s-budget-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--d5af6-the-page-header-shows-the-next-month-s-budget-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--dc927-roup-name-opens-the-category-group-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--f2198-the-page-header-shows-the-next-month-s-budget-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--f224f-nt-amount-opens-the-budget-summary-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--f7fa3-page-header-shows-the-previous-month-s-budget-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--f8a19-in-the-page-header-opens-the-budget-page-menu-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-button-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-balance-cell-opens-the-balance-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-budgeted-cell-opens-the-budget-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-category-name-opens-the-category-menu-modal-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-category-name-opens-the-category-menu-modal-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking-the-category-name-opens-the-category-menu-modal-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-loads-the-budget-page-with-budgeted-amounts-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-loads-the-budget-page-with-budgeted-amounts-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-loads-the-budget-page-with-budgeted-amounts-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-updates-the-budgeted-amount-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-updates-the-budgeted-amount-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-updates-the-budgeted-amount-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.test.js-snapshots/Budget-renders-the-summary-information-available-funds-overspent-budgeted-and-for-next-month-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.test.js-snapshots/Budget-renders-the-summary-information-available-funds-overspent-budgeted-and-for-next-month-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-checks-that-settings-page-can-be-opened-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-creates-a-transaction-from-accounts-id-page-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-creates-a-transaction-via-footer-button-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-loads-the-budget-page-with-budgeted-amounts-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-loads-the-budget-page-with-budgeted-amounts-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-loads-the-budget-page-with-budgeted-amounts-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-opens-the-accounts-page-and-asserts-on-balances-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-opens-the-accounts-page-and-asserts-on-balances-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/mobile.test.js-snapshots/Mobile-opens-the-accounts-page-and-asserts-on-balances-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-summary-button-shows-the-summary-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/rules.test.js-snapshots/Rules-checks-the-page-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/rules.test.js-snapshots/Rules-creates-a-rule-and-makes-sure-it-is-applied-when-creating-a-transaction-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/rules.test.js-snapshots/Rules-creates-a-split-transaction-rule-and-makes-sure-it-is-applied-when-creating-a-transaction-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/schedules.test.js-snapshots/Schedules-creates-a-new-schedule-posts-the-transaction-and-later-completes-it-7-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/schedules.test.js-snapshots/Schedules-creates-a-new-schedule-posts-the-transaction-and-later-completes-it-8-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-6-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-from-accounts-id-page-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.mobile.test.js-snapshots/Mobile-Transactions-creates-a-transaction-via-footer-button-6-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!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-test-transaction-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-creates-a-transfer-test-transaction-6-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-filters-transactions-by-date-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-filters-transactions-by-date-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/transactions.test.js-snapshots/Transactions-filters-transactions-by-date-6-chromium-linux.pngis excluded by!**/*.pngupcoming-release-notes/3506.mdis excluded by!**/*.mdyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
- packages/desktop-client/e2e/accounts.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/budget.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/page-models/mobile-account-page.js (1 hunks)
- packages/desktop-client/e2e/page-models/mobile-budget-page.js (1 hunks)
- packages/desktop-client/e2e/page-models/settings-page.js (1 hunks)
- packages/desktop-client/e2e/settings.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/transactions.mobile.test.js (1 hunks)
- packages/desktop-client/playwright.config.js (1 hunks)
- packages/desktop-client/src/components/common/Modal.tsx (2 hunks)
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx (16 hunks)
- packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (2 hunks)
- packages/desktop-client/src/components/modals/EnvelopeBudgetMenuModal.tsx (1 hunks)
- packages/desktop-client/src/components/modals/TrackingBudgetMenuModal.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/desktop-client/e2e/accounts.mobile.test.js
- packages/desktop-client/e2e/budget.mobile.test.js
- packages/desktop-client/e2e/page-models/mobile-account-page.js
- packages/desktop-client/e2e/page-models/settings-page.js
- packages/desktop-client/e2e/settings.mobile.test.js
- packages/desktop-client/e2e/transactions.mobile.test.js
- packages/desktop-client/playwright.config.js
- packages/desktop-client/src/components/common/Modal.tsx
- packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx
- packages/desktop-client/src/components/modals/EnvelopeBudgetMenuModal.tsx
- packages/desktop-client/src/components/modals/TrackingBudgetMenuModal.tsx
🔇 Additional comments (22)
packages/desktop-client/e2e/page-models/mobile-budget-page.js (14)
1-5: LGTM! Good use of imports and constants.The import statement is correct, and the
MONTH_HEADER_DATE_FORMATconstant is well-defined with a clear and descriptive name.
23-67: LGTM! Well-structured initialization method.The
initializeBudgetTableLocatorsmethod is comprehensive and follows good practices:
- Uses descriptive names for locators, improving readability.
- Utilizes
getByTestIdandgetByRolefor robust element selection.- Clearly separates different types of buttons (envelope budget, tracking budget, etc.).
This structure will make maintenance and updates easier in the future.
69-81: LGTM! Clear and concise header locator initialization.The
initializePageHeaderLocatorsmethod is well-implemented:
- Uses descriptive names for locators, improving readability.
- Appropriately uses a mix of
getByRoleandlocatormethods for different elements.- Clearly organizes header-related elements.
This structure enhances maintainability and readability of the code.
83-85: LGTM! Good use of asynchronous wait.The
waitForBudgetTablemethod is a good practice:
- Uses
waitForto ensure the budget table is available before proceeding.- Enhances the reliability of subsequent operations that depend on the budget table's presence.
103-107: LGTM! Well-implemented asynchronous getter.The
getSelectedMonthmethod is correctly implemented:
- Uses
awaitfor the asynchronousgetAttributecall.- Focused on a single responsibility, enhancing maintainability.
109-111: LGTM! Simple and effective menu opener.The
openBudgetPageMenumethod is correctly implemented:
- Uses
awaitfor the asynchronousclickoperation.- Focused on a single responsibility, making it easy to understand and maintain.
117-128: LGTM! Well-structured category group interaction methods.These methods are well-implemented:
getCategoryGroupButtonuses precise matching withexact: true.openCategoryGroupMenucorrectly usesawaitfor asynchronous operations.- The methods follow a clear and consistent pattern, enhancing readability and maintainability.
146-168: LGTM! Robust and flexible cell button retrieval.The
getCellButtonmethod is well-implemented:
- Includes good error handling and retry logic.
- Uses descriptive error messages.
- Flexibly handles both budgeted and spent buttons.
- Attempts to toggle visible columns if the button is not initially found.
This implementation enhances the reliability of interacting with cell buttons.
170-176: LGTM! Good use of code reuse and single responsibility.The
getBudgetCellButtonandgetSpentCellButtonmethods are well-implemented:
- They are simple and focused, enhancing maintainability.
- They leverage the more complex
getCellButtonmethod, promoting code reuse.- Each method has a single responsibility, aligning with SOLID principles.
191-196: LGTM! Good implementation of page object model pattern.The
openSpentPagemethod is well-implemented:
- Correctly uses
awaitfor asynchronous operations.- Follows the page object model pattern by returning a new
MobileAccountPageinstance.- Enhances the fluent interface of the test framework.
198-210: LGTM! Robust implementation with good error handling.The
openBalanceMenumethod is well-implemented:
- Includes good error handling if the balance button is not visible.
- Uses a descriptive error message, aiding in debugging.
- Correctly uses
awaitfor asynchronous operations.This implementation enhances the reliability of interacting with the balance menu.
212-222: LGTM! Clear and concise month navigation methods.These methods (
goToPreviousMonth,openMonthMenu,goToNextMonth) are well-implemented:
- Simple and focused, enhancing maintainability.
- Correctly use
awaitfor asynchronous click operations.- Follow the single responsibility principle, making the code easy to understand and maintain.
224-245: LGTM! Well-structured envelope budget summary interaction.These methods are well-implemented:
getEnvelopeBudgetSummaryButtonincludes good error handling and flexibility with thethrowIfNotFoundoption.openEnvelopeBudgetSummaryMenucorrectly uses the getter method to interact with the button.- Both methods properly use
awaitfor asynchronous operations.- The error message is descriptive and helpful for debugging.
This implementation provides a robust way to interact with envelope budget summary buttons.
247-271: LGTM! Consistent and well-structured tracking budget summary interaction.These methods maintain consistency with their envelope budget counterparts:
getTrackingBudgetSummaryButtonincludes good error handling and flexibility with thethrowIfNotFoundoption.openTrackingBudgetSummaryMenucorrectly uses the getter method to interact with the button.- Both methods properly use
awaitfor asynchronous operations.- The error message is descriptive and helpful for debugging.
This implementation provides a robust and consistent way to interact with tracking budget summary buttons, aligning well with the envelope budget methods.
packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx (8)
287-292: Excellent accessibility improvement!Adding the
aria-labelto theCellValuecomponent enhances the experience for users relying on screen readers by providing context about the budgeted amount for each category.
308-308: Great accessibility enhancement!The addition of the
aria-labelto theButtoncomponent improves the user experience for those using screen readers by clearly describing the action of opening the budget menu for a specific category.
533-533: Improved test identifier specificityUpdating the
data-testidfrom "row" to "category-row" enhances the clarity and specificity of the test selector. This change will lead to more robust and maintainable tests by reducing the likelihood of selector conflicts.
609-613: Excellent accessibility enhancement!The addition of the
aria-labelto theCellValuecomponent for the spent amount improves accessibility. This change provides important context for screen reader users, helping them understand the purpose of the displayed value.
621-621: Great accessibility improvement!Adding the
aria-labelto theButtoncomponent enhances the experience for screen reader users by clearly describing the action of showing transactions for a specific category. This change makes the interface more navigable and understandable for all users.
652-652: Excellent accessibility enhancement!The addition of the
aria-labelto theBalanceWithCarryovercomponent improves the accessibility of the budget table. This change provides important context for screen reader users, helping them understand that the value represents the balance for a specific category.
685-685: Great accessibility improvement!The addition of the
aria-labelto theButtoncomponent enhances the user experience for those using screen readers. It clearly describes the action of opening the balance menu for a specific category, making the interface more navigable and understandable.
791-791: Improved testability and accessibilityGreat job on updating and adding various
data-testidattributes throughout the component. These changes will lead to more robust and maintainable tests by providing clear and specific selectors for different elements.The addition of
aria-labelattributes to the previous and next month navigation buttons enhances accessibility for screen reader users, clearly describing the purpose of these controls.These improvements contribute to a more testable and accessible budget table component.
Also applies to: 847-847, 987-987, 1041-1041, 1147-1147, 1215-1219, 1659-1659, 1745-1745, 1967-1967, 1989-1989, 1996-1996
| this.initializePageHeaderLocators(page); | ||
| this.initializeBudgetTableLocators(page); | ||
|
|
||
| this.budgetType = this.determineBudgetType(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider refactoring the constructor to handle async operations.
The current implementation may lead to race conditions or unexpected behavior due to calling potentially asynchronous methods without proper handling. Consider the following changes:
- Move the async initializations to an
async init()method. - Make
determineBudgetTypean async method if it contains async operations.
Example refactor:
constructor(page) {
this.page = page;
}
async init() {
await this.initializePageHeaderLocators(this.page);
await this.initializeBudgetTableLocators(this.page);
this.budgetType = await this.determineBudgetType();
}Then, when creating an instance:
const mobileBudgetPage = new MobileBudgetPage(page);
await mobileBudgetPage.init();This approach ensures that all asynchronous operations are properly handled and completed before the object is fully initialized.
| determineBudgetType() { | ||
| return this.getEnvelopeBudgetSummaryButton({ | ||
| throwIfNotFound: false, | ||
| }) !== null | ||
| ? 'Envelope' | ||
| : 'Tracking'; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make determineBudgetType an async method.
The determineBudgetType method is calling getEnvelopeBudgetSummaryButton, which appears to be an asynchronous method. To ensure proper handling of asynchronous operations, consider the following change:
async determineBudgetType() {
return (await this.getEnvelopeBudgetSummaryButton({
throwIfNotFound: false,
})) !== null
? 'Envelope'
: 'Tracking';
}This change ensures that the Promise returned by getEnvelopeBudgetSummaryButton is properly resolved before determining the budget type.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (5)
packages/desktop-client/e2e/page-models/mobile-budget-page.js (5)
4-4: Consider using a more specific date format string.The
MONTH_HEADER_DATE_FORMATconstant uses a non-standard date format string. It might be better to use a widely recognized format string or a library like date-fns for better maintainability.Consider using a standard format string or a date formatting library:
- MONTH_HEADER_DATE_FORMAT = 'MMMM 'yy'; + MONTH_HEADER_DATE_FORMAT = 'MMMM yyyy';
83-85: Consider adding a timeout towaitForBudgetTableThe
waitForBudgetTablemethod waits indefinitely for the budget table to appear. Consider adding a timeout to prevent the test from hanging if the table never appears.async waitForBudgetTable(timeout = 30000) { await this.budgetTable.waitFor({ timeout }); }
103-107: EnsuregetSelectedMonthreturns a stringThe
getSelectedMonthmethod returns an attribute value, which might be null. Consider adding a fallback or error handling.async getSelectedMonth() { const month = await this.heading .locator('[data-month]') .getAttribute('data-month'); if (!month) { throw new Error('Selected month attribute not found'); } return month; }
113-115: Add input validation togetCategoryGroupNameForRowThe method doesn't validate the
idxparameter. Consider adding a check to ensureidxis within the valid range of category group rows.async getCategoryGroupNameForRow(idx) { const count = await this.categoryGroupNames.count(); if (idx < 0 || idx >= count) { throw new Error(`Invalid index: ${idx}. Valid range is 0 to ${count - 1}.`); } return this.categoryGroupNames.nth(idx).textContent(); }
130-132: Add input validation togetCategoryNameForRowSimilar to
getCategoryGroupNameForRow, this method should include input validation for theidxparameter.async getCategoryNameForRow(idx) { const count = await this.categoryNames.count(); if (idx < 0 || idx >= count) { throw new Error(`Invalid index: ${idx}. Valid range is 0 to ${count - 1}.`); } return this.categoryNames.nth(idx).textContent(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/desktop-client/e2e/budget.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/page-models/mobile-budget-page.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/desktop-client/e2e/budget.mobile.test.js
🔇 Additional comments (7)
packages/desktop-client/e2e/page-models/mobile-budget-page.js (7)
23-67: Well-structured initialization of budget table locatorsThe
#initializeBudgetTableLocatorsmethod is well-organized and comprehensive. It initializes various locators for different elements of the budget table, which will be useful for interacting with these elements in other methods.
69-81: Well-structured initialization of page header locatorsThe
#initializePageHeaderLocatorsmethod is well-organized and initializes locators for various elements in the page header.
87-101: Well-implementedtoggleVisibleColumnsmethodThe
toggleVisibleColumnsmethod is well-implemented with a good retry mechanism and error handling.
146-168: Well-implemented#getButtonForCellmethodThe
#getButtonForCellmethod is well-implemented with good error handling and retry logic for toggling visible columns.
212-226: Well-implemented#waitForNewMonthToLoadmethodThe
#waitForNewMonthToLoadmethod is well-implemented with good retry logic and error handling.
258-274: Well-implemented budget summary button methodsThe
#getButtonForEnvelopeBudgetSummaryand#getButtonForTrackingBudgetSummarymethods are well-implemented with good error handling and flexibility.Also applies to: 281-301
1-306: Overall well-structured and comprehensive implementation with room for improvementThe
MobileBudgetPageclass is well-organized and provides a comprehensive set of methods for interacting with a mobile budget page. The code is generally clean and follows good practices. However, there are a few areas where improvements can be made:
- Handling of asynchronous operations in the constructor.
- Input validation in several methods.
- Error handling and fallback mechanisms in some methods.
- Consistency in error messages.
Addressing these issues will further improve the robustness and reliability of this class.
| this.#initializePageHeaderLocators(page); | ||
| this.#initializeBudgetTableLocators(page); | ||
|
|
||
| this.budgetType = this.determineBudgetType(); |
There was a problem hiding this comment.
Potential issue with async operations in constructor
The constructor is calling #initializePageHeaderLocators and determineBudgetType, which appear to be asynchronous methods, without awaiting them. This could lead to race conditions or unexpected behavior.
Consider refactoring to handle these async operations correctly:
constructor(page) {
this.page = page;
this.#initializeBudgetTableLocators(page);
}
async init() {
await this.#initializePageHeaderLocators(this.page);
this.budgetType = await this.determineBudgetType();
}Then, when creating an instance:
const mobileBudgetPage = new MobileBudgetPage(page);
await mobileBudgetPage.init();| determineBudgetType() { | ||
| return this.#getButtonForEnvelopeBudgetSummary({ | ||
| throwIfNotFound: false, | ||
| }) !== null | ||
| ? 'Envelope' | ||
| : 'Tracking'; | ||
| } |
There was a problem hiding this comment.
Make determineBudgetType an async method
The determineBudgetType method is calling #getButtonForEnvelopeBudgetSummary, which appears to be an asynchronous method. To ensure proper handling of asynchronous operations, consider the following change:
async determineBudgetType() {
return (await this.#getButtonForEnvelopeBudgetSummary({
throwIfNotFound: false,
})) !== null
? 'Envelope'
: 'Tracking';
}| async setBudget(categoryName, newAmount) { | ||
| const budgetedButton = await this.#getButtonForBudgeted(categoryName); | ||
| await budgetedButton.click(); | ||
|
|
||
| await this.page.keyboard.type(String(newAmount)); | ||
| await this.page.keyboard.press('Enter'); | ||
| } |
There was a problem hiding this comment.
Add input validation to setBudget
The setBudget method should include input validation for the newAmount parameter to ensure it's a valid number.
async setBudget(categoryName, newAmount) {
if (typeof newAmount !== 'number' || isNaN(newAmount) || newAmount < 0) {
throw new Error('Invalid budget amount. Must be a non-negative number.');
}
const budgetedButton = await this.#getButtonForBudgeted(categoryName);
await budgetedButton.click();
await this.page.keyboard.type(String(newAmount));
await this.page.keyboard.press('Enter');
}| async goToPreviousMonth({ maxAttempts = 3 } = {}) { | ||
| const currentMonth = await this.getSelectedMonth(); | ||
|
|
||
| await this.previousMonthButton.click(); | ||
|
|
||
| return await this.#waitForNewMonthToLoad({ | ||
| currentMonth, | ||
| maxAttempts, | ||
| errorMessage: | ||
| 'Failed to navigate to the previous month after maximum attempts', | ||
| }); | ||
| } |
There was a problem hiding this comment.
Inconsistent error messages in goToPreviousMonth and goToNextMonth
The error message in goToNextMonth incorrectly mentions "previous month" instead of "next month".
Update the error message in goToNextMonth:
- 'Failed to navigate to the previous month after maximum attempts',
+ 'Failed to navigate to the next month after maximum attempts',Also applies to: 245-256
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
packages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--0dfe7-page-header-shows-the-previous-month-s-budget-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/budget.mobile.test.js-snapshots/Mobile-Budget-Tracking-checks-that-clicking--d5af6-the-page-header-shows-the-next-month-s-budget-1-chromium-linux.pngis excluded by!**/*.png
📒 Files selected for processing (2)
- packages/desktop-client/e2e/budget.mobile.test.js (1 hunks)
- packages/desktop-client/e2e/page-models/mobile-budget-page.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/desktop-client/e2e/budget.mobile.test.js
🔇 Additional comments (16)
packages/desktop-client/e2e/page-models/mobile-budget-page.js (16)
1-2: LGTM: Import statement and class definition.The import statement and class definition look good. The
MobileAccountPageimport suggests good separation of concerns between different page models.
4-12: LGTM: Constructor implementation.The constructor is well-structured, using private methods to initialize locators. This approach promotes clean code organization.
21-65: LGTM: Comprehensive locator initialization.The
#initializeBudgetTableLocatorsmethod is well-structured and thorough. It initializes locators for various UI elements, including budget summary buttons, table headers, and category rows. This approach will make element interactions more reliable and maintainable throughout the class.
67-79: LGTM: Clear page header locator initialization.The
#initializePageHeaderLocatorsmethod effectively sets up locators for the page header elements, including navigation buttons and the budget page menu. This organization enhances the readability and maintainability of the code.
81-83: LGTM: Simple and effective wait method.The
waitForBudgetTablemethod provides a clear way to ensure the budget table is loaded before proceeding with other actions. This is a good practice for improving test reliability.
107-109: LGTM: Simple menu opening method.The
openBudgetPageMenumethod is concise and clear in its purpose. It correctly uses the locator defined in the constructor to interact with the budget page menu button.
115-126: LGTM: Well-structured category group interaction methods.The
#getButtonForCategoryGroupandopenCategoryGroupMenumethods are well-implemented. The private method effectively encapsulates the button retrieval logic, while the public method provides a clear interface for opening the category group menu. This design promotes good separation of concerns and code reusability.
132-142: LGTM: Consistent category interaction methods.The
#getButtonForCategoryandopenCategoryMenumethods are well-implemented and consistent with their category group counterparts. This consistency in design makes the code more predictable and easier to maintain.
144-174: LGTM: Well-designed cell button retrieval methods.The
#getButtonForCell,getButtonForBudgeted, andgetButtonForSpentmethods are excellently implemented. The private method effectively handles the common logic for both budgeted and spent buttons, including visibility checks and column toggling. The public methods provide a clear interface for retrieving specific button types. This design promotes code reuse and maintainability.
176-179: LGTM: Clear methods for budget and spent interactions.The
openBudgetMenuandopenSpentPagemethods are well-implemented, providing clear ways to interact with budget and spent cells. TheopenSpentPagemethod's return of a newMobileAccountPageinstance is a good practice for page object model design.Also applies to: 189-194
196-208: LGTM: Robust balance menu interaction.The
openBalanceMenumethod is well-implemented. It includes a visibility check for the balance button and appropriate error handling if the button is not found or not visible. This approach enhances the reliability of the interaction with the balance menu.
210-224: LGTM: Robust month loading check.The
#waitForNewMonthToLoadmethod is well-implemented. It includes a retry mechanism with a configurable number of attempts and appropriate error handling. This approach enhances the reliability of month navigation operations.
226-237: LGTM: Well-structured month navigation methods.The
goToPreviousMonthandopenMonthMenumethods are well-implemented. They provide clear ways to navigate between months and open the month selection menu. The use of#waitForNewMonthToLoadingoToPreviousMonthensures reliable navigation.Also applies to: 239-241
256-277: LGTM: Robust envelope budget summary interaction.The
#getButtonForEnvelopeBudgetSummaryandopenEnvelopeBudgetSummaryMenumethods are well-implemented. The private method includes thorough visibility checks for different button types and appropriate error handling. The public method provides a clear interface for opening the envelope budget summary menu. This design ensures reliable interactions with envelope budget summary elements.
279-303: LGTM: Consistent and robust tracking budget summary interaction.The
#getButtonForTrackingBudgetSummaryandopenTrackingBudgetSummaryMenumethods are well-implemented and consistent with their envelope budget counterparts. The private method includes thorough visibility checks for different button types and appropriate error handling. The public method provides a clear interface for opening the tracking budget summary menu. This consistency in design across different budget types enhances the overall maintainability of the code.
243-254:⚠️ Potential issueFix error message in
goToNextMonth.The error message in
goToNextMonthincorrectly mentions "previous month" instead of "next month". Update the error message:- 'Failed to navigate to the previous month after maximum attempts', + 'Failed to navigate to the next month after maximum attempts',This change ensures consistency with the method's purpose.
Likely invalid or redundant comment.
| async determineBudgetType() { | ||
| return (await this.#getButtonForEnvelopeBudgetSummary({ | ||
| throwIfNotFound: false, | ||
| })) !== null | ||
| ? 'Envelope' | ||
| : 'Tracking'; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure proper async handling in determineBudgetType.
The determineBudgetType method is using an await inside a return statement, which is good. However, to improve readability and make the async nature more explicit, consider refactoring it slightly:
async determineBudgetType() {
const envelopeButton = await this.#getButtonForEnvelopeBudgetSummary({
throwIfNotFound: false,
});
return envelopeButton !== null ? 'Envelope' : 'Tracking';
}This change makes the async operation more evident and easier to understand at a glance.
| async toggleVisibleColumns(maxAttempts = 3) { | ||
| for (let i = 0; i < maxAttempts; i++) { | ||
| if (await this.budgetedHeaderButton.isVisible()) { | ||
| await this.budgetedHeaderButton.click(); | ||
| return; | ||
| } | ||
| if (await this.spentHeaderButton.isVisible()) { | ||
| await this.spentHeaderButton.click(); | ||
| return; | ||
| } | ||
| await this.page.waitForTimeout(1000); | ||
| } | ||
|
|
||
| throw new Error('Budgeted/Spent columns could not be located on the page'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider parameterizing the wait time in toggleVisibleColumns.
The toggleVisibleColumns method is well-implemented with a good retry mechanism. To improve flexibility, consider parameterizing the wait time:
async toggleVisibleColumns(maxAttempts = 3, waitTime = 1000) {
for (let i = 0; i < maxAttempts; i++) {
// ... existing code ...
await this.page.waitForTimeout(waitTime);
}
// ... existing error handling ...
}This change allows for easier adjustment of the wait time if needed in different scenarios or environments.
| async getSelectedMonth() { | ||
| return await this.heading | ||
| .locator('[data-month]') | ||
| .getAttribute('data-month'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a single line for getSelectedMonth.
The getSelectedMonth method can be simplified for better readability:
async getSelectedMonth() {
return this.heading.locator('[data-month]').getAttribute('data-month');
}This change maintains the method's functionality while making it more concise.
| async getCategoryGroupNameForRow(idx) { | ||
| return this.categoryGroupNames.nth(idx).textContent(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation to getCategoryGroupNameForRow.
To prevent potential errors, consider adding input validation:
async getCategoryGroupNameForRow(idx) {
const count = await this.categoryGroupNames.count();
if (idx < 0 || idx >= count) {
throw new Error(`Invalid index: ${idx}. Valid range is 0 to ${count - 1}.`);
}
return this.categoryGroupNames.nth(idx).textContent();
}This change ensures that the provided index is within the valid range of category group rows.
| async getCategoryNameForRow(idx) { | ||
| return this.categoryNames.nth(idx).textContent(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation to getCategoryNameForRow.
Similar to the suggestion for getCategoryGroupNameForRow, consider adding input validation:
async getCategoryNameForRow(idx) {
const count = await this.categoryNames.count();
if (idx < 0 || idx >= count) {
throw new Error(`Invalid index: ${idx}. Valid range is 0 to ${count - 1}.`);
}
return this.categoryNames.nth(idx).textContent();
}This change ensures that the provided index is within the valid range of category rows.
| async setBudget(categoryName, newAmount) { | ||
| const budgetedButton = await this.getButtonForBudgeted(categoryName); | ||
| await budgetedButton.click(); | ||
|
|
||
| await this.page.keyboard.type(String(newAmount)); | ||
| await this.page.keyboard.press('Enter'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation to setBudget.
Consider adding input validation for the newAmount parameter:
async setBudget(categoryName, newAmount) {
if (typeof newAmount !== 'number' || isNaN(newAmount) || newAmount < 0) {
throw new Error('Invalid budget amount. Must be a non-negative number.');
}
const budgetedButton = await this.getButtonForBudgeted(categoryName);
await budgetedButton.click();
await this.page.keyboard.type(String(newAmount));
await this.page.keyboard.press('Enter');
}This change ensures that only valid budget amounts are set, preventing potential errors.
Adding Visual Regression Tests for the mobile view. I split the current
mobile.test.jsto multiple files. One for each mobile page i.e.budget.mobile.test.js,accounts.mobile.test.js,transactions.mobile.test.js, etcThis PR mainly focuses on the mobile budget page. I have added some tests for both Envelope and Tracking budget types.