🌍 improve translation strings - part 1 #4041
Conversation
actual-github-bot
bot
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
matt-fidd
changed the title
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 No assets were bigger Smaller No assets were smaller Unchanged
|
|
Warning Rate limit exceeded@matt-fidd has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 2 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request encompasses a comprehensive set of modifications across multiple components in the desktop client, primarily focusing on internationalization, localization, and text consistency. The changes span various files within the
The modifications do not alter the fundamental logic or functionality of the components but instead focus on improving the user interface's linguistic precision and localization capabilities. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (19)
packages/desktop-client/src/components/modals/PasswordEnableModal.tsx (1)
51-51: Offer more context for "Internal error" message
As with similar changes in other files, updating from "Internal server error" to "Internal error" is simpler, but it may be unclear to non-technical users. Consider adding guidance in the UI on how to proceed if they encounter this error.packages/desktop-client/src/components/reports/SaveReportDelete.tsx (1)
25-32: Consider consolidating the translation for better context clarity.
Wrapping<Text>and<View>components in a single<Trans>block is valid, but nested<Trans>elements may introduce complexity or confusion in translation files. Consider providing placeholders for style or structural elements within a single<Trans>for more streamlined translations.packages/loot-core/src/client/shared-listeners.ts (1)
52-52: Consider using placeholders or i18n interpolation for future-proofing links.
Adding the link as an inline markdown string is fine if the consumer always parses Markdown. However, if there’s any shift in how notifications handle user-facing content or a localizer needs to adapt the link, having a placeholder in the translation might offer more flexibility.packages/desktop-client/src/components/settings/Upcoming.tsx (3)
46-46: Consider adopting a more descriptive translation key.
While'Upcoming Length'works for now, a more structured approach (e.g.'settings.upcoming.lengthTitle') could help translators and maintain consistency across the app.
53-57: Add translation context for the InfoBubble label.
Providing translators with more context can reduce confusion. For instance, add a comment or structured key to clarify that this label pertains to recurring transactions.
84-85: Provide a localized fallback for the upcoming length label.
Right now, the fallback('1 Week')is hard-coded in English. Consider usingt('1 Week')or another localized alternative for a fully translated experience.packages/desktop-client/src/components/payees/PayeeMenu.tsx (1)
66-70: Enhance flexibility of translated text and placeholdersCurrently,
"and more"is embedded directly into<Trans>, which might limit translation flexibility for certain languages where placeholder ordering differs. Consider usingi18nKeyand placeholders for clarity and potential reordering in translations:{selectedPayees.size > 4 ? ( - <Trans>{{ selectedPayeeNames }}, and more</Trans> + <Trans + i18nKey="payeeMenu.andMore" + defaults="{{selected}} and more" + values={{ selected: selectedPayeeNames }} + /> ) : ( selectedPayeeNames )}packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx (1)
518-544: Consider consolidating the translated sentence.Currently, the sentence around “Move schedule before/after weekend” is split across multiple
<Trans>invocations, which can be trickier for translators and could impact readability. A single translation string might be more straightforward for localization teams.Below is an example of unifying these lines into a single translation block. This diff modifies the lines within this segment to illustrate how the text could be combined under one
<Trans>element:518-544 - <Trans> - <label - htmlFor="form_skipwe" - style={{ - userSelect: 'none', - marginRight: 5, - }} - > - Move schedule{' '} - </label> - <Select - id="solve_dropdown" - options={[ - ['before', 'before'], - ['after', 'after'], - ]} - value={state.config.weekendSolveMode} - onChange={value => dispatch({ type: 'set-weekend-solve', value })} - disabled={!skipWeekend} - /> - <label - htmlFor="solve_dropdown" - style={{ userSelect: 'none', marginLeft: 5 }} - > - <Trans> {{ beforeOrAfter: '' }} weekend</Trans> - </label> - </Trans> + <Trans i18nKey="moveScheduleWeekend"> + Move schedule + <Select + id="solve_dropdown" + options={[ + ['before', 'before'], + ['after', 'after'], + ]} + value={state.config.weekendSolveMode} + onChange={value => dispatch({ type: 'set-weekend-solve', value })} + disabled={!skipWeekend} + /> + weekend + </Trans>packages/desktop-client/src/components/settings/index.tsx (4)
48-51: Ensure punctuation and spacing are contextual within multi-line translations.
When splitting the translation text across multiple lines, confirm the correct spacing is preserved by either including line breaks or punctuation in the translation file. This ensures the final rendered text remains grammatically sound.
87-87: Use consistent apostrophe styling.
While the existing text “You’re up to date!” generally looks fine, ensure that all apostrophes (smart quotes vs. straight quotes) are consistent throughout the translation files to avoid rendering issues.
116-121: Check that placeholders and grammar remain appropriate across locales.
These lines effectively present descriptive text within theTranswrapper, but confirm that placeholders (like<strong>IDs</strong>) and sentence structure read correctly in other languages.
129-131: Promote consistency in naming placeholders.
Here,syncIdis used as the placeholder key, while in other translations it might appear differently. Confirm uniform placeholder usage to avoid confusion in translation files.packages/desktop-client/src/components/modals/GoalTemplateModal.tsx (4)
32-35: Consider wrapping example text with translation or placeholders.
The raw string#template $10 repeat every week starting 2025-01-03appears to be an illustrative command. If users in other locales need these instructions, you may want to consider localizing currency/format or explicitly marking them as code examples.
40-43: Same consideration for localized date and currency.
The string#template $10 repeat every week starting 2025-01-03 up to $80is kept as a literal. If you want broader localization support, consider parameterizing or wrapping it with<Trans>ort().
64-68: Potential param-based approach for currency.
Lines showing#template up to $150and<Trans>Up to $150 each month...</Trans>are clear, but for completeness in different locales, you might consider dynamic placeholders for currency.
174-184: Link wrapped in<Trans>tag.
This approach is valid. If you ever need deeper i18n segmentation, you could use placeholders for the link. For now, this is entirely acceptable.packages/desktop-client/src/components/settings/BudgetTypeSettings.tsx (1)
33-38: Check punctuation within translatable text.The text here references the concept of “don’t overspend.” If a translation requires straight quotes vs. curly quotes (’ vs '), ensure consistency across all strings for improved translator clarity and uniform style.
packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx (1)
45-45: **Concatenate translation strings carefully **Using
' - ' + t('copy')is fine, but consider whether translators need the dash as part of the string. In some languages, spacing or punctuation may differ. For example:- const fileEndingTranslation = ' - ' + t('copy'); + const fileEndingTranslation = t('{{dash}} copy', { dash: ' -' });However, if all translators are aware of this requirement, the current approach is acceptable.
packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx (1)
275-275: **Ensure consistent capitalization for “Set up bank-sync” **Manual check for translations. Some languages may require different letter case or spacing. This is acceptable as is, provided all references to “bank-sync” maintain the same style across the app.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
packages/desktop-client/src/components/App.tsx(1 hunks)packages/desktop-client/src/components/LoggedInUser.tsx(2 hunks)packages/desktop-client/src/components/Modals.tsx(1 hunks)packages/desktop-client/src/components/accounts/AccountSyncCheck.tsx(1 hunks)packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsx(1 hunks)packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx(3 hunks)packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx(1 hunks)packages/desktop-client/src/components/budget/IncomeHeader.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/ChangePassword.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/OpenIdForm.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/common.tsx(1 hunks)packages/desktop-client/src/components/modals/CloseAccountModal.tsx(8 hunks)packages/desktop-client/src/components/modals/CreateAccountModal.tsx(5 hunks)packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx(7 hunks)packages/desktop-client/src/components/modals/EditRuleModal.jsx(11 hunks)packages/desktop-client/src/components/modals/EditUser.tsx(2 hunks)packages/desktop-client/src/components/modals/GoCardlessExternalMsgModal.tsx(3 hunks)packages/desktop-client/src/components/modals/GoCardlessInitialiseModal.tsx(3 hunks)packages/desktop-client/src/components/modals/GoalTemplateModal.tsx(6 hunks)packages/desktop-client/src/components/modals/ImportTransactionsModal/ParsedDate.tsx(3 hunks)packages/desktop-client/src/components/modals/LoadBackupModal.tsx(2 hunks)packages/desktop-client/src/components/modals/ManageRulesModal.tsx(1 hunks)packages/desktop-client/src/components/modals/OpenIDEnableModal.tsx(1 hunks)packages/desktop-client/src/components/modals/PasswordEnableModal.tsx(2 hunks)packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx(8 hunks)packages/desktop-client/src/components/modals/SimpleFinInitialiseModal.tsx(3 hunks)packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx(2 hunks)packages/desktop-client/src/components/payees/PayeeMenu.tsx(3 hunks)packages/desktop-client/src/components/reports/DateRange.tsx(1 hunks)packages/desktop-client/src/components/reports/Header.tsx(2 hunks)packages/desktop-client/src/components/reports/Overview.tsx(1 hunks)packages/desktop-client/src/components/reports/SaveReportDelete.tsx(3 hunks)packages/desktop-client/src/components/reports/reports/NetWorth.tsx(1 hunks)packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx(5 hunks)packages/desktop-client/src/components/settings/BudgetTypeSettings.tsx(2 hunks)packages/desktop-client/src/components/settings/Encryption.tsx(2 hunks)packages/desktop-client/src/components/settings/Format.tsx(4 hunks)packages/desktop-client/src/components/settings/Reset.tsx(3 hunks)packages/desktop-client/src/components/settings/Themes.tsx(4 hunks)packages/desktop-client/src/components/settings/Upcoming.tsx(4 hunks)packages/desktop-client/src/components/settings/index.tsx(5 hunks)packages/desktop-client/src/components/sidebar/BudgetName.tsx(1 hunks)packages/loot-core/src/client/shared-listeners.ts(1 hunks)packages/loot-core/src/shared/errors.ts(1 hunks)
✅ Files skipped from review due to trivial changes (14)
- packages/desktop-client/src/components/budget/IncomeHeader.tsx
- packages/desktop-client/src/components/modals/EditUser.tsx
- packages/desktop-client/src/components/manager/subscribe/common.tsx
- packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
- packages/desktop-client/src/components/sidebar/BudgetName.tsx
- packages/loot-core/src/shared/errors.ts
- packages/desktop-client/src/components/manager/subscribe/OpenIdForm.tsx
- packages/desktop-client/src/components/accounts/AccountSyncCheck.tsx
- packages/desktop-client/src/components/modals/LoadBackupModal.tsx
- packages/desktop-client/src/components/modals/OpenIDEnableModal.tsx
- packages/desktop-client/src/components/reports/Header.tsx
- packages/desktop-client/src/components/modals/GoCardlessExternalMsgModal.tsx
- packages/desktop-client/src/components/LoggedInUser.tsx
- packages/desktop-client/src/components/reports/reports/NetWorth.tsx
🧰 Additional context used
📓 Learnings (2)
packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx (5)
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: 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: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.
Learnt from: tlesicka
PR: actualbudget/actual#3689
File: packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx:93-94
Timestamp: 2024-11-10T16:45:31.225Z
Learning: In the `onBlur` handler, it's unnecessary to call `setNewName` with the trimmed value because `validateAndSetName` already calls `setNewName` with the trimmed name.
packages/desktop-client/src/components/modals/CreateAccountModal.tsx (2)
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/modals/CreateLocalAccountModal.tsx:32-33
Timestamp: 2024-11-10T16:45:31.225Z
Learning: In this codebase, account name uniqueness checks are case-sensitive, allowing account names that differ only by case.
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/modals/CreateLocalAccountModal.tsx:47-47
Timestamp: 2024-11-10T16:45:25.627Z
Learning: Validating balance is outside the scope in `CreateLocalAccountModal.tsx`.
🪛 Biome (1.9.4)
packages/desktop-client/src/components/modals/EditRuleModal.jsx
[error] 531-538: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🪛 GitHub Check: lint
packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx
[warning] 150-150:
Avoid using straight quotes (' or ") in user-visible text. Use curly quotes (‘ ’ or “ ”) instead
[failure] 150-150:
' can be escaped with ', ‘, ', ’
🔇 Additional comments (133)
packages/desktop-client/src/components/manager/subscribe/ChangePassword.tsx (1)
32-32: Validate user feedback or logging for unexpected "Internal error"
This change broadens the default error message to “Internal error.” Consider whether this message provides enough guidance for users to understand or report the error. Providing instructions or a link to a help resource might enhance user experience.
packages/desktop-client/src/components/modals/PasswordEnableModal.tsx (1)
41-41: Standardize capitalization of error messages
Changing "Invalid Password" to "Invalid password" improves consistency. Ensure consistency across the rest of the app to create a uniform user experience.
packages/desktop-client/src/components/modals/GoCardlessInitialiseModal.tsx (4)
3-3: Usage of both useTranslation and Trans is consistent with modern i18n best practices.
This approach allows more flexible translation, especially where elements are interspersed.
86-86: Corrected hyphenation in the title from "Set-up" to "Set up."
The updated string is more accurate and consistent with standard English usage.
91-103: Use of <Trans> to enclose HTML elements and text improves translation profiling.
Wrapping the text with <Trans> allows for context-rich translations, especially with embedded links.
145-145: Switched button text to <Trans> for a unified translation approach.
Maintaining <Trans> here ensures consistent translation patterns across the repository.
packages/desktop-client/src/components/reports/SaveReportDelete.tsx (3)
2-2: Good job adopting Trans for more flexible translations.
Importing Trans aligns with best practices for handling dynamic, interpolated text.
43-43: Straightforward translation for "Yes".
This simple usage of <Trans>Yes</Trans> is consistent with the rest of the approach.
46-46: Straightforward translation for "No".
This is consistent with how "Yes" was handled, ensuring uniform translation usage across modals.
packages/desktop-client/src/components/settings/Upcoming.tsx (4)
2-2: Good usage of the useTranslation and Trans imports.
This ensures that both simple and more complex text structures can be localized.
28-28: Localizing strings properly by using t.
The usage of t helps keep translations in one place. No issues found here.
64-67: Appropriately using <Trans> with markup is good practice.
The code correctly wraps text elements within <Trans>, maintaining any embedded HTML tags or formatting. No issues found here.
73-73: Translation usage for the "Close" button.
Localizing button labels is crucial for accessibility and user experience. This is correct.
packages/desktop-client/src/components/modals/ImportTransactionsModal/ParsedDate.tsx (4)
2-2: Good import for internationalization.
Importing Trans and useTranslation from react-i18next is correct for enabling localized strings.
20-20: useTranslation hook is properly utilized.
Extracting t from useTranslation() is a standard practice for handling translations in functional components.
33-33: Swapping hardcoded text with Trans component is appropriate.
Replacing the plain text "Empty" with <Trans>Empty</Trans> facilitates translation for different locales.
39-39: Using t('Invalid') ensures the string is translatable.
Switching the hard-coded "Invalid" to t('Invalid') allows for multi-language support.
packages/desktop-client/src/components/payees/PayeeMenu.tsx (2)
1-1: Import of Trans aligns with PR objective
The addition of Trans from react-i18next is a welcome step towards better contextual translation support.
35-39: Consider handling potentially missing or undefined payee names
Although there's a check for transfer_acct, ensure that each selectedPayees entry also exists in payeesById and includes a valid name to prevent potential runtime errors.
packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx (5)
9-9: Good integration of react-i18next.
Importing both useTranslation and Trans will help ensure clarity and consistency for future translations.
36-47: Nicely encapsulated frequency options.
Using a custom hook (useFrequencyOptions) to build and return localized frequency options centralizes translation logic. This approach promotes maintainability and consistency.
51-65: Consistent day-of-week options.
Similarly, encapsulating day-of-week options in useDayOfWeekOptions is an excellent approach. It mirrors the pattern in useFrequencyOptions, making the codebase more predictable and extensible.
286-287: Consistent usage of useDayOfWeekOptions.
Incorporating the newly introduced hook here aligns with best practices for consistency and localizability.
371-372: Excellent reuse of useFrequencyOptions.
Injecting the frequency logic similarly ensures that these options remain well-maintained and easily translatable, paralleling the day-of-week approach.
packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx (10)
4-4: Imports look good.
Importing both useTranslation and Trans from 'react-i18next' is consistent with the rest of the file’s changes to improve translation handling.
90-96: Good use of <Trans> for multi-sentence translations.
Wrapping descriptive text in <Trans> will ensure better context for translators, preventing fragmented translations. This usage is properly structured.
113-118: Clear security warning.
Using <Trans> for the important password reminder is a solid improvement, ensuring the entire warning can be translated in context.
121-124: Correct scope explanation.
Explicitly clarifying that the encryption key only applies to this file is both useful for users and consistent with translation best practices.
127-131: Straightforward instructions.
Notifying users about the reset process on other devices is crucial. The <Trans> approach provides flexibility in how translators might phrase these instructions.
134-138: Spot-on recommendation.
Recommending a different password from the login password is sound security advice. Good job preserving that guidance within a <Trans> block.
161-165: Additional clarity appreciated.
Emphasizing that key generation is randomized helps users understand that reusing the same password won't preserve the old key. This text is well-suited for <Trans>.
178-178: Localized label for password field.
Switching to <Trans> ensures the label is consistent across different locales.
210-210: User-friendly "Show password" toggle.
The translation approach for the checkbox label is aligned with best practices, keeping the entire string in one <Trans> block.
224-224: Localized action button.
Using <Trans> for the “Enable” button text harmonizes the call-to-action with the rest of the translated UI.
packages/desktop-client/src/components/settings/index.tsx (6)
2-2: Good import usage of Trans from react-i18next.
The import statement looks correct and follows best practices for localizing text using the Trans component.
74-76: Trans usage with placeholders is valid.
The usage for interpolating version in the translated string is consistent with react-i18next best practices.
83-83: Good job informing users about new releases.
Displaying the latestVersion within a translatable string is an excellent approach to maintaining multi-lingual awareness without losing dynamic content.
96-96: Clearly labeled “Release Notes” link is good.
The textual clarity of this link helps users quickly identify the relevant documentation for each release.
124-126: Excellent usage of nested components within Trans.
Referencing <IDName> within the translation call is a powerful feature of react-i18next. Ensure that the budgetId variable is always defined or a fallback is provided.
69-73: Confirm fallback logic for window.Actual?.ACTUAL_VERSION.
If window.Actual is undefined or ACTUAL_VERSION is missing, ensure the app gracefully handles the missing version.
✅ Verification successful
Let me gather more information about the fallback handling.
Optional chaining provides sufficient fallback for version display
Based on the codebase analysis, the version handling is properly implemented:
- The
ACTUAL_VERSIONis defined inbrowser-preload.browser.jswith a fallback for Playwright testing:
const ACTUAL_VERSION = Platform.isPlaywright ? '99.9.9' : packageJson.version;-
The optional chaining operator (
?.) inwindow.Actual?.ACTUAL_VERSIONsafely handles undefined cases:- If
window.Actualis undefined, it returnsundefined - The version display will show as "Client version: v" in such cases, which is a graceful fallback
- If
-
The version is consistently accessed using optional chaining across the codebase (ManagementApp, settings, versions.ts), showing this is the intended pattern.
No additional fallback logic is necessary as the current implementation safely handles cases where the version might be unavailable.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 1. Search for all references to ACTUAL_VERSION usage.
# 2. Check if fallback logic is implemented for null/undefined references.
rg 'ACTUAL_VERSION' -A 3
Length of output: 2900
Script:
#!/bin/bash
# Check how ACTUAL_VERSION is defined and initialized
ast-grep --pattern 'const ACTUAL_VERSION = $_'
# Check for any default value assignments or error handling related to version
rg -B 2 -A 2 'window\.Actual\?'
Length of output: 13230
packages/desktop-client/src/components/modals/GoalTemplateModal.tsx (10)
12-12: No significant changes.
This line is blank and does not affect functionality.
58-61: Consistent approach for monthly templates.
Everything looks fine here. If these are purely illustrative, no action needed, but consider a parameter-based approach to localize $50 each month.
72-76: Maintain consistency with other currency placeholders.
Similarly, consider whether $150 should be translated or remain a literal demonstration. Current usage is consistent, so this is optional.
92-97: No issues; date usage is consistent.
The text #template $500 by 2025-03 repeat every 6 months appears to be a clear example. Localizing or parameterizing the date is a possible improvement if needed.
102-107: Optional localization for yearly repeats.
Similar to the previous block, this is consistent. Consider localizing $500 by 2025-03 if desired in the future.
112-117: Consistent multi-year approach.
This block follows the same pattern. Looks good from an implementation standpoint.
132-134: Schedule name placeholders.
#template schedule SCHEDULE_NAME is likely just an example. No code or i18n issues noted here.
138-142: Alternate schedule approach.
This second schedule template is consistent with the prior usage. Everything looks fine.
150-150: Typo fix from "Goal Tempaltes" to "Goal Templates".
Thank you for correcting the spelling; no further action required.
157-159: Clear demonstration of monthly vs. long-term goals.
#goal 1000 is straightforward, and the translation text is consistent.
packages/desktop-client/src/components/modals/ManageRulesModal.tsx (1)
28-28: Consider reintroducing translation or confirm intent for modal name.
Switching from t('manage-rules') to a static string "manage-rules" loses the ability to localize the modal name. If the name prop is being used only as an internal reference or ID, this may be fine. Otherwise, retaining a translated string would be more consistent with the broader i18n push.
packages/desktop-client/src/components/settings/BudgetTypeSettings.tsx (5)
2-2: Good usage of the Trans component import.
Importing the Trans component allows for richer interpolation and HTML element usage in translated strings, aligning well with the PR’s objectives of improving i18n.
24-28: Effective interpolation with <Trans> for dynamic text.
You're leveraging <Trans> to dynamically insert budgetType values. This approach is recommended for ensuring context-rich translations without string concatenation. The usage here appears correct.
44-44: Consistent approach to link translations.
Wrapping "Learn more" in <Trans> ensures a consistent i18n pattern across the UI. Great job on aligning link text with the new translation strategy.
48-53: Logical grouping within <Trans> block.
This chunk effectively demonstrates how to handle multiple sentences and inline tags (like <strong>) in a single translational context. This consolidated approach is more translator-friendly than splitting text segments.
59-59: Maintain consistent link translation usage.
Wrapping "Learn more" again with <Trans> is consistent with line 44. This consistency promotes clarity for translators and uniformity in the UI.
packages/desktop-client/src/components/reports/DateRange.tsx (1)
75-77: Fallback to unwrapped JSX is acceptable here.
By directly displaying the date range ({formattedStartDate} - {typeOrFormattedEndDate}) without <Trans>, it preserves simplicity for a scenario where translation placeholders are minimal. Ensure all necessary contexts remain translatable if requirements change in the future.
packages/desktop-client/src/components/settings/Reset.tsx (6)
2-2: Excellent switch to <Trans> component.
Switching from the t function to <Trans> fosters consistency with the rest of the i18n changes in the codebase and supports more complex inline translations as needed.
27-27: Replace button text with <Trans> - well done.
Utilizing <Trans> for button labels allows for flexible formatting and consistent translation handling across the app.
32-38: Ensure text chunk clarity for translators.
This larger text block includes HTML tags like <strong> and multiple sentences. Using <Trans> is optimal for providing translators with context, but ensure punctuation and line breaks are carefully placed to avoid confusion.
65-65: Maintaining a consistent translation approach for button text.
The Reset sync button text has been aligned with <Trans>, mirroring the pattern used for Reset budget cache. This consistency is beneficial for overall i18n clarity.
71-77: Large descriptive text block is effectively localized.
All text, including <strong> sections, is captured within <Trans>. This ensures translators can see the entire context. No issues found.
81-83: Conditional text translation is handled properly.
The fallback text for a disabled state is correctly included in <Trans>. This pattern ensures both states remain localized.
packages/desktop-client/src/components/settings/Themes.tsx (4)
2-2: Great addition of the Trans component import for richer translations
This change helps manage multi-element translations consistently.
62-62: Use of translation for "Theme"
Replacing hardcoded strings with t() is a recommended practice and increases localization consistency.
77-77: Use of translation for "Dark theme"
Likewise, using t() here ensures the UI supports multiple languages seamlessly.
96-98: HTML elements inside
Good practice mixing JSX elements (e.g., <strong>) within <Trans>. This provides a flexible approach for translators.
packages/desktop-client/src/components/modals/SimpleFinInitialiseModal.tsx (3)
3-3: Added Trans for advanced localization
Importing Trans alongside useTranslation better accommodates multi-line or complex translations.
69-81: Multi-line text translation
Using <Trans> here for instructional text is a solid approach, preserving context and formatting for translators.
109-109: Unified button text translation
Replacing hardcoded button text with <Trans> promotes consistent translation strings across the UI.
packages/desktop-client/src/components/settings/Encryption.tsx (14)
2-2: Importing Trans for more flexible text translation
This aligns well with the project’s broader initiative to improve i18n.
31-33: Button text with
Switching to <Trans> ensures better i18n coverage and potential for more complex translations.
38-38: Simple translated text
Adding <Trans> for the status message is straightforward and keeps text consistent across languages.
40-44: Paragraph text with HTML support
Using <Trans> here allows the translator to handle formatting and line breaks properly.
50-50: "Learn more" link translation
Including links within <Trans> is a great approach for preserving flexible translation while maintaining clickable elements.
56-60: Disabled button text
Even for disabled controls, using <Trans> fosters consistent localization.
63-68: Detailed explanation text
Successfully captures multi-sentence translation. Ensure translation keys convey full context to translators.
74-74: "Learn more" link
Again, good use of <Trans> for link text.
82-82: Enable encryption button text
Upgrading to <Trans> fosters consistency with other buttons.
87-93: Larger block of text
This thorough explanation benefits from <Trans> to avoid potential fragmentation for translators.
99-99: Link translation usage
Consistent style with your other link translations, which is good for clarity.
105-109: Disabled encryption button
Following the same localization pattern prevents overlooked strings in future translations.
112-116: Local encryption explanation
A clear multi-sentence message—this usage helps translators preserve context.
122-122: "Learn more" usage
Maintains a consistent user experience in line with the rest of the app.
packages/desktop-client/src/components/settings/Format.tsx (4)
3-3: Importing Trans
This is essential for advanced text handling, helping unify approach with the rest of the PR.
61-61: Ensuring each translation key is well-defined
While we see no immediate issue, consider verifying that the key 'Numbers' has a clear context in your i18n resource files.
[approve]
116-116: Checkbox label translation
Replacing a plain text label with <Trans> is beneficial for multi-language support.
142-145: Formatting explanation
Using <strong> within <Trans> clarifies important text across translations without losing emphasis.
packages/desktop-client/src/components/App.tsx (1)
137-137: **Use of consistent login expiration message **
This updated phrasing improves clarity and aligns with user-friendly best practices. Please confirm all references to token expiration utilize the same message for consistency.
packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsx (1)
166-174: **Excellent use of <Trans> for improved translatability **
By wrapping the entire text in <Trans> and separating the "Learn more" link, you preserve context for translators and ensure the link is localized. Great approach for consistent internationalization.
packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx (1)
230-234: **Clearer user messaging for duplicating budgets **
Using <Trans>Duplicate locally</Trans> vs. <Trans>Duplicate</Trans> affirms the distinction between local-based duplication and general duplication. This improves user comprehension.
packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx (7)
2-2: **Import both useTranslation and Trans **
Importing both for the same file is conventional in react-i18next and follows best practices for inline translation (<Trans>) and manual translation calls (t()).
22-35: **Well-structured hook for consistent “add account” options **
Creating a dedicated hook to encapsulate on/off-budget account labels is excellent for maintainability and ensures consistent internationalization across components.
53-54: **Custom hook usage is straightforward **
Pulling the new account options via useAddBudgetAccountOptions() keeps logic consistent and localized. Keep it up!
141-144: **Inline <Trans> block for instruction text **
Wrapping multi-line text in <Trans> ensures the translator sees the full context. This is a good practice for unified translation of contiguous text.
201-201: **Use of <Trans> for button text **
Replacing plain text with <Trans> is aligned with the goal of maximizing translation coverage.
218-219: **No duplication concerns for multiple hook calls **
The same hook is called in both the parent and child components, which is valid for scoping separate functionalities. The approach is fine.
265-265: **Maintaining consistent terminology **
“Remove bank-sync” is a clear directive. Confirm it matches the same style and phrasing used project-wide.
packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx (3)
49-49: Minor grammar improvement verified.
The adjusted text "Login expired, please log in again." is more consistent.
235-243: Clean integration of translation with explanatory text and link.
Using <Trans> around multi-sentence text plus a <Link> fosters robust i18n. This approach cleanly handles phrasing changes across locales, ensuring both the text and the link are translatable.
285-285: Great use of templated variables in translation.
This approach {{ count: selectedInst.items.size }} is recommended to simplify numeric placeholders for i18n.
packages/desktop-client/src/components/modals/CloseAccountModal.tsx (8)
4-4: Import of React i18n translation elements is correct.
Ensures the file can use <Trans> and the translation hook properly.
122-128: Enhanced i18n usage for closing an account.
Multiple <Trans> blocks now encapsulate dynamic text (account name, delete scenarios). This is a sound approach to maintaining context-rich translations.
Also applies to: 131-134, 138-141
154-161: Contextual balance explanation now properly wrapped for translation.
It’s valuable that these lines handle the dynamic balance within <Trans>. Keep verifying that all references to integerToCurrency are consistent with user’s locale.
192-192: Good practice for highlighting required fields with <FormError>.
This fosters clarity for users and avoids silent failures.
199-203: Purposeful explanation of on-budget to off-budget transfer.
The <Trans> usage for multi-line text is correct and keeps the logic straightforward.
231-233: Explicit form error message for missing category.
This ensures mandatory data is not overlooked.
243-260: Clear instructions for force close.
Adding <Trans> for the force-close warning is beneficial, helping to localize the entire explanation thoroughly, including the mention of potential budget changes.
278-278: Consistent usage of <Trans> in modal actions.
Replacing direct button text with <Trans> fosters unified i18n usage. Good job keeping CTA language clear.
Also applies to: 287-287
packages/desktop-client/src/components/modals/CreateAccountModal.tsx (6)
180-180: Titles dynamically set for adding vs. linking accounts.
Using two separate title variables for different usage contexts ensures clarity for end-users.
Also applies to: 185-185
212-212: Improved clarity in button text.
Swapping to "Create a local account" is more natural-sounding in English.
217-228: Well-structured multi-sentence <Trans> block.
The combination of bold text, link usage, and plain text is a best practice for robust translation.
291-297: Consistent usage to describe EU bank linking.
This chunk improves clarity for i18n, ensuring placeholders are minimal and the text is translatable in full.
Also applies to: 298-300
356-362: Parallel approach for North American bank linking.
Mirrors the structure from the EU bank block. Maintains uniform translation logic.
Also applies to: 363-365
396-396: Separate text blocks for bank sync setup explanations.
Explicit mention of connecting to an Actual server fosters user comprehension. Wrapping instructions in <Trans> is beneficial.
Also applies to: 399-409
packages/desktop-client/src/components/reports/Overview.tsx (1)
362-362: Improved markdown snippet for text widget additions.
Embedding translations in a markdown template is a practical approach; ensure the placeholders and emphasis are localized effectively.
packages/desktop-client/src/components/Modals.tsx (1)
305-305: Use of translation for button text:
Switching from a hardcoded string to t('Add') is an excellent step toward better internationalization.
packages/desktop-client/src/components/modals/EditRuleModal.jsx (16)
2-2: Import of Trans component:
Importing { useTranslation, Trans } aligns with the PR's goal of improving i18n. No issues found.
340-340: Payee label translation:
Great work using <Trans> to wrap the "Payee" label. This makes the text more flexible for translators.
349-349: Amount label translation:
Correct usage of <Trans>Amount</Trans>. This maintains consistency with other i18n changes.
353-354: "Next" label translation:
Wrapping "Next" with <Trans> properly localizes the label. No issues found.
531-536: Multiline translation block:
The <Trans> component handles multiline strings effectively. The static analysis warning about fragments appears to be a false positive here.
🧰 Tools
🪛 Biome (1.9.4)
[error] 531-538: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
711-711: "Add condition" label translation:
This change allows the condition label to be properly translated.
1053-1055: "Stage of rule" translation:
Replacing literal text with <Trans> ensures full i18n coverage.
1062-1062: "Pre" translation:
Wrapped in <Trans> for consistency. Looks good.
1068-1068: "Default" translation:
Correct usage of <Trans>Default</Trans>. No issues found.
1074-1074: "Post" translation:
Maintains consistency with the other stage labels.
1093-1106: Complex conditional text with :
This is a helpful approach for localizing strings that contain inline JSX elements.
1119-1119: "Then apply these actions" translation:
Straightforward replacement with <Trans>. Good job.
1127-1127: "Add action" translation:
Consistent with the rest of the i18n changes.
1249-1249: "This rule applies to these transactions" translation:
Properly localized statement describing the transactions under rule scope.
1257-1257: "Apply actions" translation:
Localization facilitates clear, user-friendly messaging.
1280-1280: "Save" translation:
This final UI element is correctly wrapped in <Trans>. No issues.
packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx
Outdated
Show resolved
Hide resolved
matt-fidd
force-pushed
the
translation-consistency
branch
from
8d411a3 to
115c474
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/desktop-client/src/components/settings/BudgetTypeSettings.tsx (3)
23-27: Consider adding type safety to the budgetType mappingThe translation implementation looks good! However, consider making the budgetType mapping more type-safe.
- {{ budgetType: budgetType === 'report' ? 'envelope' : 'tracking' }}{' '} + {{ budgetType: budgetType === 'report' ? 'envelope' : 'tracking' } as const}{' '}
43-43: Consider extracting "Learn more" as a reusable translation keyThe implementation is correct, but since "Learn more" is used multiple times, consider creating a shared translation key to ensure consistency across the application.
This could be implemented in your translation file as:
{ "common": { "learnMore": "Learn more" } }Then use it consistently across components:
- <Trans>Learn more</Trans> + <Trans i18nKey="common.learnMore">Learn more</Trans>Also applies to: 58-58
Line range hint
1-62: Great implementation of internationalization best practicesThe changes effectively improve translation string context by:
- Keeping related text together in single translation blocks
- Preserving HTML structure within translations
- Properly handling dynamic content interpolation
This approach will significantly help translators understand the context and produce more accurate translations.
packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx (1)
36-47: Consider memoizing the options arrays for better performance.The implementation of
useFrequencyOptionsanduseDayOfWeekOptionslooks good, but the options arrays are recreated on every render. Consider usinguseMemoto optimize performance.Here's how you can memoize the options:
function useFrequencyOptions() { const { t } = useTranslation(); - const FREQUENCY_OPTIONS = [ + const FREQUENCY_OPTIONS = useMemo(() => [ { id: 'daily', name: t('Days') }, { id: 'weekly', name: t('Weeks') }, { id: 'monthly', name: t('Months') }, { id: 'yearly', name: t('Years') }, - ] as const; + ] as const, [t]); return { FREQUENCY_OPTIONS }; }Apply the same pattern to
useDayOfWeekOptions.Also applies to: 51-65
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
upcoming-release-notes/4041.mdis excluded by!**/*.md
📒 Files selected for processing (42)
packages/desktop-client/src/components/App.tsx(1 hunks)packages/desktop-client/src/components/LoggedInUser.tsx(2 hunks)packages/desktop-client/src/components/Modals.tsx(1 hunks)packages/desktop-client/src/components/accounts/AccountSyncCheck.tsx(1 hunks)packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsx(1 hunks)packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx(3 hunks)packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx(1 hunks)packages/desktop-client/src/components/budget/IncomeHeader.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/ChangePassword.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/OpenIdForm.tsx(1 hunks)packages/desktop-client/src/components/modals/CloseAccountModal.tsx(8 hunks)packages/desktop-client/src/components/modals/CreateAccountModal.tsx(5 hunks)packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx(7 hunks)packages/desktop-client/src/components/modals/EditRuleModal.jsx(11 hunks)packages/desktop-client/src/components/modals/EditUser.tsx(2 hunks)packages/desktop-client/src/components/modals/GoCardlessExternalMsgModal.tsx(3 hunks)packages/desktop-client/src/components/modals/GoCardlessInitialiseModal.tsx(3 hunks)packages/desktop-client/src/components/modals/ImportTransactionsModal/ParsedDate.tsx(3 hunks)packages/desktop-client/src/components/modals/LoadBackupModal.tsx(2 hunks)packages/desktop-client/src/components/modals/ManageRulesModal.tsx(1 hunks)packages/desktop-client/src/components/modals/OpenIDEnableModal.tsx(1 hunks)packages/desktop-client/src/components/modals/PasswordEnableModal.tsx(2 hunks)packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx(8 hunks)packages/desktop-client/src/components/modals/SimpleFinInitialiseModal.tsx(3 hunks)packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx(2 hunks)packages/desktop-client/src/components/payees/PayeeMenu.tsx(3 hunks)packages/desktop-client/src/components/reports/DateRange.tsx(1 hunks)packages/desktop-client/src/components/reports/Header.tsx(2 hunks)packages/desktop-client/src/components/reports/Overview.tsx(1 hunks)packages/desktop-client/src/components/reports/SaveReportDelete.tsx(3 hunks)packages/desktop-client/src/components/reports/reports/NetWorth.tsx(1 hunks)packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx(5 hunks)packages/desktop-client/src/components/settings/BudgetTypeSettings.tsx(2 hunks)packages/desktop-client/src/components/settings/Encryption.tsx(2 hunks)packages/desktop-client/src/components/settings/Format.tsx(4 hunks)packages/desktop-client/src/components/settings/Reset.tsx(3 hunks)packages/desktop-client/src/components/settings/Themes.tsx(4 hunks)packages/desktop-client/src/components/settings/Upcoming.tsx(4 hunks)packages/desktop-client/src/components/settings/index.tsx(5 hunks)packages/desktop-client/src/components/sidebar/BudgetName.tsx(1 hunks)packages/loot-core/src/client/shared-listeners.ts(1 hunks)packages/loot-core/src/shared/errors.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (40)
- packages/desktop-client/src/components/budget/IncomeHeader.tsx
- packages/desktop-client/src/components/reports/Header.tsx
- packages/desktop-client/src/components/accounts/AccountSyncCheck.tsx
- packages/desktop-client/src/components/manager/subscribe/OpenIdForm.tsx
- packages/desktop-client/src/components/manager/subscribe/ChangePassword.tsx
- packages/desktop-client/src/components/sidebar/BudgetName.tsx
- packages/desktop-client/src/components/reports/Overview.tsx
- packages/desktop-client/src/components/modals/ManageRulesModal.tsx
- packages/desktop-client/src/components/modals/OpenIDEnableModal.tsx
- packages/desktop-client/src/components/settings/Encryption.tsx
- packages/desktop-client/src/components/reports/reports/NetWorth.tsx
- packages/desktop-client/src/components/modals/PasswordEnableModal.tsx
- packages/desktop-client/src/components/modals/ImportTransactionsModal/ParsedDate.tsx
- packages/desktop-client/src/components/modals/LoadBackupModal.tsx
- packages/desktop-client/src/components/reports/SaveReportDelete.tsx
- packages/desktop-client/src/components/modals/EditUser.tsx
- packages/desktop-client/src/components/settings/Upcoming.tsx
- packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsx
- packages/desktop-client/src/components/Modals.tsx
- packages/desktop-client/src/components/modals/GoCardlessExternalMsgModal.tsx
- packages/desktop-client/src/components/App.tsx
- packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx
- packages/desktop-client/src/components/modals/CloseAccountModal.tsx
- packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
- packages/desktop-client/src/components/settings/Themes.tsx
- packages/desktop-client/src/components/payees/PayeeMenu.tsx
- packages/desktop-client/src/components/modals/SimpleFinInitialiseModal.tsx
- packages/desktop-client/src/components/modals/CreateAccountModal.tsx
- packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx
- packages/desktop-client/src/components/modals/EditRuleModal.jsx
- packages/desktop-client/src/components/settings/Format.tsx
- packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx
- packages/loot-core/src/client/shared-listeners.ts
- packages/desktop-client/src/components/LoggedInUser.tsx
- packages/desktop-client/src/components/modals/GoCardlessInitialiseModal.tsx
- packages/desktop-client/src/components/settings/index.tsx
- packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx
- packages/loot-core/src/shared/errors.ts
- packages/desktop-client/src/components/settings/Reset.tsx
- packages/desktop-client/src/components/reports/DateRange.tsx
🔇 Additional comments (4)
packages/desktop-client/src/components/settings/BudgetTypeSettings.tsx (3)
2-2: LGTM! Import changes align with internationalization improvements
The switch from useTranslation to Trans import aligns with the PR objective to improve translation string context.
32-37: LGTM! Well-structured translation block
The envelope budgeting description provides excellent context for translators with proper HTML structure within the Trans component.
47-52: LGTM! Consistent with envelope budgeting section
The tracking budgeting description maintains consistency in structure and provides clear context for translators.
packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx (1)
286-287: LGTM! Good use of the new hook for internationalization.
The integration of useDayOfWeekOptions hook improves the translation capabilities while maintaining the existing functionality.
packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx
Outdated
Show resolved
Hide resolved
|
/update-vrt |
matt-fidd
force-pushed
the
translation-consistency
branch
from
409f0b3 to
a84a907
Compare
packages/desktop-client/src/components/modals/CloseAccountModal.tsx
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/modals/CloseAccountModal.tsx
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/settings/BudgetTypeSettings.tsx
Outdated
Show resolved
Hide resolved
| htmlFor="solve_dropdown" | ||
| style={{ userSelect: 'none', marginLeft: 5 }} | ||
| > | ||
| <Trans> {{ beforeOrAfter: '' }} weekend</Trans> |
There was a problem hiding this comment.
Clever! I wonder if there's some way we can generalise this, I think there a lot of places in the app where we have an English sentence with dynamic portions
There was a problem hiding this comment.
Maybe we could have some sort of inverted pattern for a Select where we do
<TransSelect options={[
<Trans>Move schedule <Item>before</Item> weekend</Trans>,
<Trans>Move schedule <Item>after</Item> weekend,
]}/>The rationale with them being separate strings is this expands better to more options, and I'm not sure if there are languages where the position of the word could vary depending on the word.
But just food for thought! We don't have to implement this now
packages/desktop-client/src/components/modals/ImportTransactionsModal/ParsedDate.tsx
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx
Show resolved
Hide resolved
| const selectedPayeeNames = [...selectedPayees] | ||
| .slice(0, 4) | ||
| .map(id => payeesById[id].name) | ||
| .join(', '); |
There was a problem hiding this comment.
One more quick suggestion: we have a useFormatList hook that is perfect for this type of thing! Maybe we can even adapt that hook to take an optional max # and auto-abbreviate with "and more" once the limit is reached
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx (1)
Error handling needs improvement for account linking operations
Based on the code analysis, the account linking operations in
SelectLinkedAccountsModal.jsxlack proper error handling for several critical scenarios:
- The
unlinkAccountaction dispatches errors through notifications but the component doesn't handle potential failures during bulk unlinking operations.- The
linkAccountandlinkAccountSimpleFinoperations are executed without try-catch blocks to handle potential failures.- No error handling exists for network failures or invalid states during the linking process.
Required changes:
- Add try-catch blocks around the account linking/unlinking operations
- Handle potential failures during bulk operations
- Add error notifications for failed operations
- Consider adding rollback functionality for partially completed bulk operations
Example fix:
try { // Unlink accounts await Promise.all( localAccounts .filter(acc => acc.account_id) .filter(acc => !chosenLocalAccountIds.includes(acc.id)) .map(acc => dispatch(unlinkAccount(acc.id))) ); // Link new accounts await Promise.all( Object.entries(chosenAccounts).map(async ([chosenExternalAccountId, chosenLocalAccountId]) => { const externalAccount = externalAccounts.find( account => account.account_id === chosenExternalAccountId ); if (!externalAccount) return; const offBudget = chosenLocalAccountId === addOffBudgetAccountOption.id; try { if (syncSource === 'simpleFin') { await dispatch(linkAccountSimpleFin(/*...args*/)); } else { await dispatch(linkAccount(/*...args*/)); } } catch (error) { dispatch(addNotification({ type: 'error', message: `Failed to link account: ${error.message}` })); } }) ); } catch (error) { dispatch(addNotification({ type: 'error', message: 'Failed to update account links. Some operations may not have completed.' })); }🔗 Analysis chain
Line range hint
57-93: Verify error handling for account linking operations.The account linking logic handles multiple scenarios but should be verified for proper error handling, especially for:
- Network failures during linking/unlinking
- Invalid account states
- Concurrent modifications
Let's verify the error handling implementation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for error handling patterns in account linking logic echo "Searching for error handling in account linking..." rg -A 5 "catch|error|fail" --glob "packages/desktop-client/src/**/*.{js,jsx,ts,tsx}" # Search for tests covering error scenarios echo "Searching for related test files..." fd -e test.js -e test.jsx -e test.ts -e test.tsx . packages/desktop-client/src | xargs rg "account.*link|link.*account"Length of output: 69575
Script:
#!/bin/bash # Search for error handling in account linking actions and reducers echo "Searching for account linking actions and error handling..." rg -A 5 "linkAccount|unlinkAccount" --glob "packages/desktop-client/src/store/**/*.{js,jsx,ts,tsx}" # Search for account linking error handling in the component echo "Searching for error handling in the component..." rg -A 5 "catch|try|error" packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsxLength of output: 590
Script:
#!/bin/bash # Search for account linking actions in the entire codebase echo "Searching for account linking actions..." rg -A 10 "linkAccount|unlinkAccount" --type js --type jsx --type ts --type tsx # Search for error handling in the component and its imports echo "Searching for error handling in the component and related files..." rg -A 5 "addNotification|dispatch.*error" --type js --type jsx --type ts --type tsxLength of output: 456
Script:
#!/bin/bash # Search for account linking actions in the entire codebase echo "Searching for account linking actions..." rg -A 10 "linkAccount|unlinkAccount" # Search for error handling in the component and related files echo "Searching for error handling in the component and related files..." rg -A 5 "addNotification|dispatch.*error"Length of output: 61212
🧹 Nitpick comments (5)
packages/desktop-client/src/components/settings/Upcoming.tsx (2)
17-31: LGTM: Well-structured hook with proper translations.The hook effectively manages the translation of time period options while maintaining type safety.
Consider extracting the options array to a constant and mapping translations:
function useUpcomingLengthOptions() { const { t } = useTranslation(); + + const LENGTH_OPTIONS = [ + { value: '1', key: '1 day' }, + { value: '7', key: '1 week' }, + { value: '14', key: '2 weeks' }, + { value: '30', key: '1 month' }, + ] as const; const upcomingLengthOptions: { value: SyncedPrefs['upcomingScheduledTransactionLength']; label: string; - }[] = [ - { value: '1', label: t('1 day') }, - { value: '7', label: t('1 week') }, - { value: '14', label: t('2 weeks') }, - { value: '30', label: t('1 month') }, - ]; + }[] = LENGTH_OPTIONS.map(({ value, key }) => ({ + value, + label: t(key), + })); return { upcomingLengthOptions }; }This makes it easier to maintain the options list and keeps the translation logic separate.
96-99: Consider making the default value handling more explicit.The fallback logic for the upcoming length label could be more clearly expressed.
- {upcomingLengthOptions.find(x => x.value === upcomingLength)?.label ?? - t('1 week')} + {upcomingLengthOptions.find(x => x.value === upcomingLength)?.label || + upcomingLengthOptions.find(x => x.value === '7')?.label || + t('1 week')}packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx (3)
2-2: Consider enhancing the translation context and hook organization.The hook implementation looks good, but there are a few suggestions for improvement:
- Consider adding translation context using
t('Create new account', { context: 'on-budget' })to help translators understand the difference between the two options.- Consider moving this hook to a separate file (e.g.,
hooks/useAccountOptions.js) if it might be reused elsewhere.function useAddBudgetAccountOptions() { const { t } = useTranslation(); const addOnBudgetAccountOption = { id: 'new-on', - name: t('Create new account'), + name: t('Create new account', { context: 'on-budget' }), }; const addOffBudgetAccountOption = { id: 'new-off', - name: t('Create new account (off budget)'), + name: t('Create new account', { context: 'off-budget' }), };Also applies to: 22-35
141-144: Consider improving translator experience for multi-line text.While the
Transcomponent usage is correct, consider making the text more translator-friendly by keeping it on a single line. This helps prevent accidental line breaks in translations.- <Trans> - We found the following accounts. Select which ones you want to - add: - </Trans> + <Trans>We found the following accounts. Select which ones you want to add:</Trans>
264-264: Consider adding context to button translations.The button text translations could benefit from additional context for translators. Consider using translation keys that provide more context about where and when these buttons appear.
- <Trans>Remove bank-sync</Trans> + <Trans context="account-linking-modal">Remove bank synchronization</Trans> - <Trans>Set up bank-sync</Trans> + <Trans context="account-linking-modal">Set up bank synchronization</Trans>Also applies to: 274-274
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
packages/desktop-client/e2e/reports.test.js-snapshots/Reports-loads-cash-flow-graph-and-checks-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-loads-cash-flow-graph-and-checks-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-loads-cash-flow-graph-and-checks-visuals-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-loads-net-worth-graph-and-checks-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-loads-net-worth-graph-and-checks-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-loads-net-worth-graph-and-checks-visuals-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/settings.test.js-snapshots/Settings-checks-the-page-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.test.js-snapshots/Settings-checks-the-page-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.test.js-snapshots/Settings-checks-the-page-visuals-3-chromium-linux.pngis excluded by!**/*.pngupcoming-release-notes/4041.mdis excluded by!**/*.md
📒 Files selected for processing (45)
packages/desktop-client/e2e/page-models/navigation.js(1 hunks)packages/desktop-client/src/components/App.tsx(1 hunks)packages/desktop-client/src/components/LoggedInUser.tsx(2 hunks)packages/desktop-client/src/components/Modals.tsx(1 hunks)packages/desktop-client/src/components/accounts/AccountSyncCheck.tsx(1 hunks)packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsx(1 hunks)packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx(3 hunks)packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx(1 hunks)packages/desktop-client/src/components/budget/IncomeHeader.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/ChangePassword.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/OpenIdForm.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/common.tsx(1 hunks)packages/desktop-client/src/components/modals/CloseAccountModal.tsx(8 hunks)packages/desktop-client/src/components/modals/CreateAccountModal.tsx(5 hunks)packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx(6 hunks)packages/desktop-client/src/components/modals/EditRuleModal.jsx(11 hunks)packages/desktop-client/src/components/modals/EditUser.tsx(2 hunks)packages/desktop-client/src/components/modals/GoCardlessExternalMsgModal.tsx(3 hunks)packages/desktop-client/src/components/modals/GoCardlessInitialiseModal.tsx(3 hunks)packages/desktop-client/src/components/modals/GoalTemplateModal.tsx(6 hunks)packages/desktop-client/src/components/modals/ImportTransactionsModal/ParsedDate.tsx(3 hunks)packages/desktop-client/src/components/modals/LoadBackupModal.tsx(2 hunks)packages/desktop-client/src/components/modals/ManageRulesModal.tsx(1 hunks)packages/desktop-client/src/components/modals/OpenIDEnableModal.tsx(1 hunks)packages/desktop-client/src/components/modals/PasswordEnableModal.tsx(2 hunks)packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx(8 hunks)packages/desktop-client/src/components/modals/SimpleFinInitialiseModal.tsx(3 hunks)packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx(2 hunks)packages/desktop-client/src/components/payees/PayeeMenu.tsx(3 hunks)packages/desktop-client/src/components/reports/DateRange.tsx(1 hunks)packages/desktop-client/src/components/reports/Header.tsx(2 hunks)packages/desktop-client/src/components/reports/Overview.tsx(1 hunks)packages/desktop-client/src/components/reports/SaveReportDelete.tsx(3 hunks)packages/desktop-client/src/components/reports/reports/NetWorth.tsx(1 hunks)packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx(5 hunks)packages/desktop-client/src/components/settings/BudgetTypeSettings.tsx(2 hunks)packages/desktop-client/src/components/settings/Encryption.tsx(2 hunks)packages/desktop-client/src/components/settings/Format.tsx(4 hunks)packages/desktop-client/src/components/settings/Reset.tsx(3 hunks)packages/desktop-client/src/components/settings/Themes.tsx(4 hunks)packages/desktop-client/src/components/settings/Upcoming.tsx(4 hunks)packages/desktop-client/src/components/settings/index.tsx(5 hunks)packages/desktop-client/src/components/sidebar/BudgetName.tsx(1 hunks)packages/loot-core/src/client/shared-listeners.ts(1 hunks)packages/loot-core/src/shared/errors.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (42)
- packages/desktop-client/src/components/budget/IncomeHeader.tsx
- packages/desktop-client/src/components/reports/Header.tsx
- packages/desktop-client/src/components/modals/ManageRulesModal.tsx
- packages/desktop-client/src/components/manager/subscribe/OpenIdForm.tsx
- packages/desktop-client/src/components/sidebar/BudgetName.tsx
- packages/desktop-client/src/components/modals/EditUser.tsx
- packages/desktop-client/src/components/modals/OpenIDEnableModal.tsx
- packages/desktop-client/src/components/manager/subscribe/common.tsx
- packages/desktop-client/src/components/reports/reports/NetWorth.tsx
- packages/desktop-client/src/components/manager/subscribe/ChangePassword.tsx
- packages/desktop-client/src/components/reports/Overview.tsx
- packages/desktop-client/e2e/page-models/navigation.js
- packages/desktop-client/src/components/settings/Themes.tsx
- packages/desktop-client/src/components/modals/PasswordEnableModal.tsx
- packages/desktop-client/src/components/reports/SaveReportDelete.tsx
- packages/desktop-client/src/components/accounts/AccountSyncCheck.tsx
- packages/desktop-client/src/components/App.tsx
- packages/desktop-client/src/components/LoggedInUser.tsx
- packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx
- packages/desktop-client/src/components/modals/CloseAccountModal.tsx
- packages/loot-core/src/client/shared-listeners.ts
- packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
- packages/desktop-client/src/components/modals/GoCardlessExternalMsgModal.tsx
- packages/desktop-client/src/components/modals/LoadBackupModal.tsx
- packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsx
- packages/desktop-client/src/components/Modals.tsx
- packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx
- packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx
- packages/desktop-client/src/components/modals/EditRuleModal.jsx
- packages/desktop-client/src/components/modals/GoalTemplateModal.tsx
- packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx
- packages/desktop-client/src/components/settings/Format.tsx
- packages/desktop-client/src/components/reports/DateRange.tsx
- packages/desktop-client/src/components/settings/index.tsx
- packages/desktop-client/src/components/payees/PayeeMenu.tsx
- packages/desktop-client/src/components/modals/GoCardlessInitialiseModal.tsx
- packages/desktop-client/src/components/modals/SimpleFinInitialiseModal.tsx
- packages/desktop-client/src/components/settings/Reset.tsx
- packages/desktop-client/src/components/settings/Encryption.tsx
- packages/loot-core/src/shared/errors.ts
- packages/desktop-client/src/components/settings/BudgetTypeSettings.tsx
- packages/desktop-client/src/components/modals/CreateAccountModal.tsx
🧰 Additional context used
🪛 GitHub Check: lint
packages/desktop-client/src/components/modals/ImportTransactionsModal/ParsedDate.tsx
[warning] 20-20:
't' is assigned a value but never used. Allowed unused vars must match /^(_|React)/u
🔇 Additional comments (4)
packages/desktop-client/src/components/modals/ImportTransactionsModal/ParsedDate.tsx (1)
33-33: LGTM! Good i18n improvements
The changes properly wrap the translatable strings with the Trans component, which will provide better context for translators. This aligns well with the PR's objective to improve translation strings.
Note: This addresses the previous review comment suggesting the use of Trans.
Also applies to: 39-39
packages/desktop-client/src/components/settings/Upcoming.tsx (2)
2-2: LGTM: Import changes align with internationalization goals.
The addition of useTranslation and Trans imports from react-i18next is appropriate for the internationalization improvements.
76-79: LGTM: Proper usage of Trans component for complex text.
The Trans component is correctly used to handle text with embedded strong tags while maintaining translation context.
packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx (1)
53-54: LGTM! Consistent hook usage across components.
The hook is properly integrated in both the modal and table row components, improving code maintainability by centralizing the account option definitions.
Also applies to: 217-218
packages/desktop-client/src/components/modals/ImportTransactionsModal/ParsedDate.tsx
Outdated
Show resolved
Hide resolved
| @@ -27,13 +28,13 @@ export function ParsedDate({ | |||
| <Text> | |||
| {date || ( | |||
| <Text style={{ color: theme.pageTextLight, fontStyle: 'italic' }}> | |||
| Empty | |||
| <Trans>Empty</Trans> | |||
There was a problem hiding this comment.
It would maybe be nice to add context on what is empty here via <Trans context="...">. But ff to defer to another PR
Co-authored-by: Julian Dominguez-Schatz <julian.dominguezschatz@gmail.com>
matt-fidd
force-pushed
the
translation-consistency
branch
from
d56d051 to
30bf0c6
Compare
|
@jfdoming WRT nested Trans, it looks like it was actually causing duplication in the locale file. Found a way to fix it and appease Typescript in i18next/react-i18next#1483 (comment) diff en_before.json /home/matt/actual/packages/desktop-client/locale/en.json
2d1
< " {{beforeOrAfter}} weekend": " {{beforeOrAfter}} weekend",
9,10d7
< "{{accountName}}": "{{accountName}}",
< "{{balance}}": "{{balance}}",
20d16
< "{{name}}": "{{name}}",
27c23
< "<0>Are you sure you want to delete report: </0><1><0>{{name}}</0></1>": "<0>Are you sure you want to delete report: </0><1><0>{{name}}</0></1>",
---
> "<0>Are you sure you want to delete report: </0><1>{{name}}</1>": "<0>Are you sure you want to delete report: </0><1>{{name}}</1>",
41c37
< "<0>Move schedule </0><1></1><2><0> {{beforeOrAfter}} weekend</0></2>": "<0>Move schedule </0><1></1><2><0> {{beforeOrAfter}} weekend</0></2>",
---
> "<0>Move schedule </0><1></1><2> {{beforeOrAfter}} weekend</2>": "<0>Move schedule </0><1></1><2> {{beforeOrAfter}} weekend</2>",
130c126
< "Are you sure you want to close <2><0>{{accountName}}</0></2>? ": "Are you sure you want to close <2><0>{{accountName}}</0></2>? ",
---
> "Are you sure you want to close <2>{{accountName}}</2>? ": "Are you sure you want to close <2>{{accountName}}</2>? ",
549c545,546
< "logged in as:": "logged in as:",
---
> "logged in as: <1>{{userName}}</1>": "logged in as: <1>{{userName}}</1>",
> "logged in as: <2><0>{{userName}}</0></2>": "logged in as: <2><0>{{userName}}</0></2>",
938c935
< "This account has a balance of <2><0>{{balance}}</0></2>. To close this account, select a different account to transfer this balance to:": "This account has a balance of <2><0>{{balance}}</0></2>. To close this account, select a different account to transfer this balance to:",
---
> "This account has a balance of <2>{{balance}}</2>. To close this account, select a different account to transfer this balance to:": "This account has a balance of <2>{{balance}}</2>. To close this account, select a different account to transfer this balance to:",Fixed in 30bf0c6 |
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
packages/desktop-client/src/components/reports/SaveReportDelete.tsx (1)
Inconsistent confirmation dialog patterns need alignment
Based on the verification results, there are inconsistencies in how deletion confirmations are implemented across the codebase:
SaveReportDelete.tsxuses:
- Yes/No buttons
- Trans component directly wrapping the confirmation text
- Inline styling for error text
ConfirmTransactionDeleteModal.tsxuses:
- Delete/Cancel buttons
- Translation through t() function
- Proper component composition with Modal, ModalHeader
- Responsive button styling
ConfirmCategoryDeleteModal.tsxuses:
- Only a Delete button with different flow
- Translation through t() function
- More complex UI with category transfer
Recommendations:
- Align button labels across confirmation dialogs (either Yes/No or Delete/Cancel)
- Standardize translation approach (prefer t() function over direct Trans wrapper)
- Use consistent modal composition pattern from ConfirmTransactionDeleteModal
- Extract common confirmation dialog styling to reduce duplication
🔗 Analysis chain
Line range hint
1-46: Verify consistent Trans component usage patternLet's verify if similar deletion confirmations across the codebase follow the same pattern to ensure consistency.
Let me gather more information about the specific usage patterns in these files.
Let me check one more pattern to see how the Yes/No buttons are implemented in these confirmation dialogs.
Let me check the actual implementation of these delete modals to compare their patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar deletion confirmation patterns ast-grep --pattern 'Are you sure you want to delete' # Look for other Yes/No translation patterns rg -l "Trans.*Yes" packages/desktop-client/src/components rg -l "Trans.*No" packages/desktop-client/src/componentsLength of output: 823
Script:
#!/bin/bash # Check the specific usage patterns of Trans with Yes/No in these files rg -A 2 -B 2 "Trans.*Yes|Trans.*No" packages/desktop-client/src/components/accounts/Account.tsx packages/desktop-client/src/components/accounts/Header.tsx packages/desktop-client/src/components/modals/EditAccess.tsx packages/desktop-client/src/components/manager/BudgetList.tsx # Look for similar deletion confirmation dialogs rg -A 5 -B 5 "Are you sure.*delete" packages/desktop-client/src/componentsLength of output: 6963
Script:
#!/bin/bash # Check button implementations in confirmation dialogs rg -A 10 "variant=\"primary\".*Yes|variant=\"primary\".*No" packages/desktop-client/src/components/modals/ConfirmCategoryDeleteModal.tsx packages/desktop-client/src/components/modals/ConfirmTransactionDeleteModal.tsx # Also check for any other deletion confirmation components fd -e tsx -e jsx DeleteModal packages/desktop-client/src/componentsLength of output: 441
Script:
#!/bin/bash # Get the full content of the delete confirmation modals cat packages/desktop-client/src/components/modals/ConfirmCategoryDeleteModal.tsx packages/desktop-client/src/components/modals/ConfirmTransactionDeleteModal.tsx # Also check for any other components using similar button patterns rg -A 5 "Button.*variant=\"primary\".*onPress" packages/desktop-client/src/components/modals/Length of output: 8892
🧰 Tools
🪛 GitHub Check: lint
[failure] 28-28:
Unexpected any. Specify a different type
🧹 Nitpick comments (7)
packages/desktop-client/src/components/settings/Upcoming.tsx (3)
17-31: Consider memoizing the options array.The implementation looks good, but since the options array is recreated on every render, consider using
useMemoto optimize performance.function useUpcomingLengthOptions() { const { t } = useTranslation(); - const upcomingLengthOptions: { + const upcomingLengthOptions = React.useMemo(() => { + const options: { value: SyncedPrefs['upcomingScheduledTransactionLength']; label: string; - }[] = [ + }[] = [ { value: '1', label: t('1 day') }, { value: '7', label: t('1 week') }, { value: '14', label: t('2 weeks') }, { value: '30', label: t('1 month') }, - ]; + ]; + return options; + }, [t]); return { upcomingLengthOptions }; }
96-99: Simplify the button label construction.The current implementation could be simplified and made more robust by moving the interpolation logic into a translation key.
- <Trans>Edit Upcoming Length</Trans> ( - {upcomingLengthOptions.find(x => x.value === upcomingLength)?.label ?? - t('1 week')} - ) + {t('Edit Upcoming Length ({{length}})', { + length: upcomingLengthOptions.find(x => x.value === upcomingLength)?.label ?? t('1 week') + })}
Line range hint
17-99: Well-structured internationalization implementation.The overall approach to internationalization in this component is well-thought-out:
- Separation of translation logic into a custom hook
- Consistent use of translation functions
- Proper handling of complex text with Trans component
- Type-safe implementation
This provides a good pattern for similar internationalization work in other components.
packages/desktop-client/src/components/modals/CloseAccountModal.tsx (1)
240-257: Consider enhancing warning visibilityWhile the translation is well-structured, the warning about permanent data deletion could be more prominent.
Consider wrapping the warning in a dedicated warning component:
-<Text style={{ fontSize: 12 }}> +<WarningText> <Trans> You can also{' '} <Link variant="text" onClick={() => { setLoading(true); dispatch(forceCloseAccount(account.id)); close(); }} style={{ color: theme.errorText }} > force close </Link>{' '} the account which will delete it and all its transactions permanently. Doing so may change your budget unexpectedly since money in it may vanish. </Trans> -</Text> +</WarningText>packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx (2)
36-47: Consider memoizing the options arrays for better performance.The frequency and day-of-week options arrays are recreated on every render. Consider using
useMemoto optimize performance:function useFrequencyOptions() { const { t } = useTranslation(); - const FREQUENCY_OPTIONS = [ + const FREQUENCY_OPTIONS = useMemo(() => [ { id: 'daily', name: t('Days') }, { id: 'weekly', name: t('Weeks') }, { id: 'monthly', name: t('Months') }, { id: 'yearly', name: t('Years') }, - ] as const; + ] as const, [t]); return { FREQUENCY_OPTIONS }; }Apply the same pattern to
useDayOfWeekOptions.Also applies to: 51-65
518-545: Consider improving the translation object literal.The empty string in the object literal could be replaced with the actual value from the select component for better clarity in translations:
-{{ beforeOrAfter: '' } as any} weekend +{{ beforeOrAfter: state.config.weekendSolveMode }} weekendThis way, translators can see the actual value that will be inserted.
🧰 Tools
🪛 GitHub Check: lint
[failure] 543-543:
Unexpected any. Specify a different typepackages/desktop-client/src/components/reports/SaveReportDelete.tsx (1)
40-43: Consider adding translation context for Yes/No buttonsWhile the Trans components are correctly implemented, these generic Yes/No translations might benefit from additional context for translators.
Consider using translation keys that provide context:
- <Trans>Yes</Trans> + <Trans i18nKey="confirmation.delete.yes">Yes</Trans> - <Trans>No</Trans> + <Trans i18nKey="confirmation.delete.no">No</Trans>This helps translators understand these are confirmation buttons for deletion, which might affect the translation in some languages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
packages/desktop-client/e2e/reports.test.js-snapshots/Reports-loads-cash-flow-graph-and-checks-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-loads-cash-flow-graph-and-checks-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-loads-cash-flow-graph-and-checks-visuals-3-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-loads-net-worth-graph-and-checks-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-loads-net-worth-graph-and-checks-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/reports.test.js-snapshots/Reports-loads-net-worth-graph-and-checks-visuals-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/settings.test.js-snapshots/Settings-checks-the-page-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.test.js-snapshots/Settings-checks-the-page-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.test.js-snapshots/Settings-checks-the-page-visuals-3-chromium-linux.pngis excluded by!**/*.pngupcoming-release-notes/4041.mdis excluded by!**/*.md
📒 Files selected for processing (45)
packages/desktop-client/e2e/page-models/navigation.js(1 hunks)packages/desktop-client/src/components/App.tsx(1 hunks)packages/desktop-client/src/components/LoggedInUser.tsx(2 hunks)packages/desktop-client/src/components/Modals.tsx(1 hunks)packages/desktop-client/src/components/accounts/AccountSyncCheck.tsx(1 hunks)packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsx(1 hunks)packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx(3 hunks)packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx(1 hunks)packages/desktop-client/src/components/budget/IncomeHeader.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/ChangePassword.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/OpenIdForm.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/common.tsx(1 hunks)packages/desktop-client/src/components/modals/CloseAccountModal.tsx(8 hunks)packages/desktop-client/src/components/modals/CreateAccountModal.tsx(5 hunks)packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx(6 hunks)packages/desktop-client/src/components/modals/EditRuleModal.jsx(11 hunks)packages/desktop-client/src/components/modals/EditUser.tsx(2 hunks)packages/desktop-client/src/components/modals/GoCardlessExternalMsgModal.tsx(3 hunks)packages/desktop-client/src/components/modals/GoCardlessInitialiseModal.tsx(3 hunks)packages/desktop-client/src/components/modals/GoalTemplateModal.tsx(6 hunks)packages/desktop-client/src/components/modals/ImportTransactionsModal/ParsedDate.tsx(2 hunks)packages/desktop-client/src/components/modals/LoadBackupModal.tsx(2 hunks)packages/desktop-client/src/components/modals/ManageRulesModal.tsx(1 hunks)packages/desktop-client/src/components/modals/OpenIDEnableModal.tsx(1 hunks)packages/desktop-client/src/components/modals/PasswordEnableModal.tsx(2 hunks)packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx(8 hunks)packages/desktop-client/src/components/modals/SimpleFinInitialiseModal.tsx(3 hunks)packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx(2 hunks)packages/desktop-client/src/components/payees/PayeeMenu.tsx(3 hunks)packages/desktop-client/src/components/reports/DateRange.tsx(1 hunks)packages/desktop-client/src/components/reports/Header.tsx(2 hunks)packages/desktop-client/src/components/reports/Overview.tsx(1 hunks)packages/desktop-client/src/components/reports/SaveReportDelete.tsx(3 hunks)packages/desktop-client/src/components/reports/reports/NetWorth.tsx(1 hunks)packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx(5 hunks)packages/desktop-client/src/components/settings/BudgetTypeSettings.tsx(2 hunks)packages/desktop-client/src/components/settings/Encryption.tsx(2 hunks)packages/desktop-client/src/components/settings/Format.tsx(4 hunks)packages/desktop-client/src/components/settings/Reset.tsx(3 hunks)packages/desktop-client/src/components/settings/Themes.tsx(4 hunks)packages/desktop-client/src/components/settings/Upcoming.tsx(4 hunks)packages/desktop-client/src/components/settings/index.tsx(5 hunks)packages/desktop-client/src/components/sidebar/BudgetName.tsx(1 hunks)packages/loot-core/src/client/shared-listeners.ts(1 hunks)packages/loot-core/src/shared/errors.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (40)
- packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
- packages/desktop-client/src/components/manager/subscribe/common.tsx
- packages/loot-core/src/shared/errors.ts
- packages/desktop-client/src/components/modals/EditUser.tsx
- packages/desktop-client/e2e/page-models/navigation.js
- packages/desktop-client/src/components/reports/Header.tsx
- packages/desktop-client/src/components/manager/subscribe/ChangePassword.tsx
- packages/desktop-client/src/components/settings/Encryption.tsx
- packages/desktop-client/src/components/modals/ManageRulesModal.tsx
- packages/desktop-client/src/components/modals/OpenIDEnableModal.tsx
- packages/desktop-client/src/components/reports/reports/NetWorth.tsx
- packages/desktop-client/src/components/accounts/AccountSyncCheck.tsx
- packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx
- packages/desktop-client/src/components/settings/Format.tsx
- packages/desktop-client/src/components/reports/Overview.tsx
- packages/desktop-client/src/components/manager/subscribe/OpenIdForm.tsx
- packages/desktop-client/src/components/modals/LoadBackupModal.tsx
- packages/desktop-client/src/components/App.tsx
- packages/desktop-client/src/components/sidebar/BudgetName.tsx
- packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx
- packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsx
- packages/desktop-client/src/components/modals/SimpleFinInitialiseModal.tsx
- packages/desktop-client/src/components/modals/ImportTransactionsModal/ParsedDate.tsx
- packages/loot-core/src/client/shared-listeners.ts
- packages/desktop-client/src/components/Modals.tsx
- packages/desktop-client/src/components/modals/PasswordEnableModal.tsx
- packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx
- packages/desktop-client/src/components/settings/Themes.tsx
- packages/desktop-client/src/components/settings/BudgetTypeSettings.tsx
- packages/desktop-client/src/components/modals/CreateAccountModal.tsx
- packages/desktop-client/src/components/modals/GoalTemplateModal.tsx
- packages/desktop-client/src/components/modals/GoCardlessInitialiseModal.tsx
- packages/desktop-client/src/components/settings/index.tsx
- packages/desktop-client/src/components/modals/GoCardlessExternalMsgModal.tsx
- packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx
- packages/desktop-client/src/components/settings/Reset.tsx
- packages/desktop-client/src/components/modals/EditRuleModal.jsx
- packages/desktop-client/src/components/reports/DateRange.tsx
- packages/desktop-client/src/components/budget/IncomeHeader.tsx
- packages/desktop-client/src/components/payees/PayeeMenu.tsx
🧰 Additional context used
📓 Learnings (1)
packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx (1)
Learnt from: matt-fidd
PR: actualbudget/actual#4041
File: packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx:518-544
Timestamp: 2024-12-26T01:16:39.339Z
Learning: A <Trans> inside another <Trans> does not necessarily produce two distinct strings. Instead, it merges them into a single translation entry, allowing for object literals to be used within the same translation string.
🪛 GitHub Check: lint
packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx
[failure] 543-543:
Unexpected any. Specify a different type
packages/desktop-client/src/components/LoggedInUser.tsx
[failure] 211-211:
Unexpected any. Specify a different type
[failure] 227-227:
Unexpected any. Specify a different type
packages/desktop-client/src/components/modals/CloseAccountModal.tsx
[failure] 124-124:
Unexpected any. Specify a different type
[failure] 154-154:
Unexpected any. Specify a different type
packages/desktop-client/src/components/reports/SaveReportDelete.tsx
[failure] 28-28:
Unexpected any. Specify a different type
🔇 Additional comments (8)
packages/desktop-client/src/components/settings/Upcoming.tsx (3)
2-2: LGTM! Import changes align with internationalization goals.
82-83: Reminder: Translate aria-labels in follow-up PR.As discussed in previous reviews, the aria-labels still need to be translated. This was intentionally deferred to a follow-up PR.
Also applies to: 91-93
76-79: LGTM! Good use of Trans component for complex text.The Trans component is correctly used here to handle text with embedded strong tags while maintaining proper translation context.
packages/desktop-client/src/components/modals/CloseAccountModal.tsx (2)
4-4: LGTM: Import change aligns with translation improvementsThe addition of the
Transcomponent import alongsideuseTranslationsupports the PR's objective of improving translation string handling.
189-189: LGTM: Consistent translation handling for UI textError messages and button text are properly wrapped in
Transcomponents, maintaining consistency across the UI.Also applies to: 229-229, 275-275, 284-284
packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx (2)
286-287: LGTM! Good use of the new hook for localization.The integration of
useDayOfWeekOptionsproperly supports the translation of day names.
371-372: LGTM! Good use of the new hook for localization.The integration of
useFrequencyOptionsproperly supports the translation of frequency options.packages/desktop-client/src/components/reports/SaveReportDelete.tsx (1)
2-2: LGTM: Clean import of Trans componentThe addition of the Trans component import aligns with the PR's objective to improve translation string handling.
packages/desktop-client/src/components/modals/CloseAccountModal.tsx
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/modals/CloseAccountModal.tsx
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/reports/SaveReportDelete.tsx
Outdated
Show resolved
Hide resolved
matt-fidd
force-pushed
the
translation-consistency
branch
from
b8d638f to
519926b
Compare
|
Okay, two ways to quieten down the linter... 1 (not a fan): {/* eslint-disable @typescript-eslint/no-explicit-any */}
<span>{{ userName: userData?.displayName } as any}</span>
{/* eslint-enable @typescript-eslint/no-explicit-any */}
<span>{{ userName: userData?.displayName } as TransObjectLiteral}</span>Used option 2 for now in 519926b but open to a change of wording if there are any better suggestions, I was struggling |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx (2)
521-548: Consider simplifying the weekend handling translation structure.While the nested
Transcomponents work as intended (as confirmed by the retrieved learning), the current structure might be complex for translators. ThebeforeOrAftertranslation object is empty and cast asTransObjectLiteral.Consider this simpler structure:
- <Trans> - <label - htmlFor="form_skipwe" - style={{ - userSelect: 'none', - marginRight: 5, - }} - > - Move schedule{' '} - </label> - <Select - id="solve_dropdown" - options={[ - ['before', 'before'], - ['after', 'after'], - ]} - value={state.config.weekendSolveMode} - onChange={value => dispatch({ type: 'set-weekend-solve', value })} - disabled={!skipWeekend} - /> - <label - htmlFor="solve_dropdown" - style={{ userSelect: 'none', marginLeft: 5 }} - > - {' '} - {{ beforeOrAfter: '' } as TransObjectLiteral} weekend - </label> - </Trans> + <Trans + i18nKey="schedule.weekend.move" + values={{ + direction: ( + <Select + id="solve_dropdown" + options={[ + ['before', t('schedule.weekend.before')], + ['after', t('schedule.weekend.after')], + ]} + value={state.config.weekendSolveMode} + onChange={value => dispatch({ type: 'set-weekend-solve', value })} + disabled={!skipWeekend} + /> + ), + }} + > + Move schedule {{ direction }} weekend + </Trans>This approach:
- Uses a single translation key
- Properly handles the select component interpolation
- Translates the select options
545-546: Type cast can be improved.The empty string cast as
TransObjectLiteralis not necessary since the type is already known.-{{ beforeOrAfter: '' } as TransObjectLiteral} weekend +{{ beforeOrAfter: state.config.weekendSolveMode }} weekend
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/desktop-client/src/components/LoggedInUser.tsx(3 hunks)packages/desktop-client/src/components/modals/CloseAccountModal.tsx(9 hunks)packages/desktop-client/src/components/reports/SaveReportDelete.tsx(3 hunks)packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx(5 hunks)packages/loot-core/src/types/util.d.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/desktop-client/src/components/reports/SaveReportDelete.tsx
- packages/desktop-client/src/components/modals/CloseAccountModal.tsx
- packages/desktop-client/src/components/LoggedInUser.tsx
🧰 Additional context used
📓 Learnings (1)
packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx (1)
Learnt from: matt-fidd
PR: actualbudget/actual#4041
File: packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx:518-544
Timestamp: 2024-12-26T01:16:39.339Z
Learning: A <Trans> inside another <Trans> does not necessarily produce two distinct strings. Instead, it merges them into a single translation entry, allowing for object literals to be used within the same translation string.
🔇 Additional comments (3)
packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx (3)
15-18: LGTM! Good type safety improvements.The addition of
TransObjectLiteraltype from the util types improves type safety for translation objects.
39-50: LGTM! Well-structured hook for frequency options.The
useFrequencyOptionshook effectively encapsulates the translation logic for frequency options. The use ofas constensures type safety for the options array.
54-68: LGTM! Well-structured hook for day of week options.The
useDayOfWeekOptionshook follows the same pattern asuseFrequencyOptions, maintaining consistency in the codebase.
There was a problem hiding this comment.
I don't love it but can't think of a better workaround. Could be worth having a translation-specific utils file—one other quirk of Trans (apparently) is that it doesn't rerender when the language changes, and you have to create your own component to do that (we can do it later once we actually let the user pick a language):
import type { ComponentProps } from 'react';
import { Trans as DefaultTrans } from 'react-i18next';
export const Trans = (props: ComponentProps<DefaultTrans>) => {
const { t, i18n } = useTranslation();
return <DefaultTrans t={t} i18n={i18n} {...props} />
}
This is the first of a series of changes to improve the translatable strings generated with
react-i18n. At the moment, some sentences are broken up and lose context once exported.This PR is already quite big, and I'm barely past B in the locale file so there might be quite a few parts to this to make the reviews more manageable.
I've also fixed any typos I've seen and migrated every file I've been in so far to use
<Trans>inside React components where possible for consistency. There are also some places where the text used for the same action is inconsistent across the app - I've changed these where possible too.