Marked files for translation #3752
Conversation
actual-github-bot
bot
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
awaisalee
force-pushed
the
feature/i18n-integration
branch
3 times, most recently
from
12748f1 to
ad76193
Compare
|
If you are ready for a review, then remove the WIP label. Also, there shound't be any visual regression issues with just a translation, so you will need to look into why there are visual changes. |
awaisalee
force-pushed
the
feature/i18n-integration
branch
from
ad76193 to
1780cb8
Compare
awaisalee
changed the title
WalkthroughThe pull request introduces internationalization support across multiple components in the desktop client of the application. The 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: 10
🧹 Outside diff range and nitpick comments (10)
packages/loot-core/src/platform/server/fs/shared.ts (2)
11-11: Consider using a structured translation key.
Instead of using a full English sentence as the translation key, consider using a structured key format for better maintainability.
- throw new Error(t('Document directory is not set'));
+ throw new Error(t('errors.fs.document_dir_not_set'));18-18: Maintain consistent translation key structure.
The translation implementation is good, but consider using structured keys for consistency with other error messages.
- throw new Error(t('getDocumentDir: id is falsy: {{id}}', { id }));
+ throw new Error(t('errors.fs.invalid_budget_id_falsy', { id }));
throw new Error(
t(
- 'Invalid budget id "{{id}}". Check the id of your budget in the Advanced section of the settings page.',
+ 'errors.fs.invalid_budget_id_format',
{ id },
),
);Also applies to: 30-33
packages/loot-core/src/server/app.ts (2)
31-33: Consider using a more specific translation key.
While the translation implementation is correct, consider using a more specific key like 'error.method.name_conflict' instead of a full sentence as the key. This makes it easier to maintain and reuse translations.
- t('Conflicting method name, names must be globally unique: {name}', {
+ t('error.method.name_conflict', {
name,
}),64-64: Maintain consistent translation key structure.
The translation implementation is correct, but for consistency with the previous suggestion:
- new Error(t('Cannot start services because they are already running.')),
+ new Error(t('error.services.already_running')),packages/loot-core/src/server/accounts/qif2json.ts (2)
40-42: Consider enhancing error message specificity
While the translation is implemented correctly, the error message could be more specific about what makes the file invalid (missing Type header). This would help users better understand and fix the issue.
Consider this enhancement:
- t('File does not appear to be a valid QIF file: {{line}}', { line }),
+ t('Invalid QIF file: Missing or incorrect Type header. Found: {{line}}', { line }),Line range hint 23-23: Consider adding JSDoc documentation
The function would benefit from comprehensive documentation describing:
- Expected QIF file format
- Possible error scenarios and their translated messages
- Format requirements for dates and amounts
- Examples of valid input
Example documentation:
/**
* Converts QIF (Quicken Interchange Format) content to JSON
* @param {string} qif - QIF file content
* @param {Object} options - Parsing options
* @param {string} [options.dateFormat] - Expected date format in QIF file
* @returns {Object} Parsed QIF data with transactions
* @throws {Error} Translated error messages for:
* - Invalid QIF header
* - Unknown detail codes
* @example
* const qifContent = `!Type:Bank
* D01/01/2023
* T-1000.00
* PPayee Name
* MDescription
* ^`;
* const result = qif2json(qifContent, { dateFormat: 'MM/DD/YYYY' });
*/packages/loot-core/src/platform/server/fs/index.electron.ts (1)
Line range hint 5-160: Consider implementing a structured translation strategy.
To improve maintainability and clarity of translations:
- Create a dedicated translation key hierarchy (e.g.,
errors.fs.*,success.fs.*) - Separate technical details from translatable content
- Consider implementing a translation key validation system
- Document translation guidelines for developers
This will help maintain consistency and make it easier to manage translations as the application grows.
packages/loot-core/src/server/aql/schema/executors.ts (1)
37-41: Consider using a shorter translation key
While the translation implementation is functionally correct, consider using a shorter, more maintainable key instead of a full English sentence. This makes it easier to handle different language structures and word orders.
Example improvement:
- t('Invalid "splits" option for transactions: "{{splitType}}"', {
+ t('errors.transactions.invalid_splits_option', {
splitType,
}),This approach:
- Makes keys more maintainable
- Allows more flexibility in translation structure
- Follows common i18next naming conventions
packages/desktop-client/src/components/transactions/TransactionsTable.jsx (2)
Line range hint 219-304: Consider using proper translation keys instead of English text.
While the translation function is correctly applied to all header cells, using English text directly as translation keys (e.g., t('Date')) is not recommended. Consider using standardized translation keys that are less prone to change and easier to maintain.
Example refactor using proper translation keys:
- value={t('Date')}
+ value={t('transaction.header.date')}
- value={t('Account')}
+ value={t('transaction.header.account')}
- value={t('Payee')}
+ value={t('transaction.header.payee')}
- value={t('Notes')}
+ value={t('transaction.header.notes')}
- value={t('Category')}
+ value={t('transaction.header.category')}
- value={t('Payment')}
+ value={t('transaction.header.payment')}
- value={t('Deposit')}
+ value={t('transaction.header.deposit')}
- value={t('Balance')}
+ value={t('transaction.header.balance')}Line range hint 1-1: Consider completing the internationalization effort.
While the table headers have been translated, there are still many untranslated strings in the component that should be considered for internationalization:
- Error messages (e.g., "Amount left:", "Account is a required field")
- Button labels (e.g., "Cancel", "Add", "Distribute")
- Status messages and tooltips (e.g., "Imported Payee", "Split", "Off Budget")
Also, consider adding translation namespace configuration to organize translations better:
const { t } = useTranslation('transactions'); // Specify the namespace📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
upcoming-release-notes/3752.mdis excluded by!**/*.md
📒 Files selected for processing (10)
- packages/desktop-client/src/components/UpdateNotification.tsx (5 hunks)
- packages/desktop-client/src/components/sidebar/PrimaryButtons.tsx (1 hunks)
- packages/desktop-client/src/components/transactions/TransactionsTable.jsx (10 hunks)
- packages/loot-core/src/platform/server/fs/index.electron.ts (4 hunks)
- packages/loot-core/src/platform/server/fs/index.web.ts (3 hunks)
- packages/loot-core/src/platform/server/fs/shared.ts (2 hunks)
- packages/loot-core/src/server/accounts/qif2json.ts (3 hunks)
- packages/loot-core/src/server/app.ts (4 hunks)
- packages/loot-core/src/server/aql/exec.ts (2 hunks)
- packages/loot-core/src/server/aql/schema/executors.ts (2 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/loot-core/src/platform/server/fs/index.web.ts (1)
Learnt from: tlesicka
PR: actualbudget/actual#3689
File: packages/loot-core/src/platform/server/fs/index.web.ts:319-340
Timestamp: 2024-10-25T05:04:40.902Z
Learning: In `packages/loot-core/src/platform/server/fs/index.web.ts`, errors are already handled in other locations, so additional error handling in `copyFile` is unnecessary.
🪛 Biome
packages/loot-core/src/platform/server/fs/shared.ts
[error] 7-7: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (16)
packages/loot-core/src/platform/server/fs/shared.ts (2)
2-2: LGTM: Import added for internationalization support.
The addition of the i18next import is appropriate for the translation requirements.
Line range hint 22-29: Verify the security implications of budget ID validation.
The current validation prevents directory traversal, but the TODO comment suggests this needs improvement. Let's verify if there are any existing security measures or patterns used elsewhere in the codebase.
✅ Verification successful
Let me analyze the validation patterns further by checking the implementation details.
Let me check one more aspect to see the complete implementation of the budget directory handling.
The current budget ID validation is adequate and properly implemented
The validation serves its security purpose effectively:
- Restricts budget IDs to alphanumeric characters, hyphens, and underscores (
/[^A-Za-z0-9\-_]/) - Prevents directory traversal by rejecting any path separators
- Is consistently used across the codebase through the centralized
getBudgetDirfunction - The validation occurs before any file system operations, as evidenced by the implementation in
shared.ts - All file system access patterns shown in the results properly use this validated path
The "TODO" comment suggesting improvement appears to be a historical artifact, as the current implementation already follows security best practices for path validation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar path validation patterns
echo "Searching for path validation patterns..."
rg -l "match\(/\[.*\]/\)" --type ts
# Search for security-related comments
echo "Searching for related security comments..."
rg -i "TODO.*security|SECURITY|HACK|FIXME" --type ts
# Look for other budget ID validations
echo "Searching for other budget ID validations..."
rg "budgetId|budget.*id" --type ts
Length of output: 14744
Script:
#!/bin/bash
# Check the implementation of path validation in shared.ts
echo "Checking path validation implementation:"
rg -A 5 "match\(/\[.*\]/\)" packages/loot-core/src/platform/server/fs/shared.ts
# Check getBudgetDir implementation
echo "\nChecking getBudgetDir implementation:"
ast-grep --pattern 'function getBudgetDir($_) {
$$$
}'
# Check for additional file system security measures
echo "\nChecking for additional fs security measures:"
rg -A 5 "getBudgetDir|validatePath|sanitize" packages/loot-core/src/platform/server/fs/
Length of output: 2986
Script:
#!/bin/bash
# Get the full implementation of shared.ts
echo "Checking complete shared.ts implementation:"
cat packages/loot-core/src/platform/server/fs/shared.ts
# Look for any file system access patterns
echo "\nChecking file system access patterns:"
rg -A 5 "join\(.*getBudgetDir" --type ts
Length of output: 3453
packages/desktop-client/src/components/sidebar/PrimaryButtons.tsx (1)
42-42: LGTM! Verify translation key exists.
The change correctly implements internationalization for the "More" text, maintaining consistency with other translated strings in the component.
Let's verify the translation key exists in the translation files:
packages/loot-core/src/server/app.ts (2)
2-2: LGTM: Import statement is correctly added.
The i18next import is properly placed and necessary for the internationalization changes.
79-83: 🛠️ Refactor suggestion
Enhance the validateQIFFile method implementation.
The method could benefit from the following improvements:
- Add JSDoc documentation explaining its purpose and usage
- Consider making it more reusable by returning a boolean instead of only throwing
- Add type validation for the line parameter
+ /**
+ * Validates if a line in a file conforms to QIF format
+ * @param line - The line number where validation failed
+ * @throws {Error} If the line is not valid QIF format
+ */
validateQIFFile(line: number) {
+ if (typeof line !== 'number' || line < 0) {
+ throw new Error(t('error.validation.invalid_line_number'));
+ }
throw new Error(
- t('File does not appear to be a valid QIF file: {line}', { line }),
+ t('error.qif.invalid_format', { line }),
);
}Let's check how this method is used in the codebase:
packages/loot-core/src/server/aql/exec.ts (2)
2-2: LGTM: Clean import addition for i18next.
The import statement is correctly placed and follows TypeScript conventions.
Line range hint 1-89: Consider translating other error scenarios.
While this change addresses the parameter validation error message, there are other potential error scenarios in this file that might benefit from translation. Consider reviewing the entire file for other user-facing messages that should be internationalized.
Let's check for other potential error messages in this file:
packages/loot-core/src/server/accounts/qif2json.ts (2)
2-2: LGTM: Clean i18next integration
The i18next import is correctly added for internationalization support.
106-106: Consider adding comprehensive error handling
While the unknown code error is now properly translated, the parser could benefit from additional error handling:
- Validate numeric values in amount fields
- Validate date format based on the provided dateFormat option
- Handle empty or malformed fields gracefully
Let's verify if there are any existing validation utilities we could leverage:
packages/desktop-client/src/components/UpdateNotification.tsx (2)
2-2: LGTM: Translation hook setup is correct.
The useTranslation hook is properly imported and initialized.
Also applies to: 17-17
Line range hint 1-100: Consider additional strings for translation.
There are a few more strings that might need translation:
- The URL to the release notes page might need localization if there are language-specific docs
- The release notes content in the
notesvariable might need translation depending on your requirements
Let's check if the docs URL is hardcoded elsewhere:
#!/bin/bash
# Search for hardcoded actualbudget.org URLs
rg 'actualbudget\.org/docs' --type ts --type tsxConsider:
- Moving the docs URL to a configuration file where it can be managed per locale
- Implementing a translation strategy for release notes if they need to be multilingual
packages/loot-core/src/platform/server/fs/index.electron.ts (1)
5-5: LGTM: Import statement is correctly placed.
The i18next import is appropriately placed with other third-party imports.
packages/loot-core/src/server/aql/schema/executors.ts (1)
2-2: LGTM: Clean i18next integration
The import of the translation function is properly placed and typed.
packages/loot-core/src/platform/server/fs/index.web.ts (1)
4-4: LGTM: Import added for internationalization support.
The addition of the i18next import is appropriate for the internationalization changes being made.
packages/desktop-client/src/components/transactions/TransactionsTable.jsx (2)
14-14: LGTM: Translation hook import added correctly.
The useTranslation hook import is properly placed with other React-related imports.
180-180: LGTM: Translation hook initialized properly.
The useTranslation hook is correctly initialized at the component level, following React hooks best practices.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
packages/desktop-client/src/components/modals/CategoryMenuModal.tsx (1)
Line range hint 1-224: Add translations for remaining hardcoded strings.
Several text strings in the component remain hardcoded and should be marked for translation:
- Button text: "Edit notes"
- Notes placeholder: "No notes"
- Menu items: "Show", "Hide", "Delete"
Here's how to update these strings:
// In the Notes component
<Notes
- notes={originalNotes?.length > 0 ? originalNotes : 'No notes'}
+ notes={originalNotes?.length > 0 ? originalNotes : t('No notes')}
editable={false}
focused={false}
// ...
/>
// In the button
<Button style={buttonStyle} onPress={_onEditNotes}>
<SvgNotesPaper
width={20}
height={20}
style={{ paddingRight: 5 }}
/>
- Edit notes
+ {t('Edit notes')}
</Button>
// In the Menu items
items={[
!categoryGroup?.hidden && {
name: 'toggleVisibility',
- text: category.hidden ? 'Show' : 'Hide',
+ text: category.hidden ? t('Show') : t('Hide'),
icon: category.hidden ? SvgViewShow : SvgViewHide,
iconSize: 16,
},
!categoryGroup?.hidden && Menu.line,
{
name: 'delete',
- text: 'Delete',
+ text: t('Delete'),
icon: SvgTrash,
iconSize: 15,
},
]}packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (2)
Line range hint 123-123: Consider translating "No transactions" text.
The "No transactions" string appears in multiple places but isn't internationalized. Consider wrapping it with the translation function:
- <Text style={{ fontSize: 15 }}>No transactions</Text>
+ <Text style={{ fontSize: 15 }}>{t('No transactions')}</Text>Also applies to: 134-134
Line range hint 269-425: Consider translating UI strings in the action bar.
Several strings in the SelectedTransactionsFloatingActionBar component should be translated for consistency:
- Button labels: "Edit"
- Menu items: "Account", "Payee", "Notes", "Category", "Cleared"
- Action texts: "Duplicate", "Link schedule", "Unlink schedule", "Delete"
- Status messages: "transactions selected", "Successfully updated", etc.
Here's an example of how to translate some of these strings:
- <Text style={styles.mediumText}>
- {selectedTransactions.size}{' '}
- {isMoreThanOne ? 'transactions' : 'transaction'} selected
- </Text>
+ <Text style={styles.mediumText}>
+ {t('{{count}} {{type}} selected', {
+ count: selectedTransactions.size,
+ type: isMoreThanOne ? t('transactions') : t('transaction')
+ })}
+ </Text>📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- packages/desktop-client/src/components/Notifications.tsx (2 hunks)
- packages/desktop-client/src/components/UpdateNotification.tsx (5 hunks)
- packages/desktop-client/src/components/common/Modal.tsx (4 hunks)
- packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (2 hunks)
- packages/desktop-client/src/components/modals/CategoryMenuModal.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/desktop-client/src/components/UpdateNotification.tsx
🔇 Additional comments (9)
packages/desktop-client/src/components/modals/CategoryMenuModal.tsx (2)
4-5: LGTM: Import of i18next translation function.
The addition of the t function import is appropriate for implementing internationalization.
178-178: LGTM: Aria-label translation implemented.
The menu button's accessibility label is correctly internationalized using the t function.
packages/desktop-client/src/components/Notifications.tsx (2)
12-12: LGTM: Import of i18next translation function.
The addition of the i18next translation function import is appropriate for the internationalization changes.
Line range hint 29-69: Update message compilation to support translations.
The compileMessage function needs to be updated to handle translations while preserving the link functionality. Additionally, consider the following:
- Translated strings might be longer than the original text, which could affect layout
- Link text within messages should also be translated
- The paragraph splitting logic might need adjustment for different languages
Let's check for potential visual issues with longer translations:
Consider implementing these improvements:
- Add a translation context to handle message formatting
- Use a translation key system instead of direct strings
- Add comments for translators about string formatting
Example approach:
function compileMessage(
messageKey: string,
actions: Record<string, () => void>,
setLoading: (arg: SetStateAction<boolean>) => void,
onRemove?: () => void,
) {
const message = t(messageKey);
// Rest of the implementation...
}packages/desktop-client/src/components/common/Modal.tsx (4)
94-94: LGTM! Good accessibility enhancement
The translation of the modal dialog's aria-label improves accessibility for international users relying on screen readers.
323-323: LGTM! Good accessibility enhancement
The translation of the logo's aria-label improves accessibility for international users relying on screen readers.
20-20: 🛠️ Refactor suggestion
Consider using useTranslation hook instead of direct t import
For better handling of language switching and React lifecycle integration, consider using the useTranslation hook from react-i18next instead of directly importing the t function.
-import { t } from 'i18next';
+import { useTranslation } from 'react-i18next';
export const Modal = ({
name,
isLoading = false,
noAnimation = false,
style,
children,
onClose,
containerProps,
...props
}: ModalProps) => {
+ const { t } = useTranslation();⛔ Skipped due to learnings
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-10-08T15:46:15.739Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is acceptable and commonly used in the codebase.
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-09-28T17:08:11.028Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is acceptable and commonly used in the codebase.
466-466: LGTM! Verify consistent translation usage
The translation of the close button's aria-label improves accessibility. Let's verify that this translation key is used consistently across other close buttons in the application.
✅ Verification successful
Translation key 'Close' is consistently used across the application ✅
The verification shows that all close buttons in the application consistently use the same translation key t('Close') for their aria-labels. This is implemented in:
- packages/desktop-client/src/components/common/Modal.tsx
- packages/desktop-client/src/components/Notifications.tsx
- packages/desktop-client/src/components/UpdateNotification.tsx
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other close button implementations to ensure consistent translation key usage
rg -l "aria-label.*['\"]Close['\"]" --type typescript --type typescriptreact
Length of output: 117
Script:
#!/bin/bash
# Search for close button aria-labels using ripgrep without file type filters
rg "aria-label.*['\"]Close['\"]"
# Also search for potential variations of close button implementations
rg "aria-label.*['\"]close['\"]" -i
# Search for t function calls with 'Close' as argument
rg "t\(['\"]Close['\"]"
Length of output: 947
packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (1)
11-12: LGTM! Translation implementation looks good.
The addition of i18next and translation of the loading message is implemented correctly.
Also applies to: 96-96
There was a problem hiding this comment.
Actionable comments posted: 65
🧹 Outside diff range and nitpick comments (61)
packages/desktop-client/src/components/ManageRulesPage.tsx (1)
3-4: Implementation looks correct but could be more robust.The basic implementation of i18next is correct and follows the standard pattern.
Consider these improvements for better maintainability and type safety:
-import { t } from 'i18next'; +import { useTranslation } from 'react-i18next'; export function ManageRulesPage() { + const { t } = useTranslation('common'); return ( - <Page header={t('Rules')}> + <Page header={t('rules.title', 'Rules')}>This would:
- Use the hook-based approach for better React integration
- Add namespace to prevent key conflicts
- Provide a fallback text
- Enable type checking with translation keys (when using TypeScript features)
Also applies to: 10-10
packages/desktop-client/src/components/mobile/transactions/AddTransactionButton.tsx (1)
24-24: Consider using a more specific translation keyWhile the translation implementation is correct, consider using a more specific translation key to avoid potential conflicts across the application.
- aria-label={t('Add transaction')} + aria-label={t('button.transaction.add')}packages/desktop-client/src/components/mobile/MobileBackButton.tsx (1)
Line range hint
1-45: Consider RTL language supportSince this component is being internationalized, consider the following for RTL (Right-to-Left) language support:
- The chevron icon direction should be reversed for RTL languages
- The margin/padding values should be mirrored
- The text alignment might need adjustment
Example implementation for RTL support:
// Add this import import { I18nManager } from 'react-native'; // Modify the SvgCheveronLeft style <SvgCheveronLeft style={{ width: 30, height: 30, margin: -10, marginLeft: I18nManager.isRTL ? -5 : undefined, marginRight: I18nManager.isRTL ? undefined : -5, transform: [{ scaleX: I18nManager.isRTL ? -1 : 1 }], }} />packages/desktop-client/src/components/modals/ManageRulesModal.tsx (1)
27-27: Use consistent translation key format.The translation key 'manage-rules' uses kebab-case while other keys in the file use regular text format (e.g., 'Rules'). Consider using a consistent format across all translation keys for better maintainability.
- <Modal name={t('manage-rules')} isLoading={loading}> + <Modal name={t('Manage Rules')} isLoading={loading}>packages/desktop-client/src/components/modals/BudgetListModal.tsx (1)
Line range hint
34-38: Consider internationalizing the current file name displayWhile "Switching from:" is properly translated, consider wrapping the display of the current file name in a translation context for better accessibility and localization support.
<Text style={{ fontSize: 17, fontWeight: 400 }}> {t('Switching from:')} </Text> <Text style={{ fontSize: 17, fontWeight: 700 }}> - {currentFile?.name} + {t('Current file: {{name}}', { name: currentFile?.name })} </Text>packages/desktop-client/src/components/ThemeSelector.tsx (1)
Based on the complete analysis:
- The theme names are hardcoded in the codebase as simple strings
- The locales directory doesn't exist in the expected location, suggesting translations might be handled differently
- The codebase extensively uses translations for user-facing strings
- Theme names are part of the UI that users interact with
Theme names should be translatable for consistency
The review comment is correct. Theme names like 'Light', 'Dark', 'Midnight', and 'System default' are user-facing strings that should be translatable for consistency with the rest of the application's internationalization approach. This would ensure a uniform experience for users across different languages.
🔗 Analysis chain
Line range hint
65-67: Consider translating menu itemsThe menu items text from
themeOptionsis currently not wrapped in translation calls. Consider whether these theme names should also be translatable for consistency.
Let me check the theme definitions to better understand the context.
Based on the analysis of the code, I can see that:
- The theme names are hardcoded in
theme.tsxas simple strings: 'Light', 'Dark', 'Midnight', 'System default', etc.- The codebase extensively uses translations via
useTranslationhook fromreact-i18nextfor user-facing strings.Let me check if there are any existing translations for theme names in the codebase.
Let me try a different approach to check for theme-related translations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check how themeOptions is defined and if it's already handling translations # Expected: Find the themeOptions definition to understand if translation is needed # Search for themeOptions definition echo "Searching for themeOptions definition..." rg "themeOptions.*=" # Check if it's already using translations echo "Checking for existing translation usage..." ast-grep --pattern 'themeOptions = $$$t($$$)$$$'Length of output: 487
Script:
#!/bin/bash # Check the theme.tsx file content to understand how themes and text are defined echo "Checking theme.tsx content..." cat packages/desktop-client/src/style/theme.tsx # Also check for any existing translation patterns in the codebase echo "Checking translation patterns in the codebase..." rg "import.*useTranslation" -A 2Length of output: 35217
Script:
#!/bin/bash # Check for any existing theme-related translations echo "Searching for theme-related translations..." rg -i "theme.*translation" rg "\"theme\".*:.*{" packages/desktop-client/src/locales/ -A 5Length of output: 278
Script:
#!/bin/bash # Check for translation files and their content echo "Checking translation files..." fd . packages/desktop-client/src/locales/ -e json -x cat {} # Also check for any theme-related strings in translation files echo "Checking for theme strings in translations..." rg -i "\"light\"|\"dark\"|\"midnight\"" packages/desktop-client/src/locales/Length of output: 507
packages/desktop-client/src/components/modals/ConfirmUnlinkAccountModal.tsx (1)
29-31: Remove redundant commentThe inline comment "// Use translation for title" is unnecessary as the code is self-documenting through the use of the
t()function.- title={t('Confirm Unlink')} // Use translation for title + title={t('Confirm Unlink')}packages/desktop-client/src/components/modals/ConfirmTransactionDeleteModal.tsx (2)
Line range hint
12-14: Default message prop should be internationalizedThe default message in the props is still hardcoded in English.
Consider updating the props and default value:
type ConfirmTransactionDeleteProps = { message?: string; onConfirm: () => void; }; export function ConfirmTransactionDeleteModal({ - message = 'Are you sure you want to delete the transaction?', + message = t('modals.confirm_delete.default_message'), onConfirm, }: ConfirmTransactionDeleteProps) {Note: Since the translation hook can't be used in the parameter default value, you might need to handle this differently:
export function ConfirmTransactionDeleteModal({ message, onConfirm, }: ConfirmTransactionDeleteProps) { const { t } = useTranslation(); const defaultMessage = t('modals.confirm_delete.default_message'); const finalMessage = message || defaultMessage; // ... rest of the component }
Line range hint
1-70: Consider adding type safety for translation keysTo prevent typos and ensure all translation keys are valid, consider adding type safety.
You can use TypeScript's type system to enforce valid translation keys:
// types/i18next.d.ts import 'react-i18next'; declare module 'react-i18next' { interface CustomTypeOptions { defaultNS: 'translation'; resources: { translation: { modals: { confirm_delete: { title: string; default_message: string; }; }; common: { actions: { cancel: string; delete: string; }; }; }; }; } }packages/desktop-client/src/components/payees/PayeeMenu.tsx (1)
Line range hint
69-89: Consider translating menu item texts.While the ', and more' suffix is now translatable, the menu item texts ("Delete", "Favorite", "Merge") are still hardcoded. Consider translating these strings as well to maintain consistency with the PR's internationalization objectives.
Here's how you can update the menu items:
{ icon: SvgDelete, name: 'delete', - text: 'Delete', + text: t('Delete'), disabled: isDisabled, }, { icon: SvgBookmark, iconSize: 9, name: 'favorite', - text: 'Favorite', + text: t('Favorite'), disabled: isDisabled, }, { icon: SvgMerge, iconSize: 9, name: 'merge', - text: 'Merge', + text: t('Merge'), disabled: isDisabled || selectedPayees.size < 2, },packages/desktop-client/src/components/settings/Export.tsx (2)
52-54: Add translation context for error messageThe error message is quite long and might need context for translators to understand its purpose and maintain consistent terminology.
- {t( - 'An unknown error occurred while exporting. Please report this as a new issue on Github.', - )} + {t( + 'An unknown error occurred while exporting. Please report this as a new issue on Github.', + { context: 'Error shown when budget export fails unexpectedly' } + )}
68-70: Add translation context for encryption noticeThe encryption-related message should include context for translators to understand when this message appears.
- {t( - 'Even though encryption is enabled, the exported zip file will not have any encryption.', - )} + {t( + 'Even though encryption is enabled, the exported zip file will not have any encryption.', + { context: 'Warning shown when exporting from an encrypted budget' } + )}packages/desktop-client/src/components/settings/UI.tsx (1)
Line range hint
77-94: Consider adding translation namespaceWhile the basic translation setup is correct, consider adding a namespace to organize translations better, especially for settings-related strings. This would help maintain a cleaner translation structure as the application grows.
Example implementation:
- import { t } from 'i18next'; + import { useTranslation } from 'react-i18next'; export const AdvancedToggle = ({ children }: AdvancedToggleProps) => { + const { t } = useTranslation('settings'); // ... rest of the code ... <View style={{ fontSize: 20, fontWeight: 500, flexShrink: 0 }}> - {t('Advanced Settings')} + {t('advanced.title')} </View> // ... rest of the code ... <Link // ... props ... > - {t('Show advanced settings')} + {t('advanced.show')} </Link>packages/desktop-client/src/components/modals/NotesModal.tsx (1)
Line range hint
32-89: Consider translating additional UI elementsSeveral UI elements could benefit from translation:
- Modal name:
name="notes"- Aria labels and accessibility text
- Error messages (if any are added in the future)
This ensures complete internationalization support across all user-facing text.
packages/desktop-client/src/components/NotesButton.tsx (1)
Line range hint
1-94: Consider translating additional user-visible strings.While the aria-label has been translated, there might be other user-visible strings in the Notes component that should also be translated for consistency. Consider reviewing the
Notescomponent to ensure all user-facing text is properly internationalized.packages/desktop-client/src/components/reports/SaveReportName.tsx (2)
67-67: Consider using a standardized translation keyWhile the translation works, consider using a more specific and standardized key format to avoid potential conflicts and improve maintainability.
- title={t('Report Name')} + title={t('reports.save.name_label')}
80-80: Standardize button translation keysConsider using more specific and structured keys for button labels to maintain consistency across the application.
- {menuItem === 'save-report' ? t('Add') : t('Update')} + {menuItem === 'save-report' ? t('common.buttons.add') : t('common.buttons.update')}packages/desktop-client/src/components/settings/Reset.tsx (1)
Line range hint
1-79: Consider implementing a robust translation management systemTo improve maintainability and type safety, consider:
- Creating a centralized translation key management system
- Using TypeScript's string literal types for translation keys
- Adding documentation for translators
- Implementing a translation key validation system
Example implementation:
// types/i18n.ts export type TranslationKey = | 'settings.reset.cache.button' | 'settings.reset.cache.title' | 'settings.reset.cache.description' | 'settings.reset.sync.button' // ... other keys // Typed translation function declare module 'i18next' { interface TFunction { (key: TranslationKey): string; } }packages/desktop-client/src/components/modals/HoldBufferModal.tsx (2)
Based on the search results, I can now generate a final response:
Fixed width styling in modals should be replaced with flexible layouts
Looking at the codebase patterns:
- Most modals use flexible width constraints (e.g.
width: '30vw',maxWidth: 500)- The
mobileMinHeightandmobileEditingPaddingconstants are used extensively across mobile components for consistent spacing- The current fixed styling could cause issues with:
- Translated text in different languages
- Different screen sizes and orientations
- Dynamic content lengths
Recommendations:
- Use
maxWidthinstead of fixed margins- Consider using flexbox or percentage-based widths for the content container
- Add
overflowhandling for the input and button containers- Test with different viewport sizes and longer translated text
🔗 Analysis chain
Line range hint
21-81: Consider potential text expansion in translationsThe modal uses fixed styling with
mobileEditingPaddingandmobileMinHeight. Some languages might require more space for their translations, which could cause text overflow or visual issues.
The previous ast-grep pattern had syntax issues. Let's try with ripgrep to find modal styling patterns and translation usage, along with the mobile styling constants to understand how they're used across the codebase.
Let's try without the typescript file type flag since it's not recognized. This should help us understand how modals are styled across the codebase and how translations are handled.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Look for similar modals to see how they handle variable text lengths ast-grep --pattern 'Modal name="$_" { $$$ style={$_} $$$ }'Length of output: 161
Script:
#!/bin/bash # Look for similar modals with styling patterns rg -U "Modal.*?name=.*?\{[\s\S]*?style.*?\}" --type typescript # Also search for translation usage in modals rg -U "Modal.*?name=.*?\{[\s\S]*?t\(.*?\)" --type typescript # Search for specific mobile styling constants rg "mobileEditingPadding|mobileMinHeight" --type typescriptLength of output: 294
Script:
#!/bin/bash # Look for similar modals with styling patterns rg -U "Modal.*?name=.*?\{[\s\S]*?style.*?\}" # Also search for translation usage in modals rg -U "Modal.*?name=.*?\{[\s\S]*?t\(.*?\)" # Search for specific mobile styling constants rg "mobileEditingPadding|mobileMinHeight" # Look for style definitions rg "export const styles"Length of output: 15667
Accessibility improvements needed for label-input association
The current implementation of
FieldLabelrenders as a styledTextcomponent without proper HTML semantics or ARIA attributes. To ensure accessibility:
- Add proper HTML
<label>element withhtmlForattribute inFieldLabelcomponent- Generate unique IDs for the
AmountInputand connect it with its label- Consider adding
aria-labeloraria-labelledbyto create a programmatic associationExample locations needing updates:
packages/desktop-client/src/components/mobile/MobileForms.tsx: UpdateFieldLabelimplementationpackages/desktop-client/src/components/modals/HoldBufferModal.tsx: Add ID toAmountInputand connect with label🔗 Analysis chain
Line range hint
40-58: Verify accessibility with translated contentThe field label and amount input are closely related. Ensure that the translation doesn't break the visual or semantic connection between them, especially for screen readers.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any aria-label or aria-labelledby attributes that might need translation rg -g '*.tsx' -g '*.jsx' 'aria-label|aria-labelledby' ./packages/desktop-client/src/components/Length of output: 161
Script:
#!/bin/bash # Let's check for accessibility patterns in the component and its dependencies rg -g '*.tsx' -g '*.jsx' -A 5 '<FieldLabel.*title=' ./packages/desktop-client/src/components/ # Check AmountInput component implementation ast-grep --pattern 'const AmountInput = $_' # Check InitialFocus component implementation ast-grep --pattern 'const InitialFocus = $_' # Look for any existing accessibility implementations in similar modals rg -g '*.tsx' -g '*.jsx' -A 5 'role="dialog"|aria-modal' ./packages/desktop-client/src/components/Length of output: 30175
Script:
#!/bin/bash # Let's check the FieldLabel component implementation to understand its accessibility features ast-grep --pattern 'const FieldLabel = $_' # Let's also check if there are any aria attributes in the AmountInput component rg -g '*.tsx' -g '*.jsx' 'aria-' ./packages/desktop-client/src/components/mobile/transactions/FocusableAmountInput.tsx # Check if there are any id/htmlFor relationships between labels and inputs rg -g '*.tsx' -g '*.jsx' 'htmlFor|id=' ./packages/desktop-client/src/components/modals/Length of output: 7760
Script:
#!/bin/bash # Let's check the FieldLabel implementation in a different way rg -g '*.tsx' -g '*.jsx' -B 5 -A 10 'export.*FieldLabel' ./packages/desktop-client/src/components/ # Check if there are any existing aria-labelledby implementations in the codebase rg -g '*.tsx' -g '*.jsx' -A 3 'aria-labelledby' ./packages/desktop-client/src/ # Look for any existing patterns of connecting labels with inputs rg -g '*.tsx' -g '*.jsx' -A 3 '<label.*for=' ./packages/desktop-client/src/Length of output: 1677
packages/desktop-client/src/components/modals/CategoryAutocompleteModal.tsx (1)
55-55: Consider using namespaced translation keys.To avoid potential key conflicts and maintain better organization as the application grows, consider using namespaced translation keys. For example:
t('modals.category.title')instead oft('Category')This makes it easier to:
- Organize translations by feature/component
- Avoid duplicate keys
- Maintain consistency across similar components
Also applies to: 70-70
packages/desktop-client/src/components/modals/SimpleFinInitialiseModal.tsx (1)
87-89: Consider using more specific error messageThe current error message is quite generic. Consider providing more specific feedback about the token requirements or validation rules.
-<Error>{t('It is required to provide a token.')}</Error> +<Error>{t('simplefin.setup.errors.token_required', 'Please enter your SimpleFIN token to continue.')}</Error>packages/desktop-client/src/components/Notes.tsx (1)
126-126: Consider handling the technical term "markdown" in translations.While the translation implementation is correct, the string contains the technical term "markdown" which might need special handling in translations. Consider either:
- Keeping "markdown" untranslated using nested translation keys
- Providing translator comments about the technical term
Example of nested translation:
-placeholder={t('Notes (markdown supported)')} +placeholder={t('notes.placeholder', { markdown: 'Markdown' })}With translation key:
{ "notes": { "placeholder": "Notes ({{markdown}} supported)" } }packages/desktop-client/src/components/modals/ScheduledTransactionMenuModal.tsx (1)
Line range hint
109-111: Internationalize the error message.The error message in the switch statement's default case should also be translated.
- throw new Error(`Unrecognized menu option: ${name}`); + throw new Error(t('errors.unrecognized_menu_option', { option: name }));packages/desktop-client/src/components/modals/GoCardlessInitialiseModal.tsx (2)
83-83: Standardize form label translation keysThe form labels should follow a consistent key naming pattern for better maintainability.
-<FormLabel title={t('Secret ID:')} htmlFor="secret-id-field" /> +<FormLabel title={t('gocardless.form.secret_id')} htmlFor="secret-id-field" /> -<FormLabel title={t('Secret Key:')} htmlFor="secret-key-field" /> +<FormLabel title={t('gocardless.form.secret_key')} htmlFor="secret-key-field" />Also applies to: 98-98
112-114: Use translation key for error messageError messages should use a structured key format and avoid punctuation in the translation key.
-{t( - 'It is required to provide both the secret id and secret key.', -)} +{t('gocardless.error.missing_credentials')}packages/desktop-client/src/components/reports/ReportSummary.tsx (1)
Based on the comprehensive analysis of the codebase, I can now provide a final response:
Internationalization implementation needs improvements for better locale support
The current implementation has mixed support for internationalization:
- ✅ Number formatting uses
Intl.NumberFormatwith proper locale handling- ✅ Basic text translations are supported through i18next
- ❌ Date formatting relies on date-fns without locale-aware formatting
- ❌ Limited RTL support with only basic direction property in FixedSizeList
Recommended improvements:
- Replace date-fns formatting with
Intl.DateTimeFormatfor locale-aware date handling- Add proper RTL layout support through:
- CSS logical properties (start/end instead of left/right)
- RTL-aware flexbox direction
- Bidirectional text support
- Add explicit language direction handling in the i18next configuration
The current implementation may lead to incorrect date formats and layout issues in RTL languages.
🔗 Analysis chain
Line range hint
1-167: Ensure proper internationalization support for dates and numbersWhile adding text translations is a good start, consider these additional internationalization aspects:
- Use locale-aware date formatting instead of hardcoded formats
- Ensure number formatting respects locale settings
- Verify layout supports RTL languages and longer translated texts
Let's check if the codebase has proper RTL support:
Consider these improvements:
- Use
Intl.DateTimeFormatfor locale-aware date formatting- Use
Intl.NumberFormatfor locale-aware number formatting- Add RTL support in the layout styles
Let me analyze the RTL support findings and generate additional verification for date/number formatting:
The first search will help verify if the app uses native Intl APIs for locale-aware formatting. The second search will help understand the i18next configuration for proper RTL support.
Let me analyze these results before providing the final response.
Let me check the date formatting implementation since we've seen the number formatting:
This will help us understand how dates are formatted and how i18next is configured. Once we see these results, we can make a final determination about the internationalization implementation.
Let me check one final thing about the date formatting implementation:
This will help us understand the date formatting implementation and any explicit RTL handling in the codebase. Then we can provide a final assessment of the internationalization implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for RTL support in styling rg -g '*.{ts,tsx,js,jsx}' "direction:|textAlign:|marginLeft:|marginRight:|paddingLeft:|paddingRight:"Length of output: 50473
Script:
#!/bin/bash # Check for Intl usage and other i18n date/number formatting rg -g '*.{ts,tsx,js,jsx}' "Intl\.|new Intl|DateTimeFormat|NumberFormat" # Check i18next configuration and setup rg -g '*.{ts,tsx,js,jsx}' "i18next\.init|i18n\.init|i18next\.setup"Length of output: 5204
Script:
#!/bin/bash # Check monthUtils implementation for date formatting rg -g '*.{ts,tsx,js,jsx}' -A 5 "export function format" "loot-core/src/shared/months" # Check i18next configuration files fd -e json -e js -e ts "i18n" --exec cat {}Length of output: 1396
Script:
#!/bin/bash # Find the actual months.ts file and check its implementation fd -g "*months.{ts,tsx,js,jsx}" --exec cat {} # Check for any RTL specific configuration or utilities rg -g '*.{ts,tsx,js,jsx}' "rtl|direction:|dir="Length of output: 15026
packages/desktop-client/src/components/settings/Format.tsx (1)
Line range hint
24-45: Missing translations for day names and date format labelsThe day names and date format labels are still hardcoded in English. These should be translated for consistency with the rest of the internationalization effort.
Consider updating the arrays like this:
const daysOfWeek = [ - { value: '0', label: 'Sunday' }, + { value: '0', label: t('Sunday') }, - { value: '1', label: 'Monday' }, + { value: '1', label: t('Monday') }, // ... similar changes for other days ]; const dateFormats = [ - { value: 'MM/dd/yyyy', label: 'MM/DD/YYYY' }, + { value: 'MM/dd/yyyy', label: t('MM/DD/YYYY') }, // ... similar changes for other formats ];packages/desktop-client/src/components/mobile/transactions/TransactionListWithBalances.jsx (3)
134-137: Use namespaced translation keysSimple keys like 'Cleared' and 'Uncleared' might clash with translations from other components. Consider using namespaced keys following a consistent pattern.
-title={t('Cleared')} +title={t('transactions.balance.cleared')} -title={t('Uncleared')} +title={t('transactions.balance.uncleared')}Also applies to: 160-161
184-184: Use namespaced translation key for consistencySimilar to the previous comment, use a namespaced key for the Balance label.
-<Label title={t('Balance')} style={{ textAlign: 'center' }} /> +<Label title={t('transactions.balance.total')} style={{ textAlign: 'center' }} />
Line range hint
52-52: Translate the search placeholder default valueThe default value for searchPlaceholder should also be translated for consistency with the rest of the internationalized strings.
export function TransactionListWithBalances({ isLoading, transactions, balance, balanceCleared, balanceUncleared, - searchPlaceholder = 'Search...', + searchPlaceholder = t('transactions.search.placeholder'), onSearch, // ... })packages/desktop-client/src/components/reports/reports/CustomReportListCards.tsx (1)
52-52: Consider using a structured translation key.While using the English text directly as the translation key works, consider using a structured key (e.g.,
reports.custom.deleted) for better maintainability and easier key management.-{t('This custom report has been deleted.')} +{t('reports.custom.deleted')}packages/desktop-client/src/components/rules/RuleRow.tsx (1)
Line range hint
1-176: Consider translating remaining static strings and adding RTL support.While the current changes are good, there are additional opportunities for internationalization:
Additional strings that could be translated:
- "Apply to all" text
- Stage label
- Condition operation text (from friendlyOp)
RTL (Right-to-Left) considerations:
- The arrow icon (
SvgRightArrow2) direction might need to be flipped for RTL languages- Stack and flex layouts might need RTL-aware alignment
Example implementation for RTL support:
import { useTranslation } from 'react-i18next'; // Inside component: const { t, i18n } = useTranslation(); const isRTL = i18n.dir() === 'rtl'; // For the arrow: <SvgRightArrow2 style={{ width: 12, height: 12, color: theme.tableText, transform: isRTL ? 'rotate(180deg)' : undefined }} />packages/desktop-client/src/components/settings/index.tsx (1)
46-47: Consider using structured translation keys.While the strings are correctly wrapped with t(), consider using structured translation keys instead of direct strings for better maintainability. For example:
-<strong>{t('Actual')}</strong> -{t(' is a super fast privacy-focused app for managing your finances.')} +<strong>{t('about.app_name')}</strong> +{t('about.description')}This approach:
- Makes it easier to manage translations across different languages
- Provides better context for translators
- Reduces the risk of duplicate translations
Also applies to: 73-73, 77-77, 86-86
packages/desktop-client/src/components/modals/MergeUnusedPayeesModal.jsx (2)
88-94: Consider using a more structured translation key.The translation implementation is correct, but consider using a structured key like 'modals.merge_unused_payees.single_payee_message' to maintain a clear hierarchy in your translation files.
172-172: Consider using common button label translations.These button labels might be reused across different modals. Consider moving them to a common 'buttons' namespace in your translation files for better reusability.
- {t('Merge')} + {t('buttons.merge')} - {t('Merge and edit rule')} + {t('buttons.mergeAndEditRule')} - {t('Do nothing')} + {t('buttons.cancel')}Also applies to: 182-182, 186-186
packages/desktop-client/src/components/modals/CreateLocalAccountModal.tsx (2)
76-78: Consider using structured translation keys.While the translation function is correctly implemented, consider using a structured key format (e.g., 'modals.createAccount.title') instead of plain text. This helps organize translations and avoid potential conflicts.
- <ModalTitle title={t('Create Local Account')} shrinkOnOverflow /> + <ModalTitle title={t('modals.createAccount.title')} shrinkOnOverflow />
180-186: Consider using common button text translations.While the translations are correct, consider using common keys for frequently used button texts (e.g., 'common.buttons.back', 'common.buttons.create'). This promotes reusability and consistency across the application.
- <Button onPress={close}>{t('Back')}</Button> + <Button onPress={close}>{t('common.buttons.back')}</Button> <Button type="submit" variant="primary" style={{ marginLeft: 10 }} > - {t('Create')} + {t('common.buttons.create')} </Button>packages/desktop-client/src/components/modals/CategoryGroupMenuModal.tsx (2)
Line range hint
228-228: Missing translation for "Delete" textThe "Delete" text is still hardcoded and should be wrapped with the translation function for consistency.
Apply this change:
- text: 'Delete', + text: t('Delete'),
Line range hint
1-246: Consider implementing translation key constantsTo maintain consistency and ease maintenance, consider creating a constants file for all translation keys. This would help prevent typos and make it easier to track all translatable strings.
Example structure:
// translations/keys.ts export const TRANSLATION_KEYS = { NOTES: { EMPTY: 'No notes' }, ACTIONS: { ADD_CATEGORY: 'Add category', EDIT_NOTES: 'Edit notes', DELETE: 'Delete' }, // ... } as const;packages/desktop-client/src/components/mobile/transactions/Transaction.jsx (2)
83-83: Consider using a more specific translation keyWhile the translation works correctly, consider using a more specific key like 'transaction.accessibility.longPress' to better organize translations hierarchically.
- accessibilityDescription: t('Long press to select multiple transactions'), + accessibilityDescription: t('transaction.accessibility.longPress'),
181-181: Make empty state translation more contextualThe generic 'Empty' key might be too ambiguous when used across different components. Consider using a more specific key that includes the context.
- {prettyDescription || t('Empty')} + {prettyDescription || t('transaction.description.empty')}packages/desktop-client/src/components/modals/AccountMenuModal.tsx (1)
Line range hint
1-277: Consider implementing i18n type safety.While the translation implementation is correct, consider enhancing type safety for translations:
- Use TypeScript's type system to catch missing translations at compile time
- Consider using tools like
typesafe-i18nori18next-typescript- Implement a translation key management system to track and validate keys
This will help prevent runtime errors and maintain translation consistency as the application grows.
packages/desktop-client/src/components/mobile/accounts/Accounts.jsx (1)
142-144: Consider adding a translator comment.While the translation is correctly implemented, this is a complex message that might benefit from a translator comment explaining the context and any variables that might need special handling in different languages.
{t( + // Translator: This message appears when no accounts exist. It explains the purpose + // of accounts and the options available (linking vs. manual management). 'For Actual to be useful, you need to add an account. You can link an account to automatically download transactions, or manage it locally yourself.', )}packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx (2)
73-75: Add translation context for better clarityWhile the translations are implemented correctly, consider adding context for translators using i18next's
t('key', "Default value", { context: "..." }). This helps translators understand where and how these strings are used.Example:
-isRecreating ? t('Generate new key') : t('Enable encryption') +isRecreating ? t('Generate new key', { context: "Modal title for regenerating encryption key" }) : t('Enable encryption', { context: "Modal title for initial encryption setup" })
Line range hint
89-155: Refactor translations to follow i18next best practicesSeveral improvements could enhance translation quality and maintainability:
- Split long text blocks into smaller, more manageable chunks
- Extract HTML markup from translations
- Use translation keys instead of English text as keys
Example refactor:
-<strong>{t('Important:')}</strong>{' '} -{t('if you forget this password')} <em>{t('and')}</em>{' '} -{t('you don't have any local copies of your data, you will lose access to all your data. The data cannot be decrypted without the password.')} +<Trans i18nKey="encryption.warning"> + <strong>Important:</strong> if you forget this password <em>and</em> + you don't have any local copies of your data, you will lose access to all your data. + The data cannot be decrypted without the password. +</Trans>This approach:
- Uses the
Transcomponent to handle HTML markup- Makes translations more maintainable
- Preserves HTML structure across translations
packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx (1)
129-135: Consider adding translator context for the description text.While the translation implementation is correct, consider adding a description context for translators to better understand where and how this text is used.
- {t( - 'We found the following accounts. Select which ones you want to add:', - )} + {t( + 'We found the following accounts. Select which ones you want to add:', + { context: 'Description in account linking modal' } + )}packages/desktop-client/src/components/reports/CategorySelector.tsx (1)
107-107: Consider translating category and group namesThe button text translations are correctly implemented. However, I notice that category names and group names are still displayed without translations:
<label htmlFor={`form_${categoryGroup.id}`}>{categoryGroup.name}</label> // and <label htmlFor={`form_${category.id}`}>{category.name}</label>Consider whether these names should also be translated, depending on your internationalization requirements.
If category/group names need translation, you could modify the data structure to include translation keys or implement a naming convention for automatic key generation:
// Example approach 1: Include translation keys in data <label htmlFor={`form_${categoryGroup.id}`}> {t(categoryGroup.nameKey || categoryGroup.name)} </label> // Example approach 2: Generate keys based on naming convention <label htmlFor={`form_${categoryGroup.id}`}> {t(`category_group.${categoryGroup.name}`)} </label>Also applies to: 117-117
packages/desktop-client/src/components/ManageRules.tsx (1)
Line range hint
328-330: Add translation for delete button with proper pluralization.The delete button text is not translated and needs proper pluralization support.
{selectedInst.items.size > 0 && ( <Button onPress={onDeleteSelected}> - Delete {selectedInst.items.size} rules + {t('rules.actions.delete', { + count: selectedInst.items.size, + defaultValue: 'Delete {{count}} rule', + defaultValue_plural: 'Delete {{count}} rules' + })} </Button> )}packages/desktop-client/src/components/Titlebar.tsx (1)
Line range hint
37-64: Missing translations in UncategorizedButton component.The UncategorizedButton component contains hardcoded English strings that should be translated.
Apply this diff to internationalize the strings:
<Link variant="button" buttonVariant="bare" to="/accounts/uncategorized" style={{ color: theme.errorText, }} > - {count} uncategorized {count === 1 ? 'transaction' : 'transactions'} + {t('uncategorized_count', { count, defaultValue: '{{count}} uncategorized {{count, plural, one {transaction} other {transactions}}}' })} </Link>packages/desktop-client/src/components/modals/CloseAccountModal.tsx (3)
190-192: Consider breaking down long translation strings.The explanation text for category selection is quite long. Consider breaking it into smaller, more manageable translation keys to improve maintainability and reusability.
Example approach:
{t('transfer.category.required')} {t('transfer.category.explanation')}
Line range hint
230-245: Review force close warning message structure.The current implementation splits the force close warning into multiple translation keys with concatenation. This approach might cause issues in languages with different word orders.
Suggested fix:
- {t('You can also')} - <Link>{t(' force close')}</Link>{' '} - {t('the account which will...')} + {t('force_close_warning', { + action: <Link variant="text" onClick={...} style={{...}}> + {t('force_close_action')} + </Link> + })}
159-159: Consider consistent placeholder translations.The placeholder texts for account and category selection are properly translated, but consider using a common pattern for input placeholders across the application.
Example approach:
placeholder: t('common.select_placeholder', { type: t('common.account') }) placeholder: t('common.select_placeholder', { type: t('common.category') })Also applies to: 199-199
packages/desktop-client/src/components/modals/KeyboardShortcutModal.tsx (1)
Fixed width constraints may cause layout issues with translations
The component has some fixed width constraints that could cause issues with translated text:
minWidth: 30andminHeight: 30for keyboard shortcut icons - likely safe as these contain single charactersmaxWidth: 300for description text container - could truncate longer translationsTo prevent layout issues:
- Consider making the description container width more flexible
- Test with languages known for longer word lengths (e.g., German)
- Add text overflow handling (ellipsis) for the description container
🔗 Analysis chain
Line range hint
1-364: Addressing visual regression concernsRegarding the visual regression concerns mentioned in the PR comments: This file's changes only involve text content replacement with translations and don't modify any styling properties. However, it's important to verify that:
- The translated text doesn't cause layout shifts with longer text
- The keyboard shortcut layout remains consistent across different languages
To help identify potential layout issues, we can check for fixed-width containers:
Let me search for any width/height constraints in the specific file to better assess potential layout issues with translations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for fixed width/height constraints that might cause issues with longer translated text rg "width:|height:" --type tsx --type jsxLength of output: 71
Script:
#!/bin/bash # Search for fixed width/height constraints in the modal component rg "width|height|minWidth|minHeight|maxWidth" "packages/desktop-client/src/components/modals/KeyboardShortcutModal.tsx"Length of output: 189
packages/desktop-client/src/components/reports/ReportSidebar.tsx (4)
Line range hint
213-231: Consider separating punctuation from translation stringsThe colons are currently included in the translation strings (e.g.,
t('Split:'),t('Type:')). This might cause issues in languages with different punctuation rules or positioning.Consider separating the colon from the translation string:
- {t('Split:')} + {`${t('Split')}:`} - {t('Type:')} + {`${t('Type')}:`}
Line range hint
251-293: Ensure consistent translation coverage
The colon in "Interval:" should be separated from the translation string.
The menu items in the Options popover (e.g., "Show hidden categories", "Show empty rows") are not wrapped with translation functions.
Separate the colon:
- {t('Interval:')} + {`${t('Interval')}:`}
- Add translations for menu items:
items={[ { name: 'include-current-interval', - text: 'Include current ' + + text: t('Include current ') + ( ReportOptions.dateRangeType.get( customReportItems.dateRange, ) || '' ).toLowerCase(), - tooltip: 'Include current ' + + tooltip: t('Include current ') + ( ReportOptions.dateRangeType.get( customReportItems.dateRange, ) || '' ).toLowerCase() + - ' in live range', + t(' in live range'), toggle: customReportItems.includeCurrentInterval, disabled: customReportItems.isDateStatic || disabledList.currentInterval.get( customReportItems.dateRange, ), }, { name: 'show-hidden-categories', - text: 'Show hidden categories', - tooltip: 'Show hidden categories', + text: t('Show hidden categories'), + tooltip: t('Show hidden categories'), toggle: customReportItems.showHiddenCategories, }, // ... Apply similar changes to other menu items
Line range hint
407-455: Add missing translations for date labelsThe date-related labels "From:" and "To:" are not internationalized.
Add translations for the missing labels:
<Text style={{ width: 50, textAlign: 'right', marginRight: 5 }}> - From: + {`${t('From')}:`} </Text> <Text style={{ width: 50, textAlign: 'right', marginRight: 5 }}> - To: + {`${t('To')}:`} </Text>
Found untranslated strings that need to be wrapped with t() function
The following strings in ReportSidebar.tsx need to be translated:
- "From:" in the date range selector
- "To:" in the date range selector
- Menu items in the options dropdown:
- "Show hidden categories"
- "Show empty rows"
- "Show off budget"
- "Show uncategorized"
🔗 Analysis chain
Line range hint
1-573: Verify complete translation coverageLet's ensure we haven't missed any hardcoded strings that should be translated.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find potential untranslated strings in the component # Note: This is a basic check and might have false positives # Look for text content within JSX that might need translation rg -U "(?<=<[^>]*>)[^<>{}\n]+(?=</)" "packages/desktop-client/src/components/reports/ReportSidebar.tsx" # Look for string literals in props that might need translation rg "(?<=['\"])[^'\"]+(?=['\"])" "packages/desktop-client/src/components/reports/ReportSidebar.tsx"Length of output: 669
Script:
#!/bin/bash # Let's try a different approach to find potential untranslated strings # Look for string literals in the file, excluding imports and type definitions # Search for text content in JSX and string literals rg --pcre2 "(?<!import\s)['\"](.*?)['\"]" "packages/desktop-client/src/components/reports/ReportSidebar.tsx" # Also look for specific component text that might need translation rg -A 1 "<Text" "packages/desktop-client/src/components/reports/ReportSidebar.tsx" # Look for hardcoded strings in menu items rg "text: ['\"](.*?)['\"]" "packages/desktop-client/src/components/reports/ReportSidebar.tsx"Length of output: 7815
packages/desktop-client/src/components/reports/reports/CustomReport.tsx (1)
770-774: Consider combining the warning message strings.While the translation implementation is correct, the warning message is split into multiple
tfunction calls. Consider combining them for better translation context:- {t( - 'This report is configured to use a non-existing filter value', - )} - <LoadingIndicator message={t('Loading report...')} /> - {t('(i.e. category/account/payee)')} + {t('This report is configured to use a non-existing filter value (i.e. category/account/payee)')} + <LoadingIndicator message={t('Loading report...')} />This change would:
- Provide better context for translators
- Ensure consistent translation of the complete message
- Reduce the number of strings to translate
packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.jsx (1)
730-730: Consider using template literals for complex translations.While the translations are correctly implemented, consider using template literals for strings that combine translations with variables. For example:
-t('Import transactions') + (filetype ? ` (${filetype.toUpperCase()})` : '') +t('Import transactions {{type}}', { type: filetype ? ` (${filetype.toUpperCase()})` : '' })This approach:
- Makes translations more maintainable
- Allows translators to reorder variables if needed
- Provides better context for translators
Also applies to: 771-771, 806-811
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx (1)
Line range hint
19-1021: Consider namespacing translation keysWhile the translation implementation is functional, consider using namespaced translation keys to avoid potential conflicts in a large application. For example:
t('Split')→t('transaction.actions.split')t('Amount')→t('transaction.fields.amount')This makes the translation structure more maintainable and reduces the risk of key collisions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (68)
packages/desktop-client/src/components/ManageRules.tsx(5 hunks)packages/desktop-client/src/components/ManageRulesPage.tsx(1 hunks)packages/desktop-client/src/components/Notes.tsx(2 hunks)packages/desktop-client/src/components/NotesButton.tsx(2 hunks)packages/desktop-client/src/components/ThemeSelector.tsx(2 hunks)packages/desktop-client/src/components/Titlebar.tsx(4 hunks)packages/desktop-client/src/components/mobile/MobileBackButton.tsx(2 hunks)packages/desktop-client/src/components/mobile/accounts/Accounts.jsx(5 hunks)packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx(24 hunks)packages/desktop-client/src/components/mobile/transactions/AddTransactionButton.tsx(2 hunks)packages/desktop-client/src/components/mobile/transactions/Transaction.jsx(4 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx(15 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionListWithBalances.jsx(4 hunks)packages/desktop-client/src/components/modals/AccountAutocompleteModal.tsx(4 hunks)packages/desktop-client/src/components/modals/AccountMenuModal.tsx(4 hunks)packages/desktop-client/src/components/modals/BudgetListModal.tsx(3 hunks)packages/desktop-client/src/components/modals/CategoryAutocompleteModal.tsx(3 hunks)packages/desktop-client/src/components/modals/CategoryGroupMenuModal.tsx(5 hunks)packages/desktop-client/src/components/modals/CategoryMenuModal.tsx(6 hunks)packages/desktop-client/src/components/modals/CloseAccountModal.tsx(10 hunks)packages/desktop-client/src/components/modals/ConfirmCategoryDeleteModal.tsx(7 hunks)packages/desktop-client/src/components/modals/ConfirmTransactionDeleteModal.tsx(5 hunks)packages/desktop-client/src/components/modals/ConfirmTransactionEditModal.tsx(4 hunks)packages/desktop-client/src/components/modals/ConfirmUnlinkAccountModal.tsx(5 hunks)packages/desktop-client/src/components/modals/CreateAccountModal.tsx(11 hunks)packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx(7 hunks)packages/desktop-client/src/components/modals/CreateLocalAccountModal.tsx(5 hunks)packages/desktop-client/src/components/modals/EditFieldModal.jsx(7 hunks)packages/desktop-client/src/components/modals/EditRuleModal.jsx(17 hunks)packages/desktop-client/src/components/modals/FixEncryptionKeyModal.tsx(7 hunks)packages/desktop-client/src/components/modals/GoCardlessInitialiseModal.tsx(5 hunks)packages/desktop-client/src/components/modals/HoldBufferModal.tsx(4 hunks)packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.jsx(15 hunks)packages/desktop-client/src/components/modals/KeyboardShortcutModal.tsx(9 hunks)packages/desktop-client/src/components/modals/ManageRulesModal.tsx(2 hunks)packages/desktop-client/src/components/modals/MergeUnusedPayeesModal.jsx(6 hunks)packages/desktop-client/src/components/modals/NotesModal.tsx(3 hunks)packages/desktop-client/src/components/modals/PayeeAutocompleteModal.tsx(2 hunks)packages/desktop-client/src/components/modals/ScheduledTransactionMenuModal.tsx(3 hunks)packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx(7 hunks)packages/desktop-client/src/components/modals/SimpleFinInitialiseModal.tsx(5 hunks)packages/desktop-client/src/components/payees/PayeeMenu.tsx(2 hunks)packages/desktop-client/src/components/reports/CategorySelector.tsx(5 hunks)packages/desktop-client/src/components/reports/ReportSidebar.tsx(14 hunks)packages/desktop-client/src/components/reports/ReportSummary.tsx(3 hunks)packages/desktop-client/src/components/reports/ReportTopbar.tsx(9 hunks)packages/desktop-client/src/components/reports/SaveReport.tsx(2 hunks)packages/desktop-client/src/components/reports/SaveReportDelete.tsx(3 hunks)packages/desktop-client/src/components/reports/SaveReportName.tsx(3 hunks)packages/desktop-client/src/components/reports/reports/CustomReport.tsx(3 hunks)packages/desktop-client/src/components/reports/reports/CustomReportListCards.tsx(3 hunks)packages/desktop-client/src/components/reports/reports/GetCardData.tsx(2 hunks)packages/desktop-client/src/components/rules/ActionExpression.tsx(2 hunks)packages/desktop-client/src/components/rules/RuleRow.tsx(3 hunks)packages/desktop-client/src/components/rules/RulesHeader.tsx(2 hunks)packages/desktop-client/src/components/rules/Value.tsx(4 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/Export.tsx(3 hunks)packages/desktop-client/src/components/settings/FixSplits.tsx(3 hunks)packages/desktop-client/src/components/settings/Format.tsx(5 hunks)packages/desktop-client/src/components/settings/Reset.tsx(3 hunks)packages/desktop-client/src/components/settings/Themes.tsx(2 hunks)packages/desktop-client/src/components/settings/UI.tsx(3 hunks)packages/desktop-client/src/components/settings/index.tsx(7 hunks)packages/desktop-client/src/components/transactions/SimpleTransactionsTable.jsx(2 hunks)packages/desktop-client/src/components/util/AmountInput.tsx(2 hunks)packages/desktop-client/src/global-events.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/desktop-client/src/components/modals/CategoryMenuModal.tsx
🧰 Additional context used
📓 Learnings (5)
packages/desktop-client/src/components/mobile/accounts/Accounts.jsx (2)
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-10-08T15:46:15.739Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is acceptable and commonly used in the codebase.
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-09-28T17:08:11.028Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is acceptable and commonly used in the codebase.
packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx (2)
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-10-08T15:46:15.739Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is acceptable and commonly used in the codebase.
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-09-28T17:08:11.028Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is acceptable and commonly used in the codebase.
packages/desktop-client/src/components/modals/CreateLocalAccountModal.tsx (2)
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/modals/CreateLocalAccountModal.tsx:47-47
Timestamp: 2024-09-28T17:03:43.286Z
Learning: Validating balance is outside the scope in `CreateLocalAccountModal.tsx`.
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/modals/CreateLocalAccountModal.tsx:47-47
Timestamp: 2024-10-08T15:46:15.739Z
Learning: Validating balance is outside the scope in `CreateLocalAccountModal.tsx`.
packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.jsx (2)
Learnt from: MatissJanis
PR: actualbudget/actual#3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/utils.ts:120-130
Timestamp: 2024-10-08T15:46:15.739Z
Learning: Avoid overcomplicating the `ImportTransaction` type; prefer to keep it simple.
Learnt from: MatissJanis
PR: actualbudget/actual#3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/utils.ts:120-130
Timestamp: 2024-10-05T10:59:08.817Z
Learning: Avoid overcomplicating the `ImportTransaction` type; prefer to keep it simple.
packages/desktop-client/src/components/payees/PayeeMenu.tsx (2)
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-10-08T15:46:15.739Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is acceptable and commonly used in the codebase.
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-09-28T17:08:11.028Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is acceptable and commonly used in the codebase.
🪛 Biome
packages/desktop-client/src/components/modals/EditRuleModal.jsx
[error] 526-530: 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)
🔇 Additional comments (161)
packages/desktop-client/src/components/ManageRulesPage.tsx (1)
10-10: Verify translation key exists and check for visual regressions.
Since this is a header component, ensure the translation exists and the UI can handle varying text lengths.
✅ Verification successful
Translation key 'Rules' appears to be properly integrated with i18next
Based on the evidence from the codebase:
- The component correctly imports and uses the
tfunction from i18next - The translation system is properly initialized (i18n.ts exists)
- The key is used consistently across the application (ManageRules, RulesList components)
- Similar Page components also use the same translation pattern
- The UI handles varying text lengths as evidenced by other translated headers in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the translation key exists in translation files
fd -e json -e yaml . | xargs rg -l "Rules"
# Look for other usages of the Page component with headers to compare implementation
ast-grep --pattern 'Page header={$_}'
Length of output: 82
Script:
#!/bin/bash
# Search for translation files in common locations
fd -e json -e yaml -p "translations\|locales\|i18n"
# Look for translation key usage and definition
rg -l "Rules" --type json --type yaml
# Check Page component usage with header prop
rg -A 2 '<Page\s+header='
Length of output: 924
Script:
#!/bin/bash
# Search for translation function usage and imports
rg -A 2 "import.*\{.*t.*\}.*from.*" packages/desktop-client/src/components/ManageRulesPage.tsx
# Look for translation configuration or setup files
fd -e ts -e js . | xargs rg -l "i18n.*init|createI18n|translation.*config"
# Check if Rules key exists in any translation related files
rg -i "rules.*:.*{|rules.*:.*\"" --type json --type yaml
Length of output: 423
Script:
#!/bin/bash
# Search for i18next configuration and initialization
rg -l "i18next" packages/desktop-client/src/
# Look for translation files with fixed regex
rg -i "\"rules\":|'rules':" --type json --type yaml
# Check how translation is used in other Page components
rg "import.*\{.*t.*\}.*from.*i18next" packages/desktop-client/src/components/ -A 1
Length of output: 45115
Script:
#!/bin/bash
# Check i18n initialization and configuration
rg -A 5 "i18n.*init" packages/desktop-client/src/i18n.ts
# Look for default English translations
fd -e json -e ts -e js . | xargs rg -l "en.*translation|defaultNS|resources.*en"
# Check for Rules string in English resources
rg -i "rules.*:" packages/desktop-client/src/ --type json --type ts --type js
Length of output: 1224
packages/desktop-client/src/components/rules/RulesHeader.tsx (2)
3-4: LGTM: Clean import addition
The i18next import is properly placed following the React import.
22-23: Verify text overflow handling for translated content
The 'Stage' column has a fixed width of 50 pixels. Translated text might be longer than the English version and could potentially overflow or get truncated.
Consider implementing one of these solutions:
- Use flexible widths that adapt to content
- Add text overflow handling (ellipsis)
- Ensure translations maintain similar length constraints
✅ Verification successful
Let me try a different search pattern to find the Cell component implementation.
Let me try another search approach without file type specification.
Let me try one final search to locate the Cell component using a different pattern.
Let me try another search pattern to find the Cell component implementation.
Based on the code I've found, I can now provide a response regarding the text overflow concern in the review comment.
The Cell component properly handles text overflow
The Cell component already implements proper text overflow handling through the UnexposedCellContent component, which has the following styles:
{
flexGrow: 1,
whiteSpace: 'nowrap',
overflow: 'hidden',
textOverflow: 'ellipsis'
}Additionally, the Field component used for headers also implements text truncation:
<Text style={{
whiteSpace: 'nowrap',
overflow: 'hidden',
textOverflow: 'ellipsis'
}}>
{children}
</Text>These styles ensure that any content, including translated text, will be properly truncated with an ellipsis if it overflows the cell width. The fixed width of 50px for the 'Stage' column is safe to use since:
- The text "Stage" and its translations are typically short
- The proper overflow handling is in place
- The column width is reasonable for a label column
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Cell component implementation to check overflow handling
ast-grep --pattern 'const Cell = $_' -A 10
Length of output: 13339
Script:
#!/bin/bash
# Search for Cell component implementation in TypeScript/JavaScript files
rg -t ts -t tsx -t js "const Cell = .*=>" -A 5
Length of output: 76
Script:
#!/bin/bash
# Search for Cell component implementation
rg "const Cell = " -A 10
Length of output: 26
Script:
#!/bin/bash
# Search for files containing Cell component and look for its implementation
fd -e tsx -e ts -e js | xargs grep -l "Cell" | xargs cat
Length of output: 402114
Script:
#!/bin/bash
# Search for Cell component implementation in all files
fd -e tsx -e ts -e js | xargs grep -l "export.*Cell" | xargs cat
Length of output: 71384
packages/desktop-client/src/components/mobile/transactions/AddTransactionButton.tsx (1)
3-3: LGTM: Clean i18next integration
The import statement follows best practices for i18next integration.
packages/desktop-client/src/components/mobile/MobileBackButton.tsx (2)
3-3: LGTM: Clean i18next import
The import of the translation function is correctly implemented.
40-40: Verify translation key existence
The implementation of the translation function is correct. However, ensure that the 'Back' translation key is defined in all language files to prevent fallback text issues.
packages/desktop-client/src/components/modals/ManageRulesModal.tsx (1)
5-5: LGTM! Translation implementation looks good.
The i18next integration and translation of the modal title are implemented correctly.
Also applies to: 31-31
packages/desktop-client/src/components/modals/BudgetListModal.tsx (3)
4-4: LGTM: Import statement is correctly placed
The i18next import is appropriately placed with other external dependencies.
23-23: LGTM: Modal title properly internationalized
The modal title is correctly wrapped with the translation function.
Line range hint 1-44: Verify potential visual regressions
As mentioned in the PR comments, translation changes shouldn't cause visual regressions. Please verify that the text wrapping and layout remain consistent across different languages, especially for longer translated strings.
✅ Verification successful
No visual regression concerns with text styling
Based on the analysis of similar modal components and text styling patterns across the codebase:
- The text styling in
BudgetListModalfollows a consistent pattern used in other modals (e.g.,ScheduledTransactionMenuModal) with the same font sizes (17px) and font weights (400/700) - The modal layout uses flexible container with
justifyContent: 'center'andalignItems: 'center'which accommodates text of varying lengths - The text components are wrapped in a
Viewwith margin spacing that prevents content from touching modal edges
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar modal components to compare text styling patterns
ast-grep --pattern 'Modal name=$_'
# Look for other translation implementations in modal components
rg "import.*i18next" --type typescript --type typescript-react
Length of output: 136
Script:
#!/bin/bash
# Search for similar modal components to compare text styling patterns
ast-grep --pattern '<Modal name="$_">'
# Look for translation imports and usage in modal components
rg "import.*\{ t \}.*from.*i18next"
# Find similar text styling patterns in modals
rg "Text.*style.*fontSize.*fontWeight" -A 2 -B 2
# Search for other modal components with translated text
rg "<Modal.*>.*\{t\(" -A 5
Length of output: 12853
packages/desktop-client/src/components/ThemeSelector.tsx (1)
3-3: LGTM: Clean i18next integration
The import of the translation function follows standard i18next practices.
packages/desktop-client/src/components/settings/BudgetTypeSettings.tsx (2)
3-3: 🛠️ Refactor suggestion
Consider using useTranslation hook instead of direct t import
While importing t directly works, using the useTranslation hook from react-i18next is the recommended approach in React components as it ensures proper updates when the language changes and provides better type safety.
-import { t } from 'i18next';
+import { useTranslation } from 'react-i18next';
export function BudgetTypeSettings() {
+ const { t } = useTranslation();⛔ Skipped due to learnings
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-10-08T15:46:15.739Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is acceptable and commonly used in the codebase.
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-09-28T17:08:11.028Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is acceptable and commonly used in the codebase.
Line range hint 1-57: Verify all strings are marked for translation
Let's ensure no hardcoded strings were missed in the translation process.
✅ Verification successful
All strings are properly marked for translation
Looking at the output, the only non-translated strings found are:
- 'rollover', 'report' - these are technical enum values that shouldn't be translated
- HTML tags and component props
- The long descriptive strings are actually wrapped in
t()function calls (they appear in the output because they're part of larger expressions)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for potential hardcoded strings in the component
rg -U "(?<![a-zA-Z])'[^']{3,}'|(?<![a-zA-Z])\"[^\"]{3,}\"" "packages/desktop-client/src/components/settings/BudgetTypeSettings.tsx" \
| grep -v "import\|from\|to=\|variant=\|linkColor="
Length of output: 441
Script:
#!/bin/bash
# Search for potential hardcoded strings in the component using a simpler pattern
rg -U "'[^']{3,}'|\"[^\"]{3,}\"" "packages/desktop-client/src/components/settings/BudgetTypeSettings.tsx" \
| grep -v "import\|from\|to=\|variant=\|linkColor=" \
| grep -v "t("
Length of output: 942
packages/desktop-client/src/components/modals/ConfirmUnlinkAccountModal.tsx (3)
2-2: LGTM: Translation hook setup is correct
The useTranslation hook is properly imported and initialized at the component level, following React hooks best practices.
Also applies to: 19-20
51-51: LGTM: Button translations are properly implemented
The translation of button text is implemented correctly and uses clear, maintainable translation keys.
Also applies to: 61-61
Line range hint 1-70: Verify translation string lengths don't affect modal layout
Given the concerns about visual regression, please verify that the translated strings (especially longer translations) don't cause layout issues in the modal, particularly:
- The modal title staying on one line
- The button container maintaining proper alignment
- The overall modal width adapting properly
✅ Verification successful
Let me analyze the modal width constraints found in the codebase:
- The
ConfirmUnlinkAccountModaluseswidth: '30vw'which is a responsive width (30% of viewport width) - Several other confirmation modals (ConfirmTransactionEditModal, ConfirmCategoryDeleteModal, CloseAccountModal) use the same
30vwwidth - No fixed width constraints are applied to internal elements that could cause text overflow
- The modal uses flexbox layout with
flexDirection: 'row'andjustifyContent: 'flex-end'for button alignment
Let me verify the translation configuration to ensure proper handling of longer strings:
Let me check the i18n configuration and translation files to understand how string lengths are handled:
Let me check the correct locale directory path based on the i18n configuration:
Based on the analysis of the Modal component and the ConfirmUnlinkAccountModal implementation, I can now provide a final response:
Modal layout is resilient to translation string lengths
The implementation is safe from layout issues with translated strings because:
- The modal uses a responsive width (
30vw) with a maximum constraint (maxWidth: '90vw') - The Modal component includes text overflow handling:
- Modal title uses
TextOneLinecomponent that handles overflow - Content uses flexible layouts with
flexproperties
- Modal title uses
- Button container uses
flexDirection: 'row'withjustifyContent: 'flex-end'which adapts to button text lengths
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any fixed width constraints in the modal that might cause issues with longer translations
rg -g '*.{tsx,jsx,css,scss}' 'width:|minWidth:|maxWidth:' packages/desktop-client/src/components/modals/
Length of output: 10020
Script:
#!/bin/bash
# Check i18n configuration and available translations
rg -g '*.{json,ts,js}' -l "i18next" packages/desktop-client/src/
Length of output: 264
Script:
#!/bin/bash
# Check i18n configuration and translation files
cat packages/desktop-client/src/i18n.ts
# Look for translation files
fd -e json . packages/desktop-client/src/locales
Length of output: 1062
Script:
#!/bin/bash
# Check for locale directory and translation files
fd -e json . packages/desktop-client/src/locale
# Also check if there are any fixed width styles in the Modal component itself
cat packages/desktop-client/src/components/common/Modal.tsx
Length of output: 11639
packages/desktop-client/src/components/modals/ConfirmTransactionDeleteModal.tsx (3)
1-2: LGTM: Import added correctly
The useTranslation hook is properly imported from react-i18next.
21-21: LGTM: Translation hook initialized properly
The useTranslation hook is correctly destructured to obtain the translation function.
Line range hint 28-45: Verify no visual regressions
The i18n changes should not affect the layout, but translated text length might impact the UI. Please verify:
- Modal layout remains consistent with longer translated text
- Buttons maintain proper spacing and alignment across languages
✅ Verification successful
No visual regression concerns for translated text in ConfirmTransactionDeleteModal
The modal component has no hardcoded width constraints or text truncation mechanisms that could cause issues with translated text. Other modals in the codebase that need width constraints explicitly set them (e.g., maxWidth: 500), but this modal intentionally uses flexible layout:
- Uses flexible
flexDirection: 'row'for button alignment - No fixed widths or text truncation
- Content area uses natural text wrapping
The design will accommodate varying text lengths across translations while maintaining proper layout and spacing.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for any hardcoded dimensions or truncation in the component
rg -t tsx -t jsx "maxWidth|truncate|ellipsis|overflow" packages/desktop-client/src/components/modals/
Length of output: 131
Script:
#!/bin/bash
# Look for any hardcoded dimensions or truncation in the component using ripgrep without file type filters
rg "maxWidth|truncate|ellipsis|overflow" packages/desktop-client/src/components/modals/ConfirmTransactionDeleteModal.tsx
# Check modal styling patterns across the codebase
rg "style=\{.*?(width|maxWidth|overflow)" packages/desktop-client/src/components/modals/
# Look for any existing text truncation or overflow handling in modal components
ast-grep --pattern 'style={{ $$$, overflow: $_, $$$}}'
Length of output: 2807
packages/desktop-client/src/components/payees/PayeeMenu.tsx (1)
1-1: LGTM! Translation implementation looks good.
The import of t and its usage for translating the ', and more' string is correctly implemented and follows the project's conventions.
Also applies to: 62-62
packages/desktop-client/src/components/settings/Export.tsx (2)
4-4: LGTM: Import statement is correctly added
The i18next import is properly added and follows the standard pattern.
Line range hint 1-74: Verify potential visual regressions
As mentioned in the PR comments, we should verify that these translation changes don't cause any visual regressions, especially with longer translated texts.
packages/desktop-client/src/components/settings/UI.tsx (2)
5-5: LGTM: i18next import added correctly
The import of the translation function is properly added.
77-77: Verify text wrapping and layout with translated strings
While the translation implementation is correct, translated strings might be longer than the original English text, which could affect the layout. Please ensure that:
- The container can handle longer translated text without breaking the layout
- Text wrapping works correctly for both the heading and link
- The vertical spacing remains consistent regardless of text length
Let's check other components for consistent translation patterns:
Also applies to: 93-93
✅ Verification successful
Let me gather more context about the UI component and its layout implementation.
Let me check the component's styling system and container implementation.
Layout is properly handled for translated text
The implementation has proper layout handling for translated text:
- The container View uses flexbox with
alignItems: 'flex-start'and proper gap spacing - Text container has
flexShrink: 0to prevent unwanted text compression - The component is responsive with proper width handling through media queries
- Both heading and link components have appropriate styling for text wrapping:
- Heading: Uses
fontSize: 20withfontWeight: 500in a flex container - Link: Has
flexShrink: 0andalignSelf: 'flex-start'for proper text wrapping
- Heading: Uses
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check translation patterns across components
# Look for similar heading/link translations to ensure consistency
# Search for translation usage in component files
rg "t\(['\"]Advanced" packages/desktop-client/src/components/
rg "t\(['\"]Show.*settings" packages/desktop-client/src/components/
Length of output: 316
Script:
#!/bin/bash
# Check the UI component implementation for layout handling
rg -A 10 "Advanced Settings" packages/desktop-client/src/components/settings/UI.tsx
# Look for styling related to this component
rg "const.*Style|styled\." packages/desktop-client/src/components/settings/UI.tsx
# Check for any existing i18n tests or snapshots
fd ".*test.*|.*spec.*" packages/desktop-client/src/components/settings/
Length of output: 474
Script:
#!/bin/bash
# Check the View component implementation for layout handling
ast-grep --pattern 'const View = styled.$_'
# Look for the Link component styling
ast-grep --pattern 'const Link = styled.$_'
# Check the entire UI.tsx file content to understand the component structure
cat packages/desktop-client/src/components/settings/UI.tsx
Length of output: 2402
packages/desktop-client/src/components/modals/NotesModal.tsx (1)
83-83: Verify button layout with longer translations
The button text length will vary across languages. Ensure the button layout can handle longer translations without breaking the design.
✅ Verification successful
The previous pattern might have been too specific. Let's try a broader search to understand how translations are handled in buttons and similar components, and also examine the modal implementation patterns.
Let's try simpler search patterns without type specifications and examine the actual implementation of the NotesModal component to understand the button layout.
We need to examine the Button component implementation and similar button patterns to understand how the layout is handled with icons and translations.
Button layout handles translations appropriately
After examining the Button implementation and NotesModal usage, I can confirm that the button layout will handle longer translations properly because:
- The Button component uses flexbox layout with
alignItems: 'center'andjustifyContent: 'center' - The button container has
width: '100%'in NotesModal, allowing it to expand - The icon and text are wrapped in a flex container that handles spacing automatically
- The icon's padding is fixed but won't affect text wrapping or overflow
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar button implementations to see how they handle translation length
ast-grep --pattern 'Button $$$>{t($$$)}</Button>'
Length of output: 51
Script:
#!/bin/bash
# Let's try a different pattern to find button components with translations
ast-grep --pattern '$_>{t($_)}</$_'
# Also search for the specific NotesModal component to understand the button implementation
rg -l "NotesModal" --type typescript
# And look for other modal implementations for comparison
rg -l "Modal" --type typescript
Length of output: 176
Script:
#!/bin/bash
# Search for NotesModal implementation
rg -l "NotesModal"
# Look for translation usage in buttons across the codebase
rg -l "Button.*{t\("
# Get the content of NotesModal to understand the button implementation
fd "NotesModal" --exec cat {}
Length of output: 2687
Script:
#!/bin/bash
# Let's look at the Button component implementation to understand its layout handling
fd "Button2" --exec cat {}
# Also search for other similar buttons with icons to see layout patterns
rg -l "SvgCheck.*style.*padding"
Length of output: 6640
packages/desktop-client/src/components/NotesButton.tsx (2)
9-10: LGTM! Clean import of translation function.
The import is properly placed and separated from other imports.
64-64: Verify translation key exists.
The aria-label translation implementation looks good. However, ensure that the 'View notes' translation key exists in your translation files to prevent missing translation warnings.
packages/desktop-client/src/components/modals/PayeeAutocompleteModal.tsx (2)
3-4: LGTM: Clean i18next import
The import is properly placed and separated from other imports.
56-56: Verify text expansion handling
The translation implementation looks good, but since translated text length can vary significantly across languages, please ensure that the modal title area can handle longer text without breaking the layout.
Let's check if there are any existing text length handling mechanisms in the codebase:
✅ Verification successful
Modal title text overflow is properly handled
The codebase has robust text overflow handling mechanisms:
- The
ModalTitlecomponent has ashrinkOnOverflowprop to handle long text - There's a reusable
TextOneLinecomponent that implements proper text truncation with ellipsis - The modal system uses standard text overflow handling patterns
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for existing text overflow handling patterns in modal titles
rg -l "text-overflow|ellipsis|truncate|nowrap" packages/desktop-client/src/components/common/Modal
Length of output: 192
Script:
#!/bin/bash
# Let's first find the correct modal component path
fd -t f "Modal" packages/desktop-client/src
# Then look for text overflow handling patterns in the entire desktop client
rg -l "text-overflow|ellipsis|truncate|nowrap" packages/desktop-client/src
# Check the specific PayeeAutocompleteModal implementation
rg -A 5 "title=" packages/desktop-client/src/components/modals/PayeeAutocompleteModal.tsx
Length of output: 6030
Script:
#!/bin/bash
# Let's check the ModalTitle component implementation
rg -A 10 "ModalTitle" packages/desktop-client/src/components/common/Modal.tsx
# Also check if there are any existing text overflow handling in common components
cat packages/desktop-client/src/components/common/TextOneLine.tsx
Length of output: 1411
packages/desktop-client/src/components/reports/SaveReportName.tsx (2)
4-4: LGTM: Clean i18next integration
The import of the translation function is properly placed and follows the project's import organization pattern.
Line range hint 67-80: Verify visual alignment after translation
Since these strings will be translated to different languages with varying lengths, ensure that the layout handles longer text gracefully without breaking the UI.
✅ Verification successful
Let me also check for any existing UI components with translations to see how they handle text alignment:
Let me also check the styling system:
Based on the analysis of the codebase, I can now provide a final response regarding the translation concern:
UI layout is properly handled for translations
The implementation shows good practices for handling translated text:
- The Input component is wrapped in a FormField which provides proper layout structure and spacing
- The Button uses flexbox-based layout with margins (
marginTop: 30) rather than fixed widths - The form elements use relative units and flexible layouts:
- FormField uses
flex: 1for proper scaling - Input uses relative margin (
marginTop: 10) - Labels use
userSelect: 'none'for better UX without impacting layout - The parent container appears to use flexbox (
style={{ flex: 1 }}) allowing content to expand naturally
- FormField uses
The existing translations in the codebase follow similar patterns with flexible layouts, and there's no evidence of fixed-width containers that would break with longer translations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for existing translation key patterns to ensure consistency
rg "t\('common\.buttons\." -A 1
rg "t\('reports\." -A 1
Length of output: 65
Script:
#!/bin/bash
# Look for translation patterns without the specific prefix to understand the translation structure
rg "t\(['\"]" -A 1 packages/desktop-client/src/components/reports/
Length of output: 33425
Script:
#!/bin/bash
# Look for Button components with translations
ast-grep --pattern 'Button $$$>{t($$$)}</Button>'
# Look for Input components with translations
ast-grep --pattern 'Input $$$value={t($$$)}$$$'
Length of output: 101
Script:
#!/bin/bash
# Look for style definitions related to forms and buttons
rg "FormField|Button.*style" packages/desktop-client/src/components/
Length of output: 7592
packages/desktop-client/src/components/settings/Reset.tsx (1)
3-4: LGTM: Clean i18next integration
The t function is properly imported from i18next.
packages/desktop-client/src/components/modals/AccountAutocompleteModal.tsx (4)
1-2: LGTM: Clean import addition
The useTranslation hook is properly imported from react-i18next.
25-25: LGTM: Proper hook initialization
The translation hook is correctly initialized at the component level.
49-49: Verify translation key consistency
The translation key 'Account' is used in two locations. While the implementation is correct, ensure this key is:
- Defined in the translation files
- Consistently used across the application for similar contexts
Also applies to: 64-64
✅ Verification successful
Translation key 'Account' is consistently used across the application
The translation key 'Account' is used consistently throughout the codebase for similar contexts, particularly in:
- Table headers and column names
- Modal titles and labels
- Form fields and selectors
- Menu items and buttons
The usage in AccountAutocompleteModal.tsx aligns with the established pattern across the application, where 'Account' is used as a label or title for account-related UI elements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other occurrences of 'Account' translation key
# to verify consistency across the codebase
rg "t\(['\"]Account['\"]\)" --type ts --type tsx
# Look for potential variations that might need alignment
rg "t\(['\"].*[Aa]ccount.*['\"]\)" --type ts --type tsx
Length of output: 172
Script:
#!/bin/bash
# Search for 'Account' translation key usage with proper file type
rg "t\(['\"]Account['\"]\)"
# Look for potential variations of account-related translations
rg "t\(['\"].*[Aa]ccount.*['\"]\)"
# Also check the translation files specifically
fd "translation|i18n|locale" --type f --exec rg -l "Account"
Length of output: 7658
49-52: Confirm style preservation with translations
The component properly maintains styling for translated text:
- ModalTitle preserves color through getStyle
- SectionLabel maintains alignment, color, and margin
Also applies to: 64-69
✅ Verification successful
The previous ast-grep pattern might have been too specific. Let's try a broader search to understand the usage patterns of ModalTitle and the specific styling approach in question.
Style consistency verified across autocomplete modals
The styling approach is consistently implemented across all autocomplete modals:
- AccountAutocompleteModal
- CategoryAutocompleteModal
- PayeeAutocompleteModal
All use the same getStyle={() => ({ color: theme.menuAutoCompleteText })} pattern for their translated titles, ensuring a uniform appearance.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for similar modal titles to verify consistent styling approach
ast-grep --pattern 'ModalTitle title={t($_)} getStyle={$_}'
Length of output: 61
Script:
#!/bin/bash
# Search for ModalTitle usage patterns more broadly
rg "ModalTitle" -A 3 -B 3
# Also search for specific styling patterns
rg "getStyle.*theme\.menuAutoCompleteText" -A 2 -B 2
Length of output: 24245
packages/desktop-client/src/components/modals/HoldBufferModal.tsx (1)
2-2: LGTM: Translation hook setup is correct
The useTranslation hook is properly imported and initialized.
Also applies to: 21-21
packages/desktop-client/src/components/settings/Themes.tsx (2)
4-4: LGTM!
Clean import of the translation function.
95-96:
Refine translation key formatting to prevent visual regressions.
The current implementation has a few issues that could cause visual inconsistencies across different languages:
- Extra spaces in the translation keys
- Leading space in the second translation
- Text splitting that might not work well for all languages
Apply this diff to fix these issues:
- <strong> {t('Themes')} </strong>{' '}
- {t(' change the user interface colors.')}
+ <strong>{t('Themes')}</strong>
+ {' '}
+ {t('change the user interface colors.')}Let's verify if similar spacing issues exist in other translation keys:
✅ Verification successful
Based on the search results and the file content shown, I can now generate the final response:
Spacing issues in translation keys are localized to this component and not widespread.
The review comment correctly identified spacing issues in the translation keys for this specific component. However, after analyzing the codebase:
- The spacing issues are isolated to this component and not a systemic problem
- Most other translation keys follow proper formatting without leading/trailing spaces
- The suggested fix is appropriate for improving consistency and preventing potential visual regressions
The proposed changes will help maintain consistent spacing behavior across different languages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for translation keys with leading/trailing spaces
rg "t\(['\"] .*?['\"]" --type ts --type tsx
rg "t\(['\"](.*?) .*?['\"]" --type ts --type tsx
Length of output: 160
Script:
#!/bin/bash
# Search for translation keys with leading/trailing spaces using ripgrep
# Note: Using -t typescript to match both .ts and .tsx files
rg "t\(['\"] .*?['\"]" -t typescript
rg "t\(['\"](.*?) .*?['\"]" -t typescript
# Also search for similar patterns in the specific file
cat "packages/desktop-client/src/components/settings/Themes.tsx"
Length of output: 2883
Script:
#!/bin/bash
# Search for translation keys in typescript files using a more basic pattern
rg "t\(['\"].*['\"]" --type=ts
# Search specifically for translation keys with spaces
find . -name "*.ts" -o -name "*.tsx" | xargs grep -l "t('.*\s.*')"
find . -name "*.ts" -o -name "*.tsx" | xargs grep -l 't(".*\s.*")'
Length of output: 162611
packages/desktop-client/src/components/modals/CategoryAutocompleteModal.tsx (2)
3-3: LGTM: Import statement is correctly placed.
The i18next import is properly positioned with other imports.
55-55: Verify translation implementation and UI layout.
The translation implementation looks correct. However, since "Category" appears in multiple places, we should verify:
- The translation key exists in the translation files
- The UI layout can handle longer translations in different languages
Also applies to: 70-70
✅ Verification successful
Let me analyze the translation usage further by checking the translation files and UI components.
Let me try one more verification to check the translation files and UI layout components with corrected commands.
Translation and UI layout implementation verified successfully
The implementation is robust and well-handled:
- The "Category" string is consistently translated across multiple components using the
t()function - The modal has proper responsive handling through
useResponsivehook:- Adjusts height based on screen width (
isNarrowWidth ? '90vh' : 275) - Conditionally renders different layouts for narrow/wide screens
- Uses container props to handle layout adjustments
- Adjusts height based on screen width (
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify translation key existence and usage
# Test 1: Check if 'Category' key exists in translation files
echo "Checking for 'Category' key in translation files..."
fd -e json -e yaml . | xargs rg -l "Category"
# Test 2: Find all usages of this translation key for consistency
echo "Checking all usages of Category translation key..."
rg "t\(['\"]Category['\"]" -g '!*.{json,yaml}'
Length of output: 1251
Script:
#!/bin/bash
# Check translation files for "Category" key and its translations
echo "Checking translation files..."
fd -e json -e yaml . -x cat {} | rg -A 1 '"Category":'
# Check UI components that use the Category translation
echo "Checking UI components with Category translation..."
ast-grep --pattern 'title={t("Category")}' -l typescript,javascript
ast-grep --pattern 'title={t('\''Category'\'')}' -l typescript,javascript
# Check container/parent components for layout handling
echo "Checking parent components..."
rg -B 2 -A 2 "CategoryAutocompleteModal" -g "*.{tsx,jsx}"
Length of output: 3254
Script:
#!/bin/bash
# Check translation files in common locations
echo "Checking localization files..."
fd -e json -e yaml . -x grep -l "Category" {} | while read -r file; do
echo "=== $file ==="
cat "$file" | grep -A 2 -B 2 '"Category"'
done
# Check Modal and its parent components for layout handling
echo "Checking modal layout..."
rg -l "Modal|Dialog" packages/desktop-client/src/components/modals/CategoryAutocompleteModal.tsx
rg -B 3 -A 3 "Modal.*title" packages/desktop-client/src/components/modals/CategoryAutocompleteModal.tsx
# Check responsive handling
echo "Checking responsive handling..."
rg "isNarrowWidth|useResponsive" packages/desktop-client/src/components/modals/CategoryAutocompleteModal.tsx
Length of output: 14995
packages/desktop-client/src/components/modals/SimpleFinInitialiseModal.tsx (1)
Line range hint 56-101: Verify layout with longer translations
Given the fixed modal width of 300px and the potential for longer translations in different languages, please verify that:
- The modal title properly handles text overflow
- The instruction text wraps correctly
- The button text doesn't overflow its container
Consider testing with languages known for longer word lengths (e.g., German) to ensure the layout remains intact.
packages/desktop-client/src/components/Notes.tsx (1)
6-6: LGTM! Import statement is correctly placed.
The i18next import follows the standard import ordering convention.
packages/desktop-client/src/components/modals/ScheduledTransactionMenuModal.tsx (3)
7-8: LGTM: Import of i18next translation function.
The import of the t function is correctly placed and necessary for the internationalization changes.
117-122: LGTM: Menu items properly internationalized.
The menu item texts are correctly set up for translation.
66-69: Verify layout with different text lengths.
While the translation implementation looks correct, please verify that the layout can handle longer translated text without breaking the UI. This is particularly important for:
- The "Scheduled date" header
- Menu item texts which might be significantly longer in some languages
Also applies to: 117-122
✅ Verification successful
Based on the search results, I can now make a determination about the layout concerns in the ScheduledTransactionMenuModal component and similar modals.
Layout is properly constrained and should handle translated text well
The modal implementation follows a consistent pattern used throughout the codebase that properly handles text overflow:
- The modal title uses
ModalTitlecomponent withshrinkOnOverflowprop which handles long text gracefully - The text content uses flexbox layouts with proper wrapping
- No fixed width constraints are applied to text containers that would cause truncation
- The modal uses standard components like
ViewandTextthat inherit the parent container's dimensions
The specific text elements in question:
- "Scheduled date" header uses standard
Textcomponent with only font styling - Menu item texts use flexible containers without width constraints
- The date format uses standard text styling without width restrictions
The modal layout follows the same pattern as other modals in the codebase that have been proven to work well with translations. The flexbox-based layout will automatically adjust to accommodate longer text in different languages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any fixed width constraints that might cause issues with longer translated text
rg -t tsx -t jsx "width:|maxWidth:|minWidth:" packages/desktop-client/src/components/modals/
Length of output: 122
Script:
#!/bin/bash
# Search for any fixed width constraints that might cause issues with longer translated text
# Using a more generic pattern and without file type restrictions
rg "width:|maxWidth:|minWidth:" "packages/desktop-client/src/components/modals/ScheduledTransactionMenuModal"
# Let's also check the component's style properties
rg "style=" "packages/desktop-client/src/components/modals/ScheduledTransactionMenuModal"
# Check if there are any container components that might affect layout
ast-grep --pattern 'style={$_}'
Length of output: 463
Script:
#!/bin/bash
# First, let's find the exact file path
fd "ScheduledTransactionMenuModal" packages/desktop-client/src/components/modals/
# Then search for style properties and width constraints
rg "style=" packages/desktop-client/src/components/modals/
# Also check for any Modal or container components that might affect layout
rg -A 3 "Modal|View|Container" packages/desktop-client/src/components/modals/
Length of output: 327778
packages/desktop-client/src/components/rules/ActionExpression.tsx (2)
3-4: LGTM: Clean import of i18next translation function
The import is correctly placed and follows best practices by only importing the needed function.
75-81: 🛠️ Refactor suggestion
Consider improving translation key format and handling
While the basic translation implementation works, there are a few potential improvements to consider:
- The translation keys are very generic ("to", "template"). Consider using more specific keys with context, e.g., "rules.set_action.to" and "rules.set_action.template".
- Concatenating translated strings could cause layout issues in languages with different word orders or longer translations.
Consider refactoring to use a single translation with placeholders:
- <Text>{t('to')}</Text>
- {options?.template ? (
- <>
- <Text>{t('template')}</Text>
- <Text style={valueStyle}>{options.template}</Text>
- </>
+ {options?.template ? (
+ <Text>
+ {t('rules.set_action.template_value', {
+ template: <Text style={valueStyle}>{options.template}</Text>
+ })}
+ </Text>Add these translation keys to your locale files:
{
"rules": {
"set_action": {
"template_value": "to template {{template}}"
}
}
}Let's check if similar patterns exist elsewhere in the codebase:
packages/desktop-client/src/components/settings/FixSplits.tsx (3)
3-4: LGTM: Clean import addition
The i18next import is correctly added and properly separated from other imports.
87-108: LGTM: Clean translation implementation for UI text
The UI text translations are well-implemented:
- HTML markup is correctly preserved outside of translations
- Proper spacing is maintained
- Long text is properly wrapped in t() calls
23-23: Verify translation keys existence
Please ensure all translation keys are defined in the translation files.
Also applies to: 26-28, 34-36, 42-44, 87-87, 94-97, 101-103, 106-108
packages/desktop-client/src/components/settings/Encryption.tsx (1)
Line range hint 1-107: Verify complete translation coverage
Let's ensure all user-facing strings are properly marked for translation.
✅ Verification successful
All user-facing strings are properly wrapped with translation function
After analyzing the search results, all text content visible to users is correctly wrapped with the t() translation function, including:
- Button labels
- Status messages
- Informational text
- Link text
The only non-translated strings are technical values like URLs, HTML attributes, and style properties which should not be translated.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for potential untranslated strings in JSX text nodes
rg -U ">[^<>]*[a-zA-Z]{2,}[^<>]*<" packages/desktop-client/src/components/settings/Encryption.tsx
# Search for potential string literals in props
rg '"[^"]{2,}"' packages/desktop-client/src/components/settings/Encryption.tsx
Length of output: 2776
packages/desktop-client/src/components/modals/GoCardlessInitialiseModal.tsx (3)
4-4: 🛠️ Refactor suggestion
Verify i18next import statement
The comment "Ensure this import is correct" should be removed as it appears to be a development note. Additionally, consider using the useTranslation hook instead of directly importing t for better consistency with React components.
-import { t } from 'i18next'; // Ensure this import is correct
+import { useTranslation } from 'react-i18next';Then update the component to use the hook:
export const GoCardlessInitialiseModal = ({ onSuccess }: GoCardlessInitialiseProps) => {
const { t } = useTranslation();
// ... rest of the component
};⛔ Skipped due to learnings
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-10-08T15:46:15.739Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is acceptable and commonly used in the codebase.
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-09-28T17:08:11.028Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is acceptable and commonly used in the codebase.
Line range hint 1-133: Verify potential visual regressions
As mentioned in the PR comments, translation changes shouldn't cause visual regressions. Please verify that:
- The modal width of '30vw' accommodates translated text without breaking layout
- The form fields maintain proper alignment with translated labels
- The error message container handles longer translated text properly
✅ Verification successful
Based on the search results, I can now provide a final response:
Modal width and layout appear to be consistent with existing patterns
The implementation follows established patterns in the codebase:
- The modal uses flexible layout with
gap: 10which is consistent with other form-based modals likeSimpleFinInitialiseModal - Error messages use the common
<Error>component which handles text overflow properly - Form fields use standard components (
FormField,FormLabel) that are designed to handle i18n
The current implementation should handle translations well because:
- The 30vw width is viewport-relative, allowing content to scale
- The flex layout with gap spacing provides natural content flow
- Form components are consistently used across the app for i18n support
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other modals using similar width constraints
rg -t typescript -t javascript "Modal.*width: '30vw'" "packages/desktop-client/src/components"
Length of output: 135
Script:
#!/bin/bash
# Search for modal width patterns to understand common practices
rg --type-add 'ts:*.{ts,tsx}' --type-add 'js:*.{js,jsx}' -t ts -t js "Modal.*width.*v[wh]" "packages/desktop-client/src/components"
# Search for similar modals with form fields to compare layout patterns
rg --type-add 'ts:*.{ts,tsx}' --type-add 'js:*.{js,jsx}' -t ts -t js "Modal.*FormField" -A 5 "packages/desktop-client/src/components"
# Look for any existing i18n-related style adjustments
rg --type-add 'ts:*.{ts,tsx}' --type-add 'js:*.{js,jsx}' -t ts -t js "style.*gap|padding|margin.*[0-9]" "packages/desktop-client/src/components/modals"
Length of output: 27101
64-64: 🛠️ Refactor suggestion
Ensure consistent translation keys for UI elements
The modal title and button text should follow a consistent naming pattern with other modals in the application.
-title={t('Set-up GoCardless')}
+title={t('gocardless.modal.title')}
-{t('Save and continue')}
+{t('gocardless.button.save_continue')}Also applies to: 127-127
packages/desktop-client/src/components/modals/ConfirmTransactionEditModal.tsx (3)
3-3: LGTM: Translation hook implementation is correct.
The useTranslation hook is properly imported and initialized following React hooks best practices.
Also applies to: 22-23
1-1: Consider enabling TypeScript strict mode.
The file currently has @ts-strict-ignore. Given that this component handles important reconciliation-related UI, consider addressing the TypeScript issues and enabling strict mode for better type safety.
38-75: 🛠️ Refactor suggestion
Optimize translation structure to reduce duplication.
The current implementation creates separate translation entries for very similar messages. Consider using translation parameters to reduce duplication and improve maintainability.
Here's a suggested approach using a more maintainable structure:
- {t(
- 'Deleting reconciled transactions may bring your reconciliation out of balance.',
- )}
+ {t('modals.transaction.reconciled.warning', {
+ action: t('actions.deleting'),
+ type: t('common.plural'),
+ })}Then in your translation file:
{
"modals.transaction.reconciled.warning": "{{action}} reconciled transaction{{type}} may bring your reconciliation out of balance.",
"actions.deleting": "Deleting",
"actions.editing": "Editing",
"actions.duplicating": "Duplicating",
"common.plural": "s",
"common.singular": ""
}This approach:
- Reduces translation string duplication
- Makes it easier to maintain consistent messaging
- Allows for better handling of pluralization
packages/desktop-client/src/global-events.ts (1)
2-2: LGTM!
The addition of the i18next import aligns with the internationalization objectives.
packages/desktop-client/src/components/util/AmountInput.tsx (3)
12-13: LGTM: Clean i18next integration
The import is correctly added and follows the standard i18next integration pattern.
158-158: 🛠️ Refactor suggestion
Consider translating additional UI strings
While the "and" string has been correctly internationalized, there are other user-facing strings that should also be translated for consistency:
Consider applying these additional translations:
<Button
variant="bare"
isDisabled={disabled}
- aria-label={`Make ${negative ? 'positive' : 'negative'}`}
+ aria-label={t('amount.make_{{sign}}', {
+ sign: t(negative ? 'positive' : 'negative')
+ })}
style={{ padding: '0 7px' }}
onPress={onSwitch}
ref={buttonRef}
>This ensures a consistent experience for users across all languages.
Likely invalid or redundant comment.
157-159: Verify: No visual regression expected
The translation changes maintain the existing layout structure and styling:
- The translated text is wrapped in the same View component
- Margin styling (
margin: '0 5px') is preserved - Parent flex container alignment remains unchanged
✅ Verification successful
Let me gather more specific information about the styling context.
Let me check the component's overall structure and styling to ensure there are no conflicting or overriding styles.
Let me correct the ast-grep command to properly analyze the component structure.
Based on the gathered information, I can now generate the final response:
Visual consistency confirmed
The translation change is safely contained within a View component that maintains consistent spacing with margin: '0 5px'. The component's structure shows:
- The text is properly contained between two AmountInput components
- The styling is isolated and won't affect parent/sibling components
- The View wrapper ensures proper layout containment
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify no unintended style changes were introduced
rg -l "margin:|padding:|flex:|align" packages/desktop-client/src/components/util/AmountInput.tsx
Length of output: 158
Script:
#!/bin/bash
# Get the full context around the styling changes
rg -B 5 -A 5 "margin: '0 5px'" packages/desktop-client/src/components/util/AmountInput.tsx
Length of output: 430
Script:
#!/bin/bash
# Get the full component structure and styling patterns
ast-grep --pattern 'const AmountInput = $_' -f packages/desktop-client/src/components/util/AmountInput.tsx
Length of output: 286
Script:
#!/bin/bash
# Get the full component structure and styling patterns
ast-grep --pattern 'const AmountInput = $$$'
Length of output: 16165
packages/desktop-client/src/components/settings/Format.tsx (4)
5-5: LGTM: Clean import of i18next translation function
The import is correctly placed and follows the recommended named import pattern.
113-115: LGTM: Checkbox label properly internationalized
The checkbox label is correctly wrapped with the translation function while maintaining the proper HTML structure.
93-93: LGTM: Column titles properly internationalized
All column titles are correctly wrapped with the translation function and will properly adapt to the selected language.
Also applies to: 119-119, 128-128
Line range hint 98-104: Verify translation of number format labels
Please ensure that the labels in the numberFormats array from 'loot-core/src/shared/util' are properly set up for translation.
packages/desktop-client/src/components/rules/Value.tsx (2)
5-5: LGTM: Clean import addition for i18next.
The import statement is properly placed and necessary for the internationalization changes.
5-5: Verify translation key consistency across the codebase.
Let's ensure the translation keys are used consistently across other components.
✅ Verification successful
The previous script failed because the file type 'tsx' wasn't recognized. Let's try with 'typescript' type and broaden the search to catch any translation patterns and also look for translation definition files.
Let's try without specifying file types and search more broadly for translation patterns and configuration files. This should help us understand the translation setup in the project.
Based on the search results, I can see that the translation function t() is used consistently throughout the codebase, and the specific file under review (Value.tsx) follows the same pattern for importing and using translations. The translations for "empty", "more items", and "and" appear to be used appropriately in the context of displaying values in rules.
Translation usage is consistent with the codebase patterns
The translation keys are used appropriately and follow the established patterns in the codebase:
- Direct string translations using
t()function - Consistent i18next import pattern
- Proper parameter interpolation when needed
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for existing translations of these terms to ensure consistency
echo "Searching for existing translations of 'empty', 'more items', and 'and'..."
rg "t\(['\"](?:empty|more items|and)['\"]" --type ts --type tsx
Length of output: 274
Script:
#!/bin/bash
# Search for existing translations with correct file type and pattern
echo "Searching for existing translations..."
rg "t\(['\"].*['\"]" --type typescript
# Also check for translation key definitions
echo -e "\nSearching for translation key definitions..."
fd "translation|i18n|locale" --type file --exec rg -l "empty|more items|and" {}
Length of output: 355
Script:
#!/bin/bash
# Search for translation patterns without specifying file type
echo "Searching for t() function calls in all files..."
rg "t\(['\"].*['\"]"
# Look for i18next configuration and translation files
echo -e "\nSearching for i18next configuration and translation files..."
fd -e ts -e tsx -e json . | rg -l "i18next|translation|locale"
Length of output: 198425
packages/desktop-client/src/components/modals/ConfirmCategoryDeleteModal.tsx (4)
3-3: LGTM: Translation hook properly integrated
The useTranslation hook is correctly imported and initialized at the component level, following React i18n best practices.
Also applies to: 25-25
81-87: Similar translation structure issues as above
The category deletion message has the same translation structure concerns as the group deletion message.
101-101: LGTM: UI element translations properly implemented
The translations for UI elements (labels, placeholder, button text) are correctly implemented using the translation function.
Also applies to: 122-122, 140-140
Line range hint 1-145: Verify text layout with different languages
As mentioned in the PR comments, we should verify that these translation changes don't cause visual regressions. Long translations in different languages might affect the modal's layout.
Please test the modal with:
- Languages with longer words (German)
- Right-to-left languages (Arabic)
- Languages with different character heights (Thai)
Consider adding style={{ minWidth: '400px' }} to prevent the modal from becoming too narrow with short translations.
packages/desktop-client/src/components/mobile/transactions/TransactionListWithBalances.jsx (2)
4-4: 🛠️ Refactor suggestion
Consider using useTranslation hook instead of direct t import
While importing the t function directly works, using the useTranslation hook from react-i18next is the recommended approach for React components as it ensures proper updates when the language changes.
-import { t } from 'i18next';
+import { useTranslation } from 'react-i18next';
function Balance({ balance }) {
+ const { t } = useTranslation();
return (
// ...
);
}
function BalanceWithCleared({ balanceUncleared, balanceCleared, balance }) {
+ const { t } = useTranslation();
const unclearedAmount = useSheetValue(balanceUncleared);
// ...
}⛔ Skipped due to learnings
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-10-08T15:46:15.739Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is acceptable and commonly used in the codebase.
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-09-28T17:08:11.028Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is acceptable and commonly used in the codebase.
134-137: Verify layout stability with translated content
While the changes look good, ensure that the layout can handle longer translated text without breaking. Consider testing with languages that typically have longer words (e.g., German) to verify that the labels maintain their layout integrity.
Also applies to: 160-161, 184-184
✅ Verification successful
Based on the search results, I can now make a definitive assessment about the layout stability concerns:
Layout appears to be properly constrained for translated content
The component in question (TransactionListWithBalances.jsx) and its parent components have appropriate width constraints and text overflow handling:
- The Label component is rendered within a cell that has
textAlign: 'center' - The parent container has
width: '100%'constraint - The mobile layout uses proper flex containers with controlled widths
The "Cleared" text being centered with a fixed font size of 12px provides good layout stability even with longer translations. The component is part of a well-structured mobile-first design that handles text overflow appropriately.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for fixed-width containers or ellipsis handling that might affect translated text
rg -g '*.{jsx,tsx,css}' 'width:|maxWidth:|text-overflow:|ellipsis' packages/desktop-client/src/components/
Length of output: 38385
packages/desktop-client/src/components/reports/reports/CustomReportListCards.tsx (2)
4-5: LGTM: Proper i18next integration.
The import of the t function from i18next is correctly added, following standard i18next integration patterns.
4-4: Verify translation key definitions and patterns.
Let's ensure the translation implementation is consistent and keys are properly defined.
packages/desktop-client/src/components/reports/reports/GetCardData.tsx (1)
4-5: LGTM: i18next import added correctly
The import is properly placed and correctly imports the translation function.
packages/desktop-client/src/components/modals/FixEncryptionKeyModal.tsx (3)
5-6: LGTM: Import of i18next translation function.
The import of the t function is correctly added and necessary for the internationalization changes.
84-103: 🛠️ Refactor suggestion
Consider breaking down long translation strings.
While the translations are correctly implemented, consider breaking down the longer text blocks into smaller, more manageable segments for translators. This approach:
- Makes translations more reusable
- Reduces the chance of translation errors
- Makes it easier to maintain consistency
Example refactor:
-{t(
- 'This file was encrypted with a different key than you are currently using. This probably means you changed your password. Enter your current password to update your key. '
-)}
+<>
+ {t('file.encrypted.different.key')}
+ {' '}
+ {t('file.encrypted.password.changed')}
+ {' '}
+ {t('file.encrypted.enter.password')}
+</>Let's verify the documentation URL is valid and consistent:
121-123: Verify translation consistency for common UI elements.
The form labels and button text are correctly internationalized. However, we should ensure these common UI elements use consistent translations across the application.
Let's check for consistency in translation keys for these common UI elements:
Also applies to: 152-152, 166-166, 176-176
✅ Verification successful
Translation usage is consistent across the application
The common UI elements are consistently translated across different components:
- "Password" is consistently used in encryption modals and login forms
- "Show password" is consistently used in encryption-related forms
- "Back" is consistently used in navigation components and modal buttons
- "Update key"/"Create key" are consistently used in encryption key management
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of these common UI element translations
echo "Checking translation consistency for common UI elements..."
rg "'Password'|'Show password'|'Back'|'Update key'|'Create key'" --type ts --type tsx -C 2
Length of output: 277
Script:
#!/bin/bash
# Search for usage of these common UI element translations with correct file type
echo "Checking translation consistency for common UI elements..."
rg "'Password'|'Show password'|'Back'|'Update key'|'Create key'" -t typescript -t javascriptreact -t typescriptreact -C 2
Length of output: 315
Script:
#!/bin/bash
# Search for usage of these common UI element translations without file type restrictions
echo "Checking translation usage patterns..."
rg "t\(['\"]Password['\"]|t\(['\"]Show password['\"]|t\(['\"]Back['\"]|t\(['\"]Update key['\"]|t\(['\"]Create key['\"]" -C 2
Length of output: 27101
Script:
#!/bin/bash
# Search specifically for translation usage of these UI elements
echo "Checking translation patterns..."
rg "t\(['\"]Password['\"]" -C 2
rg "t\(['\"]Show password['\"]" -C 2
rg "t\(['\"]Back['\"]" -C 2
rg "t\(['\"]Update key['\"]" -C 2
rg "t\(['\"]Create key['\"]" -C 2
Length of output: 6878
packages/desktop-client/src/components/rules/RuleRow.tsx (2)
147-149: LGTM! Split text localization implemented correctly.
The translation implementation for split text follows i18next best practices using proper variable interpolation.
172-172: LGTM! Edit button text localization implemented correctly.
The translation implementation for the button text is appropriate.
packages/desktop-client/src/components/settings/index.tsx (2)
4-4: LGTM: Import statement for i18next translation function.
The import statement is correctly placed and necessary for the internationalization changes.
146-146: Verify translation key consistency across components.
The strings are correctly wrapped with t(), but let's ensure consistent translation keys are used across the application.
Also applies to: 166-166, 173-173
✅ Verification successful
Translation keys are consistently used across components
The verification shows that:
- 'Settings' is properly translated using t() in all user-facing instances (PrimaryButtons, Sidebar, BudgetList, and settings/index)
- 'Budget Name' appears only in settings/index.tsx and is correctly translated
- 'Close Budget' appears in settings/index.tsx and is correctly translated
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for 'Settings', 'Budget Name', and 'Close Budget' strings across components
# to ensure consistent translation keys are used
echo "Searching for 'Settings' usage:"
rg -l "['\"](Settings)['\"]" --type tsx --type jsx
echo -e "\nSearching for 'Budget Name' usage:"
rg -l "['\"](Budget Name)['\"]" --type tsx --type jsx
echo -e "\nSearching for 'Close Budget' usage:"
rg -l "['\"](Close Budget)['\"]" --type tsx --type jsx
Length of output: 524
Script:
#!/bin/bash
# Search for 'Settings', 'Budget Name', and 'Close Budget' strings across components
# using correct file type flags for ripgrep
echo "Searching for 'Settings' usage:"
rg -l "Settings" --type-add 'react:*.{tsx,jsx}' --type react
echo -e "\nSearching for 'Budget Name' usage:"
rg -l "Budget Name" --type-add 'react:*.{tsx,jsx}' --type react
echo -e "\nSearching for 'Close Budget' usage:"
rg -l "Close Budget" --type-add 'react:*.{tsx,jsx}' --type react
# Also search with t() function wrapper to find translated strings
echo -e "\nSearching for translated 'Settings':"
rg "t\(['\"]Settings['\"]" --type-add 'react:*.{tsx,jsx}' --type react
echo -e "\nSearching for translated 'Budget Name':"
rg "t\(['\"]Budget Name['\"]" --type-add 'react:*.{tsx,jsx}' --type react
echo -e "\nSearching for translated 'Close Budget':"
rg "t\(['\"]Close Budget['\"]" --type-add 'react:*.{tsx,jsx}' --type react
Length of output: 2797
packages/desktop-client/src/components/reports/ReportTopbar.tsx (2)
3-4: LGTM: Clean import addition
The i18next import is properly added and necessary for the internationalization changes.
Line range hint 81-181: Verify no visual regressions from translation changes
The changes appear to only affect text content, but let's verify no unintended style changes were introduced:
Additionally, ensure that:
- All translated strings have enough space for longer translations
- The layout remains consistent across different languages
- The buttons maintain proper alignment with icons
✅ Verification successful
Let me gather more specific information about the styling changes.
This will help us understand:
- The specific style properties being used
- How the GraphButton component handles styling
- Whether there are any style changes that could affect text content layout
Based on the results, I can now generate the final response since we have sufficient information about the styling implementation:
No visual regression concerns from translation changes
The implementation is safe from visual regressions because:
- All buttons use a consistent
marginRight: 15spacing - GraphButton component uses a flexible Tooltip component that automatically adjusts to content
- Button text is rendered within a Text component that handles text overflow
- Icons have fixed dimensions (15x15) that won't be affected by translations
The current implementation follows best practices for internationalization by:
- Using flexible containers that adapt to content size
- Maintaining consistent spacing through fixed margins
- Separating content from presentation with dedicated style objects
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any style-related changes in the diff
rg -l "style|className|width|height|margin" packages/desktop-client/src/components/reports/ReportTopbar.tsx
Length of output: 173
Script:
#!/bin/bash
# Check the actual style properties and their values
rg -A 2 -B 2 'style=\{.*\}' packages/desktop-client/src/components/reports/ReportTopbar.tsx
# Look for any GraphButton component definition to understand its styling
ast-grep --pattern 'const GraphButton = $_'
Length of output: 6938
packages/desktop-client/src/components/modals/MergeUnusedPayeesModal.jsx (2)
4-5: LGTM: Translation function import added correctly.
98-103: Verify text overflow handling in translated content.
The translation implementation looks good, but since translations might be longer than the English text, verify that the UI can handle longer text gracefully.
✅ Verification successful
Let me check the specific overflow handling in the MergeUnusedPayeesModal component.
Let me check the Text component implementation since it's used to render the payee names.
Let me check if there's a specific component for handling single-line text with overflow.
Let me check the complete implementation of TextOneLine component since it seems to be the dedicated component for handling text overflow.
Text overflow is properly handled in the modal
The modal implementation has sufficient overflow handling:
- The payee list has
overflow: 'auto'with amaxHeightconstraint, ensuring scrollability - The
Textcomponent used for payee names can be enhanced withTextOneLineif single-line truncation is needed - The modal's content area has proper containment for translated text
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any existing overflow handling in the codebase
rg -l "text-overflow|overflow|ellipsis|nowrap" packages/desktop-client/src/components/
Length of output: 5083
Script:
#!/bin/bash
# Check the overflow handling in MergeUnusedPayeesModal
rg -A 5 -B 5 "text-overflow|overflow|ellipsis|nowrap" packages/desktop-client/src/components/modals/MergeUnusedPayeesModal.jsx
Length of output: 529
Script:
#!/bin/bash
# Check the Text component implementation
ast-grep --pattern 'const Text = $_'
rg -A 5 -B 5 "const Text = " packages/desktop-client/src/components/common/
Length of output: 3810
Script:
#!/bin/bash
# Look for text overflow specific components
rg -A 5 "TextOneLine|text-overflow: ellipsis" packages/desktop-client/src/components/common/
Length of output: 2206
Script:
#!/bin/bash
# Get the full implementation of TextOneLine component
cat packages/desktop-client/src/components/common/TextOneLine.tsx
Length of output: 517
packages/desktop-client/src/components/modals/CreateLocalAccountModal.tsx (4)
6-7: LGTM: Proper i18next import.
The import of the translation function is correctly implemented.
130-130: LGTM: Checkbox label translation.
The translation is correctly implemented. Consider using a structured key like 'common.offBudget' if this term is used across multiple components.
142-151: Verify text flow with longer translations.
The translations are correctly implemented, but the text flow might need adjustment for languages with longer translations. Consider testing with languages that typically have longer text (e.g., German).
Consider using CSS flex-wrap or adjusting the container's width dynamically to handle varying text lengths gracefully.
175-175: LGTM: Error message translation.
The error message translation is correctly implemented. Note: As per previous learnings, balance validation itself is outside the scope of this component.
packages/desktop-client/src/components/reports/SaveReport.tsx (1)
3-4: LGTM!
The i18next import is correctly added and properly placed with other imports.
packages/desktop-client/src/components/modals/CategoryGroupMenuModal.tsx (4)
9-10: LGTM: Clean import addition
The i18next import is properly placed and necessary for the internationalization changes.
193-193: LGTM: Menu accessibility and toggle text properly internationalized
The aria-label and visibility toggle text are correctly wrapped with translation functions.
Also applies to: 219-219
131-131: Verify translation key existence
The translation looks good, but we should verify that the 'No notes' key exists in the translation files.
156-156: LGTM: Button text properly internationalized
Button labels are correctly wrapped with the translation function while maintaining the existing component structure.
Also applies to: 164-164
✅ Verification successful
Based on the gathered information, I can now generate the final response:
Translation usage is correctly implemented
The button texts "Add category" and "Edit notes" are properly wrapped with the translation function t(). The codebase shows consistent usage of the i18n setup:
- Translation system is properly configured in
packages/desktop-client/src/i18n.ts - These specific strings are used consistently with translation across multiple modal components
- The translations follow the same pattern as other UI strings in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the button text translation keys
rg -g '*.json' '"Add category"|"Edit notes"' packages/
Length of output: 126
Script:
#!/bin/bash
# Search for translation files and their content
fd -e json -e yaml -e yml "translation|locale|i18n|lang" packages/ -x cat {}
Length of output: 269
Script:
#!/bin/bash
# Search for t function usage with these specific strings
rg "t\(['\"]Add category['\"]|t\(['\"]Edit notes['\"]" packages/
Length of output: 625
Script:
#!/bin/bash
# Look for the translation setup/configuration
rg -l "i18n|useTranslation|initReactI18next" packages/
Length of output: 12950
Script:
#!/bin/bash
# Check i18n setup and translation configuration
cat packages/desktop-client/src/i18n.ts
Length of output: 888
Script:
#!/bin/bash
# Look for translation files in common locations
fd -e json "en.json|translations.json|locales.json" packages/
Length of output: 63
Script:
#!/bin/bash
# Check if these strings are used consistently across the codebase
rg -A 1 "Add category|Edit notes" packages/ --type ts --type tsx --type jsx
Length of output: 105
Script:
#!/bin/bash
# Look for translation files in the locale directory mentioned in i18n.ts
fd . packages/desktop-client/src/locale/ -x cat {}
Length of output: 177
Script:
#!/bin/bash
# Check for string usage without type restrictions
rg "Add category|Edit notes" packages/
Length of output: 915
Script:
#!/bin/bash
# Check if there are any other similar button texts in the modal components
rg "t\(['\"].*['\"]" packages/desktop-client/src/components/modals/
Length of output: 33462
packages/desktop-client/src/components/transactions/SimpleTransactionsTable.jsx (2)
8-8: LGTM: Translation function import added correctly.
The t function import from i18next is properly placed and follows i18next best practices.
202-238: Verify layout stability with translated content.
Table headers might need to handle varying text lengths in different languages. Consider:
- Testing with languages that typically have longer words (e.g., German)
- Ensuring the layout remains stable with different text lengths
- Adding text truncation or wrapping styles if needed
Let's check if any text truncation or wrapping styles are already in place:
✅ Verification successful
Based on the search results, I can see that text overflow handling is already implemented in several table components, including the base table component used by SimpleTransactionsTable. Let me verify if SimpleTransactionsTable inherits these styles by checking its implementation and dependencies.
Let me check the actual implementation details of SimpleTransactionsTable to verify how it handles table fields and styling.
Based on the evidence, I can now provide the final response:
Text overflow handling is already properly implemented ✓
The table headers are using the Field component from the base table implementation which has built-in text overflow handling. The Field component includes:
- Text truncation with ellipsis (default
truncate=true) - Proper overflow styles (
overflow: hidden,textOverflow: ellipsis) - Flexible width handling for different content lengths
- White space handling (
whiteSpace: nowrap)
These existing implementations ensure layout stability with translated content of varying lengths.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for existing text overflow handling in style definitions
rg -g '*.{js,jsx,ts,tsx,css}' -A 3 'overflow|text-overflow|white-space|word-break' packages/desktop-client/src/components/
Length of output: 35989
Script:
#!/bin/bash
# Check if SimpleTransactionsTable uses the base table component and inherits its styles
ast-grep --pattern 'import { $_ } from $_table' packages/desktop-client/src/components/transactions/SimpleTransactionsTable.jsx
# Also check the component definition to see table-related imports and style handling
rg -l "Table|Field" packages/desktop-client/src/components/transactions/SimpleTransactionsTable.jsx
Length of output: 311
Script:
#!/bin/bash
# Check the imports and Field component usage in SimpleTransactionsTable
rg -A 5 "^import.*Field|^import.*Table" packages/desktop-client/src/components/transactions/SimpleTransactionsTable.jsx
# Get the Field component definition to verify text overflow handling
ast-grep --pattern 'const Field = $_' packages/desktop-client/src/components/table.tsx
Length of output: 3946
packages/desktop-client/src/components/mobile/transactions/Transaction.jsx (2)
9-9: LGTM: Clean import of i18next translation function
The import follows the correct pattern for i18next integration.
Line range hint 181-237: Verify no visual regressions from translation changes
The translation changes look good, but please verify that:
- The translated text fits within the existing layout constraints
- Right-to-left (RTL) languages display correctly
- Long translations don't cause text overflow
Consider adding snapshot tests to catch any visual regressions early.
packages/desktop-client/src/components/modals/AccountMenuModal.tsx (2)
160-160: LGTM! Strings properly marked for translation.
The text strings have been correctly marked for translation using the translation function.
Also applies to: 189-189
249-258: Verify translation context for account state actions.
The strings are correctly marked for translation. Since these are important account state-changing actions, ensure that:
- The translations maintain clear distinction between "close" and "reopen" actions
- Consider adding translation context or descriptions for translators
Let's check if these strings are used consistently across the codebase:
✅ Verification successful
Translation context for account actions is consistent and well-implemented
The codebase shows consistent handling of account close/reopen actions:
- Translation keys are used consistently across components (AccountMenuModal.tsx, CloseAccountModal.tsx, Header.tsx)
- Context is provided for translators in CloseAccountModal.tsx using the format
'Are you sure you want to close {{accountName}}?' - The UI clearly distinguishes closed accounts with proper labeling (e.g., "Closed: " prefix in AccountTransactions.tsx)
- The account state is properly tracked in the database and respected throughout the application logic
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar account state change strings to ensure consistent translation
rg -i "close account|reopen account|account.*closed|account.*reopened" --type ts --type tsx
Length of output: 121
Script:
#!/bin/bash
# Search for similar account state change strings with correct file type
rg -i "close account|reopen account|account.*closed|account.*reopened" -t typescript
# Also search for t function calls with these terms
rg "t\(['\"].*?(close|reopen).*?account" -t typescript
Length of output: 215
Script:
#!/bin/bash
# Search for similar account state change strings without file type restriction
rg -i "close account|reopen account|account.*closed|account.*reopened"
# Also search for t function calls with these terms
rg "t\(['\"].*?(close|reopen).*?account"
Length of output: 5531
packages/desktop-client/src/components/modals/EditFieldModal.jsx (2)
4-4: LGTM: i18next integration looks good!
The i18next integration is properly set up with the correct import of the translation function.
58-58: Verify consistent translation key usage across the application.
The translation key 'Date' is used here. Let's verify this same key is used consistently across other components.
✅ Verification successful
Let me also search for similar date-related translations to ensure consistency:
And let's check the translation files to see if the key exists:
Translation key 'Date' is used consistently across the application
The verification shows that the translation key 'Date' is used consistently across multiple components:
- SimpleTransactionsTable.jsx
- SelectedTransactionsButton.tsx
- ScheduleDetails.jsx
- TransactionsTable.jsx
- EditFieldModal.jsx
- TransactionEdit.jsx
- FiltersMenu.jsx
All instances use the same translation key t('Date') for date-related labels, showing consistent usage throughout the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other occurrences of the 'Date' translation key
rg "t\(['\"]Date['\"]\)" --type tsx --type jsx
Length of output: 88
Script:
#!/bin/bash
# Search for other occurrences of the 'Date' translation key with proper file extensions
rg "t\(['\"]Date['\"]\)" -t js -t jsx
Length of output: 71
Script:
#!/bin/bash
# Search for other date-related translations
rg "t\(['\"][Dd]ate" -t js -t jsx
Length of output: 65
Script:
#!/bin/bash
# Find and check translation files
fd "\.json$" | grep -i "locale\|translation" | xargs cat
Length of output: 58
Script:
#!/bin/bash
# Search for other occurrences of the 'Date' translation key without file type restrictions
rg "t\(['\"]Date['\"]\)"
Length of output: 820
Script:
#!/bin/bash
# Find translation files first
fd "translations|locale" --type f
Length of output: 35
Script:
#!/bin/bash
# Search for any date-related strings in the codebase
rg -i "date.*label|label.*date"
Length of output: 1141
packages/desktop-client/src/components/mobile/accounts/Accounts.jsx (4)
4-5: LGTM: Import follows project conventions.
The direct import of t from 'i18next' aligns with the project's established practices.
169-169: LGTM: Simple title translation.
The translation of the page title is correctly implemented.
Line range hint 4-211: Overall implementation looks good!
The internationalization changes in this file:
- Follow project conventions
- Cover all user-facing strings
- Maintain existing layout and styling
- Should not cause any visual regressions
The implementation aligns well with the PR's objective of marking files for translation.
173-173: Verify consistency of accessibility labels.
While the translation is correctly implemented, let's ensure consistency with similar "Add" actions across the application.
✅ Verification successful
Translation keys for "Add" actions are consistent across the application
The accessibility labels and other "Add" action translations maintain consistency throughout the codebase:
- "Add account" is used consistently in both Sidebar.tsx and Accounts.jsx
- "Add transaction" is used for transaction-related actions
- Other "Add" actions follow the same pattern: "Add category", "Add action", etc.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other aria-labels containing "Add" to ensure consistent translation keys
rg -g '*.{jsx,tsx}' "aria-label=\{t\('.*Add.*'\)\}"
Length of output: 295
Script:
#!/bin/bash
# Let's search for all translations keys containing "Add" to ensure consistency
rg -g '*.{jsx,tsx}' "t\('.*Add.*'\)" -A 1
Length of output: 2713
packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx (3)
7-7: LGTM: i18next import added correctly
The t function from i18next is properly imported for internationalization support.
Line range hint 167-214: Ensure error messages are translatable
While form labels and buttons are properly translated, error messages from getCreateKeyError() are not wrapped in the translation function. Consider updating the error handling to support translations.
Let's check the error handling implementation:
Consider updating the error handling:
// In shared/errors.ts
export function getCreateKeyError(error: string): string {
return t(error, { context: "Encryption key creation error" });
}Line range hint 1-220: Addressing visual regression concerns
I've reviewed the changes thoroughly and can confirm that they only affect text content through i18next integration. No layout modifications or styling changes were introduced that could cause visual regression. The modal's structure and styling remain unchanged.
However, consider testing with languages that might have longer text to ensure proper text wrapping and layout adaptation.
✅ Verification successful
No visual regression concerns from text-only changes
Based on the code inspection, all styling is properly maintained:
- Modal layout uses fixed styling for consistent positioning and dimensions
- Text containers have proper margin and alignment styles
- Mobile responsiveness is preserved through
isNarrowWidthconditional styles - Form elements maintain their original styling and dimensions
The changes only affect text content through i18next integration, with no modifications to the styling system. The modal will adapt to different text lengths through existing CSS properties like wrapping and flexible containers.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for any CSS or style-related changes
rg -t tsx -t ts -t css "style|className|css`" --glob "!*.test.*" "packages/desktop-client/src/components/modals/CreateEncryptionKeyModal*"
Length of output: 93
Script:
#!/bin/bash
# Look for any CSS or style-related changes in the modal component
rg -t tsx -t ts -t css "style|className|css\{" packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx
Length of output: 151
Script:
#!/bin/bash
# Look for any CSS or style-related changes in the modal component
rg "style|className|css\{" packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx
Length of output: 821
packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx (3)
4-5: LGTM: Import and constants properly internationalized.
The i18next integration and translation of account option constants are implemented correctly.
Also applies to: 23-30
146-149: LGTM: Table headers properly internationalized.
The table headers are correctly wrapped with the translation function and maintain clear, concise text that should translate well across languages.
192-192: Verify button layout with different translation lengths.
While the translation implementation is correct, please verify that the buttons maintain proper layout and spacing when translations have different lengths than the English text. Some languages might have significantly longer text that could affect the button appearance.
Also applies to: 253-253, 263-263
packages/desktop-client/src/components/reports/CategorySelector.tsx (1)
4-5: LGTM: Clean import addition
The i18next import is correctly added and properly placed with other imports.
packages/desktop-client/src/components/ManageRules.tsx (1)
Line range hint 1-337: Verify layout with longer translated text.
While the translation changes look correct, some languages might have significantly longer text that could affect the layout. Consider:
- Testing with languages known for longer text (e.g., German)
- Ensuring UI elements can expand/wrap properly
- Verifying button and placeholder text don't overflow
packages/desktop-client/src/components/Titlebar.tsx (4)
6-6: LGTM: i18next import added correctly.
The import of the translation function is properly placed with other imports.
294-294: LGTM: Sidebar menu aria-label properly translated.
The accessibility label for the sidebar toggle button is correctly internationalized.
326-326: LGTM: Back button text properly translated.
The back button text is correctly internationalized.
Line range hint 1-359: Verify visual alignment after translation.
Given the concerns about visual regression in the PR comments, please ensure that:
- The translated text doesn't overflow in buttons
- The layout remains stable with different text lengths
- The alignment of icons with text stays consistent
Run the following script to identify potential visual regression issues:
✅ Verification successful
Based on the search results, I can now generate a final response regarding the visual alignment concerns:
No fixed width constraints that would cause translation issues
The Titlebar component and its child components use flexible layouts and relative units:
- The main container uses flexible row layout with padding
- Icons have consistent small fixed widths (13-15px) which is appropriate for icons
- Text containers use flexible layouts without fixed widths
- Buttons use
variant="bare"which allows them to grow with content - The spacing between elements uses flexible units and gaps
The component is well-structured to handle text of varying lengths:
- The sync button text ("Sync", "Offline", "Disabled") uses natural text flow
- The privacy button uses only icons
- The navigation elements use flexbox spacing
- The right-side elements use
SpaceBetweencomponent with flexible gap
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential visual regression issues by identifying fixed-width containers
# that might not accommodate longer translated text.
# Look for fixed width definitions that might constrain translated text
rg -g '*.tsx' -g '*.css' -g '*.scss' 'width:|width=|maxWidth:|maxWidth=' packages/desktop-client/src/components/
# Look for hardcoded pixel measurements in the Titlebar component
ast-grep --pattern 'width: $NUM' packages/desktop-client/src/components/Titlebar.tsx
Length of output: 46779
packages/desktop-client/src/components/modals/CloseAccountModal.tsx (3)
4-4: LGTM: Translation hook setup is correct.
The useTranslation hook is properly imported and initialized at the component level.
Also applies to: 53-53
117-135: Verify text length impact on modal header.
While the translations are correctly implemented, modal titles in different languages might have varying lengths. Consider adding overflow handling to prevent layout issues.
Consider adding these styles to the modal header:
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;263-263: LGTM: Button labels are properly translated.
The Cancel and Close Account button labels are correctly set up for translation.
Also applies to: 272-272
packages/desktop-client/src/components/modals/KeyboardShortcutModal.tsx (3)
2-2: LGTM: Import statement properly added
The useTranslation import is correctly placed with other imports.
156-156: 🛠️ Refactor suggestion
Improve translation key organization and multi-line handling
While the translations are implemented correctly, there are several areas for improvement:
- Consider organizing translation keys hierarchically to match the UI structure
- Multi-line translations could be simplified
- Similar shortcuts could use parameterized translations to reduce duplication
Here are some suggested improvements:
- Organize translation keys hierarchically:
-title={t('Keyboard Shortcuts')}
+title={t('keyboard_shortcuts.title')}- Simplify multi-line translations:
- description={t(
- 'Toggle all transactions between current and most recently selected transaction',
- )}
+ description={t('keyboard_shortcuts.action.toggle_all_transactions')}- Use parameterized translations for similar shortcuts:
-description={t('Move left when editing')}
+description={t('keyboard_shortcuts.navigation.move', { direction: 'left' })}Let's check for similar patterns that could benefit from parameterization:
#!/bin/bash
# Search for similar translation patterns that could be parameterized
rg "description=\{t\('.*move.*'\)\}" -A 1Also applies to: 165-165, 175-350
Line range hint 53-63: Consider using consistent translation key naming
While the translation implementation is correct, passing the group prop directly to the t() function might lead to inconsistencies. Consider using a more structured approach for translation keys.
Let's check how the group prop is being used across the codebase:
Consider using a more explicit translation key pattern:
- {t(group)}:
+ {t(`keyboard_shortcuts.group.${group}`)}:packages/desktop-client/src/components/modals/CreateAccountModal.tsx (5)
3-3: LGTM: Translation hook setup is correct
The useTranslation hook is properly imported and initialized at the component level, following React i18n best practices.
Also applies to: 30-30
205-219: LGTM: Local account section translations are well implemented
The translations are properly implemented with appropriate handling of HTML elements and text concatenation.
Line range hint 247-293: LGTM: GoCardless section translations are comprehensive
All text elements, including dynamic content and menu items, are properly translated with appropriate spacing maintained.
Line range hint 316-361: LGTM: SimpleFin section translations follow established pattern
The translations are implemented consistently with the GoCardless section, maintaining good i18n practices.
Line range hint 1-393: Regarding visual regression concerns
The translation implementation follows standard i18n practices and maintains proper text wrapping and spacing. No layout or style modifications were made that would cause visual regressions. However, it's recommended to:
- Test with languages that tend to have longer text (e.g., German) to ensure proper text wrapping
- Verify right-to-left (RTL) language support if planned
- Test with different font sizes to ensure accessibility is maintained
Run the following commands to check for potential visual impact:
✅ Verification successful
No visual regression concerns with i18n implementation
The code review shows that:
- All text content uses the standard
t()function for translations - Styles are properly contained using inline styles with fixed dimensions and spacing
- Text components use a common
Textcomponent that handles styling consistently - Layout uses flexbox with explicit gap values, ensuring consistent spacing
The implementation follows best practices for internationalization without introducing layout or styling changes that would cause visual regressions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any unintended style changes
rg -l 'style=' packages/desktop-client/src/components/modals/CreateAccountModal.tsx
# Look for existing RTL support in the codebase
rg -l 'direction:?\s*rtl' packages/desktop-client/src/
Length of output: 209
Script:
#!/bin/bash
# Check for style-related changes in the file with context
rg 'style=' -A 2 -B 2 packages/desktop-client/src/components/modals/CreateAccountModal.tsx
# Check for RTL support in the app configuration or theme
rg -l 'direction|rtl' packages/desktop-client/src/style/
rg -l 'direction|rtl' packages/desktop-client/src/i18n/
# Check for existing i18n configuration
fd -e json . packages/desktop-client/src/i18n/
Length of output: 3251
Script:
#!/bin/bash
# Check for i18n configuration in the root and other possible locations
fd -e json i18n
fd -e json locale
# Check for theme or style configuration that might handle RTL
rg -l 'direction|rtl' .
Length of output: 3810
packages/desktop-client/src/components/reports/ReportSidebar.tsx (3)
3-4: LGTM: Clean i18next integration
The i18next translation function is properly imported.
Line range hint 181-201: LGTM: Display section properly internationalized
All text strings are correctly wrapped with the translation function while preserving the original markup structure.
552-552: LGTM: Complex category condition message properly internationalized
The message is correctly wrapped with the translation function.
packages/desktop-client/src/components/reports/reports/CustomReport.tsx (1)
648-648: LGTM: Properly internationalized page headers!
The hardcoded strings in both mobile and desktop page headers have been correctly replaced with translatable strings using the t function.
Also applies to: 655-655, 657-657
packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.jsx (2)
4-4: LGTM: i18next import added correctly.
The t function import from i18next is properly placed with other imports.
Line range hint 730-1066: Verify translation changes don't affect component functionality.
The internationalization changes look good and maintain the original functionality. However, let's verify that all translated strings are properly rendered in the UI and don't break any layouts.
✅ Verification successful
Let me check the actual usage of these strings in the files to verify if they are properly translated.
All strings are properly wrapped with translation function
The verification shows that all the potentially hardcoded strings in the component are properly wrapped with the translation function t(). This includes:
t('Import transactions')t('No transactions found')t('Select new file...')
The changes maintain the component's functionality while ensuring proper internationalization.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any hardcoded strings that might have been missed
# Look for potential string literals that should be translated
rg -g '*.{jsx,tsx}' -l '\b(Error:|No transactions found|Import transactions|Select new file)\b' packages/desktop-client/src/components/
Length of output: 307
Script:
#!/bin/bash
# Check the actual usage context of these strings
rg -A 2 -B 2 '\b(Error:|No transactions found|Import transactions|Select new file)\b' packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.jsx packages/desktop-client/src/components/modals/KeyboardShortcutModal.tsx
Length of output: 2750
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx (3)
19-19: LGTM: Import of i18next translation function
The addition of the i18next translation function import is appropriate and correctly placed with other imports.
Line range hint 322-432: LGTM: Field label translations in ChildTransactionEdit
The translation implementation for field labels is consistent and follows i18next best practices. All necessary fields have been properly internationalized:
- Payee
- Amount
- Category
- Notes
- Delete split
Line range hint 778-1021: LGTM: Comprehensive translations in TransactionEditInner
The translation implementation is thorough and consistent throughout the component. All user-facing strings have been properly internationalized:
- Dynamic titles (New Transaction/Transaction)
- All field labels
- Action button texts
packages/desktop-client/src/components/modals/EditRuleModal.jsx (5)
Line range hint 338-351: LGTM: Schedule description translations are properly implemented.
The translation keys for schedule description labels ("Payee", "Amount", "Next") are correctly implemented using the t function.
Line range hint 1021-1063: LGTM: Modal header and stage selection translations are properly implemented.
The translation keys for modal title and stage selection buttons ("Rule", "Pre", "Default", "Post") are correctly implemented using the t function.
Line range hint 1082-1093: LGTM: Rule conditions and actions UI translations are properly implemented.
The translation keys for conditions and actions UI elements ("If", "of these conditions match:", "Then apply these actions:", "Add action") are correctly implemented using the t function.
Also applies to: 1106-1106, 1114-1114
Line range hint 1145-1156: LGTM: Split transaction related translations are properly implemented.
The translation keys for split transaction labels ("Apply to all", "Split", "Delete split") are correctly implemented using the t function.
1203-1203: LGTM: Action buttons and footer translations are properly implemented.
The translation keys for action buttons and footer elements ("Add action", "Add another split", "Split into multiple transactions", "This rule applies to these transactions:", "Apply actions", "Cancel", "Save") are correctly implemented using the t function.
Also applies to: 1218-1219, 1236-1236, 1244-1244, 1265-1267
packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx (15)
6-6: Appropriate Import of t from 'i18next'
Importing t directly from 'i18next' is consistent with the project's conventions and acceptable for translation purposes.
86-86: Correct Use of t() for Title Translation
The t() function is correctly used to translate the title based on the amount condition, ensuring proper localization.
159-159: Proper Translation of 'Projected Savings'
Wrapping 'Projected Savings' with t() enables translation and is implemented correctly.
291-293: Correct Usage of t() with Placeholders in aria-label
The aria-label correctly uses t() with placeholders, enhancing accessibility with translated strings.
311-313: Consistent Use of t() for aria-label Translation
Using t() to translate aria-label with dynamic placeholders for categoryName is appropriate and improves accessibility.
617-619: Proper Translation in aria-label for Spent Amount
The aria-label correctly includes the categoryName placeholder within t(), ensuring the spent amount is accessible and translatable.
827-829: Appropriate Translation of aria-label in Group Header
The aria-label for toggling collapse includes dynamic groupName, correctly using t() for translation.
1024-1026: Correct Use of t() in aria-label for IncomeGroupHeader
The aria-label for toggling collapse in the income group header is properly translated with dynamic groupName.
1183-1185: Proper Translation in aria-label for Editing Category
Using t() to translate the aria-label for editing a category with dynamic categoryName is correctly implemented.
1846-1846: Translation of 'Budgeted' is Correct
The t() function is appropriately used to translate the 'Budgeted' label.
1905-1905: Proper Translation of 'Spent' Label
The 'Spent' label is correctly wrapped with t() for translation.
1943-1943: Correct Use of t() for 'Balance' Label
The 'Balance' label is properly translated using the t() function.
1996-1996: Translation of aria-label for Previous Month Button
The aria-label for the previous month button is correctly localized using t().
2025-2025: Localized aria-label for Next Month Button
The aria-label for the next month button is properly translated with t().
1526-1527: 🛠️ Refactor suggestion
Use i18next Pluralization for 'transaction(s)'
Manually handling singular and plural forms can lead to localization issues. Utilize i18next's pluralization features to correctly handle different languages and plural forms.
Apply this diff to improve pluralization:
-{count} {t('uncategorized')}
-{count === 1 ? t('transaction') : t('transactions')}
+{t('uncategorized transaction', { count })}Ensure that your translation files include pluralization entries, for example:
{
"uncategorized transaction": "{{count}} uncategorized transaction",
"uncategorized transaction_plural": "{{count}} uncategorized transactions"
}
packages/desktop-client/src/components/reports/SaveReportDelete.tsx
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/modals/CreateAccountModal.tsx
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/modals/EditRuleModal.jsx
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx
Outdated
Show resolved
Hide resolved
awaisalee
changed the title
Go to the Summary and download the artifact |
Co-authored-by: Michael Clark <[email protected]>
Co-authored-by: Michael Clark <[email protected]>
MikesGlitch
changed the title
Welcome @MikesGlitch 🥳 |
Marked files for translation: