🌍 improve translation strings - part 2 #4154
Conversation
actual-github-bot
bot
changed the title
|
/update-vrt |
✅ 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
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged No assets were unchanged |
matt-fidd
force-pushed
the
translations-2
branch
2 times, most recently
from
bb884f7 to
f80d091
Compare
|
/update-vrt |
There was a problem hiding this comment.
@jfdoming interesting...it makes sense but I hadn't thought of it.
This is a new string that's not in weblate yet, and so pulls in as nothing when the translations are fetched.
There was a problem hiding this comment.
Yeah ok, you've got me convinced to use cimode 😅 will take a look as soon as I can
matt-fidd
force-pushed
the
translations-2
branch
from
1373116 to
397d0be
Compare
matt-fidd
changed the title
WalkthroughThis pull request encompasses a comprehensive set of modifications across multiple files in the desktop client and core packages, primarily focusing on internationalization (i18n), text formatting, and error message improvements. The changes span numerous components, with a consistent theme of enhancing translation capabilities, improving text capitalization, and refining user-facing messages. Key areas of modification include integrating the Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (6)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx (1)
104-106: LGTM! Consider one minor consistency improvement.The error message changes appropriately specify "budget file" instead of just "budget", which aligns with the PR's goal of improving translation string clarity and consistency.
For complete consistency, consider updating the error message in the notification dispatch as well:
dispatch( addNotification({ type: 'error', - message: t('Failed to duplicate budget file.'), + message: t('Failed to duplicate budget file'), }), );This removes the trailing period to match the format of the other error messages.
packages/desktop-client/src/components/schedules/ScheduleDetails.tsx (1)
974-976: Consider standardizing error message handling across the component.While this specific error message is well-formatted for translation, other error messages in the component (e.g., schedule saving error with URL) could benefit from the same named interpolation pattern for consistency.
Example of consistent error message formatting:
- error: t('An error occurred while saving. Please visit https://actualbudget.org/contact/ for support.'), + error: t('An error occurred while saving. Please visit {{supportUrl}} for support.', { supportUrl: 'https://actualbudget.org/contact/' }),packages/desktop-client/src/components/reports/graphs/SpendingGraph.tsx (1)
81-89: Good use of Trans component, with minor optimization opportunitiesThe transition to using the
Transcomponent with proper interpolation is a good improvement for translation handling. However, there are a couple of potential optimizations:
- Consider extracting the '28+' translation outside the interpolation:
+ const dayPlus = t('28+'); <Trans> Day:{' '} {{ dayOfMonth: Number(payload[0].payload.day) >= 28 - ? t('28+') + ? dayPlus : payload[0].payload.day, }} </Trans>
- Consider extracting the magic number 28 into a named constant for better maintainability:
+ const LAST_DAY_THRESHOLD = 28; <Trans> Day:{' '} {{ dayOfMonth: - Number(payload[0].payload.day) >= 28 + Number(payload[0].payload.day) >= LAST_DAY_THRESHOLD ? dayPlus : payload[0].payload.day, }} </Trans>packages/desktop-client/src/components/modals/manager/FilesSettingsModal.tsx (2)
143-143: Consider usingTranscomponent for modal title consistency.While using
t('Settings')works correctly, consider using theTranscomponent for consistency with other translated strings in this file:-title={t('Settings')} +title={<Trans>Settings</Trans>}This would maintain a consistent approach to translations throughout the component.
Also applies to: 157-157
Line range hint
143-190: Consider enhancing keyboard accessibility for scrollable content.The file paths are displayed in scrollable containers. Consider adding keyboard navigation support to allow users to scroll these containers using arrow keys. This would improve accessibility for keyboard users.
Example implementation:
function handleKeyDown(e: React.KeyboardEvent) { const container = e.currentTarget; if (e.key === 'ArrowLeft') { container.scrollLeft -= 50; } else if (e.key === 'ArrowRight') { container.scrollLeft += 50; } }Then add to the scrollable Text component:
<Text title={documentDir} + tabIndex={0} + onKeyDown={handleKeyDown} style={{ /* existing styles */ }} >packages/desktop-client/src/components/modals/EnvelopeBudgetSummaryModal.tsx (1)
78-80: Consider standardizing variable naming across notifications.The implementation is good and consistent with the transfer message pattern. Consider standardizing the variable name to
sourceCategoryNameto be more explicit about its role in the context of covering overbudgeted amounts.- message: t('Covered overbudgeted from {{categoryName}}', { - categoryName: categoriesById[categoryId].name, + message: t('Covered overbudgeted from {{sourceCategoryName}}', { + sourceCategoryName: categoriesById[categoryId].name,packages/desktop-client/src/components/budget/tracking/budgetsummary/BudgetTotal.tsx (1)
55-57: Consider adding translation comments for better context.While the translation string is now properly structured with placeholders, adding a translation comment would help translators understand what
usedAmountandallocatedAmountrepresent in this context.<Trans> + {/* Translators: Shows budget usage, e.g., "$500 of $1000" */} {{ usedAmount: '' }} of {{ allocatedAmount: '' }} </Trans>packages/desktop-client/src/components/settings/Format.tsx (1)
24-41: Well-structured hook for days of week translations.Good improvements:
- Proper typing with SyncedPrefs
- Translations moved to a reusable hook
- Consistent structure for all days
Consider adding a JSDoc comment to document the hook's purpose and return type.
+/** + * Hook to provide translated days of the week + * @returns {{ daysOfWeek: Array<{ value: SyncedPrefs['firstDayOfWeekIdx']; label: string }> }} + */ function useDaysOfWeek() {packages/desktop-client/src/components/accounts/Reconcile.tsx (1)
86-93: Improved translation string structure with proper typing.Good improvements:
- Proper use of TransObjectLiteral type
- Clear placeholders for dynamic values
- Maintained HTML structure within translation
Consider adding a translation comment to explain the context and placeholders.
<Trans> + {/* Translators: Shows reconciliation status with bank balance comparison */} Your cleared balance{' '} <strong>{{ clearedBalance } as TransObjectLiteral}</strong>{' '} needs <strong>{{ difference } as TransObjectLiteral}</strong> to match <br /> your bank's balance of{' '} <Text style={{ fontWeight: 700 }}> {{ bankBalance } as TransObjectLiteral} </Text> </Trans>packages/desktop-client/src/components/reports/spreadsheets/cash-flow-spreadsheet.tsx (1)
214-234: Consider extracting label translations to constants.While the translation implementation is correct, consider extracting these frequently used labels to constants at the top of the file to improve maintainability and reusability.
+const CASH_FLOW_LABELS = { + income: t('Income:'), + expenses: t('Expenses:'), + change: t('Change:'), + transfers: t('Transfers:'), + balance: t('Balance:'), +}; // Then in the JSX: - left={t('Income:')} + left={CASH_FLOW_LABELS.income}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (27)
packages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Area-Graph-and-checks-the-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Area-Graph-and-checks-the-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Area-Graph-and-checks-the-visuals-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Bar-Graph-and-checks-the-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Bar-Graph-and-checks-the-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Bar-Graph-and-checks-the-visuals-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Data-Table-and-checks-the-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Data-Table-and-checks-the-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Data-Table-and-checks-the-visuals-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Donut-Graph-and-checks-the-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Donut-Graph-and-checks-the-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Donut-Graph-and-checks-the-visuals-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Line-Graph-and-checks-the-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Line-Graph-and-checks-the-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Line-Graph-and-checks-the-visuals-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-labels-button-shows-the-labels-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-labels-button-shows-the-labels-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-labels-button-shows-the-labels-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-legend-button-shows-the-legend-side-bar-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-legend-button-shows-the-legend-side-bar-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-legend-button-shows-the-legend-side-bar-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/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-summary-button-shows-the-summary-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-summary-button-shows-the-summary-3-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!**/*.png
📒 Files selected for processing (39)
packages/desktop-client/src/components/App.tsx(1 hunks)packages/desktop-client/src/components/accounts/Account.tsx(2 hunks)packages/desktop-client/src/components/accounts/Reconcile.tsx(2 hunks)packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx(1 hunks)packages/desktop-client/src/components/budget/BalanceWithCarryover.tsx(2 hunks)packages/desktop-client/src/components/budget/SidebarCategory.tsx(1 hunks)packages/desktop-client/src/components/budget/SidebarGroup.tsx(1 hunks)packages/desktop-client/src/components/budget/envelope/budgetsummary/ToBudgetMenu.tsx(1 hunks)packages/desktop-client/src/components/budget/index.tsx(1 hunks)packages/desktop-client/src/components/budget/tracking/budgetsummary/BudgetTotal.tsx(2 hunks)packages/desktop-client/src/components/filters/NameFilter.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/Login.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/OpenIdForm.tsx(2 hunks)packages/desktop-client/src/components/mobile/accounts/Accounts.tsx(4 hunks)packages/desktop-client/src/components/modals/BudgetPageMenuModal.tsx(1 hunks)packages/desktop-client/src/components/modals/ConfirmTransactionEditModal.tsx(4 hunks)packages/desktop-client/src/components/modals/CreateLocalAccountModal.tsx(3 hunks)packages/desktop-client/src/components/modals/EditRuleModal.jsx(2 hunks)packages/desktop-client/src/components/modals/EnvelopeBudgetSummaryModal.tsx(5 hunks)packages/desktop-client/src/components/modals/HoldBufferModal.tsx(1 hunks)packages/desktop-client/src/components/modals/KeyboardShortcutModal.tsx(4 hunks)packages/desktop-client/src/components/modals/manager/ConfirmChangeDocumentDir.tsx(1 hunks)packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx(1 hunks)packages/desktop-client/src/components/modals/manager/FilesSettingsModal.tsx(3 hunks)packages/desktop-client/src/components/reports/graphs/CalendarGraph.tsx(2 hunks)packages/desktop-client/src/components/reports/graphs/SpendingGraph.tsx(2 hunks)packages/desktop-client/src/components/reports/reports/Calendar.tsx(2 hunks)packages/desktop-client/src/components/reports/reports/CashFlow.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/CustomReport.tsx(3 hunks)packages/desktop-client/src/components/reports/spreadsheets/cash-flow-spreadsheet.tsx(3 hunks)packages/desktop-client/src/components/schedules/ScheduleDetails.tsx(1 hunks)packages/desktop-client/src/components/schedules/ScheduleLink.tsx(1 hunks)packages/desktop-client/src/components/settings/Encryption.tsx(1 hunks)packages/desktop-client/src/components/settings/Format.tsx(2 hunks)packages/desktop-client/src/components/settings/index.tsx(1 hunks)packages/desktop-client/src/components/transactions/SimpleTransactionsTable.tsx(4 hunks)packages/loot-core/src/client/actions/budgets.ts(1 hunks)packages/loot-core/src/server/main.ts(1 hunks)packages/loot-core/src/shared/errors.ts(1 hunks)
✅ Files skipped from review due to trivial changes (23)
- packages/desktop-client/src/components/modals/manager/ConfirmChangeDocumentDir.tsx
- packages/desktop-client/src/components/manager/subscribe/Login.tsx
- packages/desktop-client/src/components/budget/SidebarGroup.tsx
- packages/desktop-client/src/components/App.tsx
- packages/desktop-client/src/components/schedules/ScheduleLink.tsx
- packages/desktop-client/src/components/filters/NameFilter.tsx
- packages/desktop-client/src/components/budget/envelope/budgetsummary/ToBudgetMenu.tsx
- packages/desktop-client/src/components/modals/HoldBufferModal.tsx
- packages/desktop-client/src/components/modals/BudgetPageMenuModal.tsx
- packages/desktop-client/src/components/modals/KeyboardShortcutModal.tsx
- packages/desktop-client/src/components/settings/Encryption.tsx
- packages/desktop-client/src/components/modals/EditRuleModal.jsx
- packages/desktop-client/src/components/accounts/Account.tsx
- packages/desktop-client/src/components/manager/subscribe/OpenIdForm.tsx
- packages/loot-core/src/server/main.ts
- packages/desktop-client/src/components/budget/SidebarCategory.tsx
- packages/desktop-client/src/components/reports/reports/Calendar.tsx
- packages/desktop-client/src/components/reports/reports/CashFlow.tsx
- packages/desktop-client/src/components/budget/index.tsx
- packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
- packages/desktop-client/src/components/settings/index.tsx
- packages/loot-core/src/client/actions/budgets.ts
- packages/desktop-client/src/components/reports/graphs/CalendarGraph.tsx
🧰 Additional context used
📓 Learnings (3)
packages/loot-core/src/shared/errors.ts (1)
Learnt from: tlesicka
PR: actualbudget/actual#3689
File: packages/loot-core/src/server/main.ts:1873-1877
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In the `duplicate-budget` function in `packages/loot-core/src/server/main.ts`, when `loadBudget` returns an object with an `error` property where `error` is a string, returning `error` directly is consistent with the function's declared return type `Promise<string>`.
packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx (4)
Learnt from: tlesicka
PR: actualbudget/actual#3689
File: packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx:181-196
Timestamp: 2024-11-10T16:45:25.627Z
Learning: After refactoring `nameError` in `DuplicateFileModal.tsx`, it's not necessary to use `setNameError` in the `onPress` handlers when validation fails.
Learnt from: tlesicka
PR: actualbudget/actual#3689
File: packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx:144-156
Timestamp: 2024-11-10T16:45:31.225Z
Learning: In `packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx`, styles for buttons may differ for each button in the future, so avoid suggesting extraction of common styles in this file.
Learnt from: tlesicka
PR: actualbudget/actual#3689
File: packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx:181-196
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In `DuplicateFileModal.tsx`, `trim()` is performed during the blur event when validating `newName`, so additional trimming is not necessary in the `onPress` handlers.
Learnt from: tlesicka
PR: actualbudget/actual#3689
File: packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx:0-0
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In `DuplicateFileModal.tsx`, the `validateAndSetName` function trims `newName` before validation and duplication.
packages/desktop-client/src/components/mobile/accounts/Accounts.tsx (1)
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-11-10T16:45:31.225Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
🔇 Additional comments (30)
packages/desktop-client/src/components/schedules/ScheduleDetails.tsx (1)
974-974: Great improvement to the translation string!The change from direct interpolation
{props.error}to named interpolation{{ errorReason: props.error }}is a good practice for internationalization. Named interpolations provide better context for translators and prevent potential issues with word order in different languages.packages/desktop-client/src/components/reports/graphs/SpendingGraph.tsx (1)
3-3: LGTM: Import change aligns with i18n best practicesAdding the
Transcomponent import is appropriate for handling complex translations with interpolation.packages/desktop-client/src/components/modals/manager/FilesSettingsModal.tsx (1)
112-117: Well-structured translation implementation!The use of
Transcomponent here is appropriate as it keeps the entire string, including HTML formatting, as a single translatable unit. This prevents sentence fragmentation and maintains context for translators.packages/desktop-client/src/components/modals/EnvelopeBudgetSummaryModal.tsx (4)
28-29: LGTM! Good hook placement.Moving the translation hook to the start of the component follows React hooks best practices and improves code organization.
45-45: Great improvement to the modal title!The new title "Transfer to category" is clearer and more translation-friendly by:
- Using simpler language
- Removing unnecessary punctuation
- Providing better context for translators
56-59: Excellent i18n implementation!The notification message now:
- Uses proper i18next interpolation syntax
- Has descriptive variable names
- Allows for flexible word order in different languages
69-69: Consistent improvement in modal title!The simplified title maintains the pattern of improvements seen earlier:
- Removes unnecessary punctuation
- Uses more natural language
- Maintains consistency with other modal titles
packages/desktop-client/src/components/budget/tracking/budgetsummary/BudgetTotal.tsx (1)
7-7: LGTM! Import statement added correctly.The
Transcomponent import aligns with the PR's objective of improving translation handling.packages/desktop-client/src/components/modals/ConfirmTransactionEditModal.tsx (3)
3-3: LGTM! Import statement updated correctly.Good practice to keep both
useTranslationandTransimports together.
38-77: Well-structured translation blocks for different scenarios.The translation strings are now properly structured and provide better context for translators. Each condition has its own clear, complete sentence.
100-118: Consistent pattern for accessibility and visible text.Good practice using:
t()for aria-labelsTranscomponent for visible button textpackages/desktop-client/src/components/settings/Format.tsx (1)
79-80: LGTM! Clean hook usage.The hook is properly destructured and used in the component.
packages/desktop-client/src/components/accounts/Reconcile.tsx (2)
8-8: LGTM! Type import added correctly.Good addition of TransObjectLiteral type for improved type safety.
100-106: Consistent capitalization in UI text.Good improvements in text consistency:
- "Done reconciling" instead of "Done Reconciling"
- "Create reconciliation transaction" instead of "Create Reconciliation Transaction"
packages/loot-core/src/shared/errors.ts (1)
104-106: LGTM! Improved error message clarity and translation support.The changes enhance the error message by:
- Using proper
{{id}}syntax for i18n variable interpolation- Correcting capitalization of "ID"
- Adding helpful context about where to find the budget ID in settings
packages/desktop-client/src/components/modals/CreateLocalAccountModal.tsx (1)
4-4: LGTM! Enhanced translation support for complex text.The changes properly implement the
Transcomponent for text containing links and HTML elements, which is the recommended approach for handling complex translations in react-i18next.Also applies to: 130-130, 142-152
packages/desktop-client/src/components/transactions/SimpleTransactionsTable.tsx (2)
129-134: LGTM! Proper handling of dynamic text translation.The account field correctly uses the
t()function for the dynamic "No account" fallback text while preserving the account name display.
220-220: LGTM! Consistent use of Trans component for static labels.Table headers now properly use the
Transcomponent for static text, improving translation management.Also applies to: 226-226, 232-232, 238-238, 244-244, 250-250, 256-256
packages/desktop-client/src/components/reports/spreadsheets/cash-flow-spreadsheet.tsx (1)
153-154: LGTM! Good variable renaming to avoid conflicts.The parameter renaming from
ttotransin map functions prevents confusion with the translation function.Also applies to: 156-157
packages/desktop-client/src/components/budget/BalanceWithCarryover.tsx (5)
8-8: LGTM!The necessary imports for the
Transcomponent andTransObjectLiteraltype are correctly added.Also applies to: 12-12
153-153: LGTM!The static text is correctly wrapped with the
Transcomponent for translation.
157-166: LGTM!The translation is correctly implemented with dynamic value interpolation for the amount.
170-179: LGTM!The translation is correctly implemented with dynamic value interpolation for the amount.
184-205: LGTM!The translations are correctly implemented with:
- Dynamic value interpolation for goal type and amount
- Proper type casting using
TransObjectLiteralpackages/desktop-client/src/components/mobile/accounts/Accounts.tsx (3)
2-2: LGTM!The necessary import for the
Transcomponent is correctly added.
198-202: LGTM!The translation is correctly implemented with HTML-like elements for rich text formatting.
259-259: LGTM!The aria-label is correctly implemented with translation support, improving accessibility.
packages/desktop-client/src/components/reports/reports/CustomReport.tsx (3)
2-2: LGTM!The necessary imports for the
Transcomponent andTransObjectLiteraltype are correctly added.Also applies to: 20-20
701-703: LGTM!The translation is correctly implemented with dynamic value interpolation for the report name.
709-718: LGTM!The translation is correctly implemented with:
- HTML-like elements for text formatting
- Dynamic value interpolation for the report name
- Proper type casting using
TransObjectLiteral
|
/update-vrt |
matt-fidd
force-pushed
the
translations-2
branch
from
397d0be to
e30f24c
Compare
|
/update-vrt |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/desktop-client/src/components/settings/Format.tsx (2)
24-41: Memoize the days array to prevent unnecessary re-renders.The
daysOfWeekarray is recreated on every render. Consider usinguseMemoto optimize performance.function useDaysOfWeek() { const { t } = useTranslation(); - const daysOfWeek: { - value: SyncedPrefs['firstDayOfWeekIdx']; - label: string; - }[] = [ + const daysOfWeek = React.useMemo(() => ([ { value: '0', label: t('Sunday') }, { value: '1', label: t('Monday') }, { value: '2', label: t('Tuesday') }, { value: '3', label: t('Wednesday') }, { value: '4', label: t('Thursday') }, { value: '5', label: t('Friday') }, { value: '6', label: t('Saturday') }, - ] as const; + ] as const), [t]); return { daysOfWeek }; }
Line range hint
1-1: Consider enabling TypeScript strict mode.The file currently disables strict type checking with
@ts-strict-ignore. Consider addressing the TypeScript errors and enabling strict mode for better type safety.packages/desktop-client/src/components/budget/BalanceWithCarryover.tsx (2)
153-179: Consider extracting funding status messages for better maintainability.The translation implementation looks good, but the nested conditional rendering could be simplified by extracting the funding status messages into a separate component or helper function.
Example refactor:
const FundingStatus = ({ difference, format }) => { if (difference === 0) { return ( <span style={{ color: theme.noticeText }}> <Trans>Fully funded</Trans> </span> ); } const isOverfunded = difference > 0; return ( <span style={{ color: isOverfunded ? theme.noticeText : theme.errorText }}> <Trans> {isOverfunded ? 'Overfunded (' : 'Underfunded ('} {{ amount: format(difference, 'financial') }} ) </Trans> </span> ); };
184-205: Consider creating a type-safe translation key mapping.While the current implementation works, we could improve type safety for translation keys by creating a mapping between the goal types and their translations.
Example:
const GOAL_TYPE_TRANSLATIONS = { LONG: 'Long', TEMPLATE: 'Template', } as const; // Then use it like: type: longGoalValue === 1 ? t(GOAL_TYPE_TRANSLATIONS.LONG) : t(GOAL_TYPE_TRANSLATIONS.TEMPLATE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (27)
packages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Area-Graph-and-checks-the-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Area-Graph-and-checks-the-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Area-Graph-and-checks-the-visuals-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Bar-Graph-and-checks-the-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Bar-Graph-and-checks-the-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Bar-Graph-and-checks-the-visuals-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Data-Table-and-checks-the-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Data-Table-and-checks-the-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Data-Table-and-checks-the-visuals-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Donut-Graph-and-checks-the-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Donut-Graph-and-checks-the-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Donut-Graph-and-checks-the-visuals-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Line-Graph-and-checks-the-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Line-Graph-and-checks-the-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Switches-to-Line-Graph-and-checks-the-visuals-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-labels-button-shows-the-labels-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-labels-button-shows-the-labels-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-labels-button-shows-the-labels-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-legend-button-shows-the-legend-side-bar-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-legend-button-shows-the-legend-side-bar-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-legend-button-shows-the-legend-side-bar-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/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-summary-button-shows-the-summary-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-custom-reports-Validates-that-show-summary-button-shows-the-summary-3-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!**/*.png
📒 Files selected for processing (39)
packages/desktop-client/src/components/App.tsx(1 hunks)packages/desktop-client/src/components/accounts/Account.tsx(2 hunks)packages/desktop-client/src/components/accounts/Reconcile.tsx(2 hunks)packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx(1 hunks)packages/desktop-client/src/components/budget/BalanceWithCarryover.tsx(2 hunks)packages/desktop-client/src/components/budget/SidebarCategory.tsx(1 hunks)packages/desktop-client/src/components/budget/SidebarGroup.tsx(1 hunks)packages/desktop-client/src/components/budget/envelope/budgetsummary/ToBudgetMenu.tsx(1 hunks)packages/desktop-client/src/components/budget/index.tsx(1 hunks)packages/desktop-client/src/components/budget/tracking/budgetsummary/BudgetTotal.tsx(2 hunks)packages/desktop-client/src/components/filters/NameFilter.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/Login.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/OpenIdForm.tsx(2 hunks)packages/desktop-client/src/components/mobile/accounts/Accounts.tsx(4 hunks)packages/desktop-client/src/components/modals/BudgetPageMenuModal.tsx(1 hunks)packages/desktop-client/src/components/modals/ConfirmTransactionEditModal.tsx(4 hunks)packages/desktop-client/src/components/modals/CreateLocalAccountModal.tsx(3 hunks)packages/desktop-client/src/components/modals/EditRuleModal.jsx(2 hunks)packages/desktop-client/src/components/modals/EnvelopeBudgetSummaryModal.tsx(5 hunks)packages/desktop-client/src/components/modals/HoldBufferModal.tsx(1 hunks)packages/desktop-client/src/components/modals/KeyboardShortcutModal.tsx(4 hunks)packages/desktop-client/src/components/modals/manager/ConfirmChangeDocumentDir.tsx(1 hunks)packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx(1 hunks)packages/desktop-client/src/components/modals/manager/FilesSettingsModal.tsx(3 hunks)packages/desktop-client/src/components/reports/graphs/CalendarGraph.tsx(2 hunks)packages/desktop-client/src/components/reports/graphs/SpendingGraph.tsx(2 hunks)packages/desktop-client/src/components/reports/reports/Calendar.tsx(2 hunks)packages/desktop-client/src/components/reports/reports/CashFlow.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/CustomReport.tsx(3 hunks)packages/desktop-client/src/components/reports/spreadsheets/cash-flow-spreadsheet.tsx(3 hunks)packages/desktop-client/src/components/schedules/ScheduleDetails.tsx(1 hunks)packages/desktop-client/src/components/schedules/ScheduleLink.tsx(1 hunks)packages/desktop-client/src/components/settings/Encryption.tsx(1 hunks)packages/desktop-client/src/components/settings/Format.tsx(2 hunks)packages/desktop-client/src/components/settings/index.tsx(1 hunks)packages/desktop-client/src/components/transactions/SimpleTransactionsTable.tsx(4 hunks)packages/loot-core/src/client/actions/budgets.ts(1 hunks)packages/loot-core/src/server/main.ts(1 hunks)packages/loot-core/src/shared/errors.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (36)
- packages/desktop-client/src/components/filters/NameFilter.tsx
- packages/desktop-client/src/components/budget/SidebarCategory.tsx
- packages/desktop-client/src/components/manager/subscribe/Login.tsx
- packages/desktop-client/src/components/modals/manager/ConfirmChangeDocumentDir.tsx
- packages/desktop-client/src/components/modals/HoldBufferModal.tsx
- packages/desktop-client/src/components/reports/reports/CashFlow.tsx
- packages/desktop-client/src/components/budget/SidebarGroup.tsx
- packages/desktop-client/src/components/settings/Encryption.tsx
- packages/desktop-client/src/components/budget/envelope/budgetsummary/ToBudgetMenu.tsx
- packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
- packages/desktop-client/src/components/schedules/ScheduleLink.tsx
- packages/desktop-client/src/components/modals/KeyboardShortcutModal.tsx
- packages/loot-core/src/shared/errors.ts
- packages/desktop-client/src/components/budget/index.tsx
- packages/desktop-client/src/components/modals/BudgetPageMenuModal.tsx
- packages/desktop-client/src/components/App.tsx
- packages/desktop-client/src/components/settings/index.tsx
- packages/loot-core/src/client/actions/budgets.ts
- packages/desktop-client/src/components/reports/graphs/CalendarGraph.tsx
- packages/desktop-client/src/components/manager/subscribe/OpenIdForm.tsx
- packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx
- packages/desktop-client/src/components/modals/EditRuleModal.jsx
- packages/desktop-client/src/components/reports/spreadsheets/cash-flow-spreadsheet.tsx
- packages/desktop-client/src/components/accounts/Reconcile.tsx
- packages/desktop-client/src/components/accounts/Account.tsx
- packages/desktop-client/src/components/reports/reports/CustomReport.tsx
- packages/desktop-client/src/components/reports/graphs/SpendingGraph.tsx
- packages/desktop-client/src/components/schedules/ScheduleDetails.tsx
- packages/desktop-client/src/components/reports/reports/Calendar.tsx
- packages/desktop-client/src/components/modals/ConfirmTransactionEditModal.tsx
- packages/desktop-client/src/components/modals/manager/FilesSettingsModal.tsx
- packages/desktop-client/src/components/transactions/SimpleTransactionsTable.tsx
- packages/loot-core/src/server/main.ts
- packages/desktop-client/src/components/modals/EnvelopeBudgetSummaryModal.tsx
- packages/desktop-client/src/components/mobile/accounts/Accounts.tsx
- packages/desktop-client/src/components/modals/CreateLocalAccountModal.tsx
🔇 Additional comments (4)
packages/desktop-client/src/components/settings/Format.tsx (1)
79-79: LGTM! Good i18n improvement.The usage of the new
useDaysOfWeekhook effectively centralizes the translation logic for days of the week, making it more maintainable and consistent with the PR's goal of improving translation strings.packages/desktop-client/src/components/budget/tracking/budgetsummary/BudgetTotal.tsx (1)
7-7: LGTM! Import of Trans component aligns with i18n improvements.The addition of the
Transcomponent import is appropriate for the improved translation handling approach.packages/desktop-client/src/components/budget/BalanceWithCarryover.tsx (2)
8-8: LGTM! Necessary imports for i18n improvements.The addition of
Transcomponent andTransObjectLiteraltype imports supports the enhanced internationalization capabilities.Also applies to: 12-13
209-229: LGTM! Verify translation string consistency across the application.The implementation of budgeted and balance displays is correct. However, we should ensure that similar financial labels are translated consistently across the application.
✅ Verification successful
Translation implementation is consistent across the codebase
The implementation of "Budgeted" and "Balance" translations follows a consistent pattern using
<Trans>components throughout the application, with proper formatting and structure maintained across all components.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other occurrences of "Budgeted" and "Balance" translations # to verify consistency echo "Checking 'Budgeted' translations:" rg -l "Budgeted" --type ts --type tsx echo "Checking 'Balance' translations:" rg -l "Balance" --type ts --type tsxLength of output: 294
Script:
#!/bin/bash # Search with correct file type handling and comprehensive patterns echo "Searching for 'Budgeted' translations:" rg -i "budgeted" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' --type tsx --type ts -C 2 echo -e "\nSearching for 'Balance' translations:" rg -i "balance" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' --type tsx --type ts -C 2 | grep -i "balance.*[\"'>]" echo -e "\nSearching for Trans components with these terms:" rg "<Trans>" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' --type tsx --type ts -A 3 -B 1 | grep -i -E "(budgeted|balance)"Length of output: 76458
packages/desktop-client/src/components/budget/tracking/budgetsummary/BudgetTotal.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Is this expected to happen? The placeholder name is missing.
Carrying on from #4041
Down to "O" now. I don't think I need to bother with release notes for the rest of these unless you do?