Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Marked files for translation #3832

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

awaisalee
Copy link
Contributor

@awaisalee awaisalee commented Nov 14, 2024

Marked files for translation:

  • packages/desktop-client/src/components/modals/EnvelopeBalanceMenuModal.tsx
  • packages/desktop-client/src/components/modals/EnvelopeBudgetMenuModal.tsx
  • packages/desktop-client/src/components/modals/EnvelopeBudgetMonthMenuModal.tsx
  • packages/desktop-client/src/components/modals/EnvelopeBudgetSummaryModal.tsx
  • packages/desktop-client/src/components/modals/LoadBackupModal.tsx
  • packages/desktop-client/src/components/modals/TrackingBalanceMenuModal.tsx
  • packages/desktop-client/src/components/modals/TrackingBudgetMenuModal.tsx
  • packages/desktop-client/src/components/modals/TrackingBudgetMonthMenuModal.tsx
  • packages/desktop-client/src/components/modals/TrackingBudgetSummaryModal.tsx
  • packages/desktop-client/src/components/reports/Header.tsx
  • packages/desktop-client/src/components/reports/reports/NetWorth.tsx
  • packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx

Copy link

netlify bot commented Nov 14, 2024

Deploy Preview for actualbudget ready!

Name Link
🔨 Latest commit 45b8f08
🔍 Latest deploy log https://app.netlify.com/sites/actualbudget/deploys/6735f88dfe580a0008476568
😎 Deploy Preview https://deploy-preview-3832.demo.actualbudget.org
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

github-actions bot commented Nov 14, 2024

Bundle Stats — desktop-client

Hey 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

Files count Total bundle size % Changed
9 5.35 MB → 5.36 MB (+983 B) +0.02%
Changeset
File Δ Size
node_modules/clsx/dist/clsx.js 🆕 +509 B 0 B → 509 B
node_modules/clsx/dist/clsx.js?commonjs-module 🆕 +27 B 0 B → 27 B
src/components/modals/EnvelopeBudgetMonthMenuModal.tsx 📈 +330 B (+4.77%) 6.75 kB → 7.07 kB
src/components/modals/TrackingBudgetMonthMenuModal.tsx 📈 +275 B (+4.16%) 6.45 kB → 6.72 kB
src/components/modals/TrackingBudgetSummaryModal.tsx 📈 +37 B (+2.29%) 1.58 kB → 1.61 kB
src/components/modals/EnvelopeBudgetSummaryModal.tsx 📈 +43 B (+1.29%) 3.27 kB → 3.31 kB
src/components/reports/Header.tsx 📈 +58 B (+1.18%) 4.81 kB → 4.87 kB
src/components/modals/LoadBackupModal.tsx 📈 +58 B (+1.14%) 4.95 kB → 5.01 kB
src/components/select/RecurringSchedulePicker.tsx 📈 +55 B (+0.33%) 16.24 kB → 16.29 kB
src/components/modals/TrackingBalanceMenuModal.tsx 📈 +5 B (+0.20%) 2.5 kB → 2.5 kB
src/components/modals/EnvelopeBalanceMenuModal.tsx 📈 +5 B (+0.19%) 2.56 kB → 2.56 kB
src/components/modals/TrackingBudgetMenuModal.tsx 📈 +5 B (+0.18%) 2.67 kB → 2.68 kB
src/components/modals/EnvelopeBudgetMenuModal.tsx 📈 +5 B (+0.18%) 2.67 kB → 2.68 kB
src/components/reports/reports/NetWorth.tsx 📈 +6 B (+0.09%) 6.18 kB → 6.18 kB
node_modules/react-grid-layout/build/ReactGridLayout.js 📉 -1 B (-0.00%) 24.96 kB → 24.96 kB
node_modules/react-grid-layout/build/GridItem.js 📉 -1 B (-0.00%) 21.49 kB → 21.49 kB
node_modules/react-grid-layout/build/components/WidthProvider.js 📉 -1 B (-0.02%) 5.22 kB → 5.22 kB
node_modules/clsx/dist/clsx.mjs 🔥 -368 B (-100%) 368 B → 0 B
node_modules/clsx/dist/clsx.mjs?commonjs-proxy 🔥 -64 B (-100%) 64 B → 0 B
View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset File Size % Changed
static/js/index.js 3.37 MB → 3.37 MB (+986 B) +0.03%

Smaller

Asset File Size % Changed
static/js/ReportRouter.js 1.49 MB → 1.49 MB (-3 B) -0.00%

Unchanged

Asset File Size % Changed
static/js/workbox-window.prod.es5.js 5.69 kB 0%
static/js/indexeddb-main-thread-worker-e59fee74.js 13.5 kB 0%
static/js/BackgroundImage.js 122.29 kB 0%
static/js/resize-observer.js 18.37 kB 0%
static/js/narrow.js 82.76 kB 0%
static/js/AppliedFilters.js 21.3 kB 0%
static/js/wide.js 242.64 kB 0%

Copy link
Contributor

github-actions bot commented Nov 14, 2024

Bundle Stats — loot-core

Hey 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

Files count Total bundle size % Changed
1 1.31 MB 0%

Changeset

No files were changed

View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

No assets were bigger

Smaller

No assets were smaller

Unchanged

Asset File Size % Changed
kcab.worker.js 1.31 MB 0%

@awaisalee awaisalee force-pushed the feature/i18n-integration branch from 7a6b03b to 45b8f08 Compare November 14, 2024 13:18
@awaisalee awaisalee changed the title WIP Marked files for translation Marked files for translation Nov 14, 2024
Copy link
Contributor

coderabbitai bot commented Nov 14, 2024

Walkthrough

The pull request introduces internationalization features across several modal components within the desktop client application. It primarily involves integrating the useTranslation hook from the react-i18next library and the t function for translating user-facing text. Specific changes include replacing hardcoded strings such as "Balance," "Budgeted," "Edit notes," and modal titles with their translatable counterparts. This allows the text to adapt dynamically based on the user's language settings. Components affected include EnvelopeBalanceMenuModal, EnvelopeBudgetMenuModal, EnvelopeBudgetMonthMenuModal, EnvelopeBudgetSummaryModal, LoadBackupModal, TrackingBalanceMenuModal, TrackingBudgetMenuModal, TrackingBudgetMonthMenuModal, TrackingBudgetSummaryModal, Header, NetWorth, and RecurringSchedulePicker. The overall control flow, logic, and error handling of these components remain unchanged, focusing solely on enhancing localization capabilities.

Possibly related PRs

Suggested labels

::sparkles: Merged

Suggested reviewers

  • MikesGlitch

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 eslint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/desktop-client/src/components/modals/EnvelopeBalanceMenuModal.tsx

Oops! Something went wrong! :(

ESLint: 8.57.1

ESLint couldn't find the plugin "eslint-plugin-eslint-plugin".

(The package "eslint-plugin-eslint-plugin" was not found when loaded as a Node module from the directory "/packages/eslint-plugin-actual".)

It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:

npm install eslint-plugin-eslint-plugin@latest --save-dev

The plugin "eslint-plugin-eslint-plugin" was referenced from the config file in "packages/eslint-plugin-actual/.eslintrc.js".

If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (14)
packages/desktop-client/src/components/modals/TrackingBudgetSummaryModal.tsx (1)

29-29: Consider using structured translation keys

While using English text directly as translation keys works, consider using structured keys for better maintainability and type safety. This helps prevent typos and makes it easier to track translations.

Example improvement:

-title={t('Budget Summary')}
+title={t('modals.budget_summary.title')}

Benefits:

  • Provides better context for translators
  • Enables type checking with TypeScript when using type-safe translation keys
  • Makes it easier to track missing translations
  • Follows i18n best practices for key structuring
packages/desktop-client/src/components/modals/TrackingBalanceMenuModal.tsx (1)

Line range hint 44-104: Consider translating the remaining user-facing strings

While the "Balance" string has been internationalized, there are other potential candidates for translation in this component:

  • The modal name "tracking-balance-menu" (if used in aria labels or debugging)
  • The category name display (though this is likely user data)

Here's how you could enhance the internationalization:

- <Modal name="tracking-balance-menu">
+ <Modal name={t('tracking-balance-menu')}>

Note: Only proceed with these additional translations if these strings are indeed meant to be shown to users in some form (e.g., accessibility labels, tooltips).

packages/desktop-client/src/components/modals/EnvelopeBalanceMenuModal.tsx (1)

71-71: Consider using a more specific translation key

While the translation works, using a generic key like 'Balance' might lead to conflicts or inconsistent translations across different contexts. Consider using a more specific key that includes the component context.

-              {t('Balance')}
+              {t('envelope.balance.label')}
packages/desktop-client/src/components/modals/EnvelopeBudgetMenuModal.tsx (1)

87-87: Consider translating additional user-facing strings.

While the "Budgeted" string is now properly internationalized, consider also translating the modal name "envelope-budget-menu" to maintain consistency in the internationalization effort.

- <Modal name="envelope-budget-menu">
+ <Modal name={t('envelope-budget-menu')}>
packages/desktop-client/src/components/modals/TrackingBudgetMenuModal.tsx (1)

87-87: Consider additional translation opportunities

The translation of "Budgeted" is correctly implemented. However, consider if the category name (category.name) should also be translatable depending on your internationalization requirements.

If category names should be translatable, consider wrapping them in the translation function:

-title={<ModalTitle title={category.name} shrinkOnOverflow />}
+title={<ModalTitle title={t(category.name)} shrinkOnOverflow />}
packages/desktop-client/src/components/modals/LoadBackupModal.tsx (2)

84-84: Consider using a more specific translation key

While the translation works, using a more specific key like 'modals.backup.title' would better organize translations and avoid potential conflicts.

-            title={t('Load Backup')}
+            title={t('modals.backup.title')}

Line range hint 1-150: Overall implementation looks good with minor suggestions

The internationalization changes are well-implemented and maintain the component's functionality while making it translation-ready. Consider:

  1. Using a more structured approach for translation keys (e.g., nested keys like 'modals.backup.messages.warning')
  2. Adding translation key comments to provide context for translators

To maintain consistency across the application, consider creating a translation key structure guide in the project documentation. This would help maintain a standardized approach as more components are internationalized.

packages/desktop-client/src/components/reports/Header.tsx (1)

97-97: Consider using more specific translation keys and consistent capitalization

While the translations are functional, consider these improvements for better maintainability:

  1. Use more specific keys instead of plain English
  2. Maintain consistent capitalization in keys
  3. Consider adding a namespace for reports/header section

Example refactor:

- {t('Static')}
+ {t('header.mode.static')}
- {t('Live')}
+ {t('header.mode.live')}
- {t('to')}
+ {t('header.date_range.connector')}
- {t('1 month')}
+ {t('header.time_range.one_month')}
- {t('3 months')}
+ {t('header.time_range.three_months')}
- {t('6 months')}
+ {t('header.time_range.six_months')}
- {t('1 Year')}
+ {t('header.time_range.one_year')}
- {t('All Time')}
+ {t('header.time_range.all_time')}

Also applies to: 116-116, 140-140, 147-147, 153-153, 159-159, 169-169

packages/desktop-client/src/components/modals/TrackingBudgetMonthMenuModal.tsx (2)

89-91: Consider simplifying the notes conditional expression

While the translation is correctly implemented, the ternary expression could be more readable.

Consider this alternative:

-notes={
-  originalNotes?.length > 0 ? originalNotes : t('No notes')
-}
+notes={originalNotes || t('No notes')}

This achieves the same result but is more concise, as empty strings and arrays are falsy in JavaScript.


Line range hint 182-184: Fix inconsistent translation pattern

There's an untranslated notification message that uses template literals instead of the t function.

Apply this change to maintain consistency with other notification messages:

-message: `${displayMonth} budgets have all been set to ${numberOfMonths === 12 ? 'yearly' : `${numberOfMonths} month`} average.`,
+message: t(
+  '{displayMonth} budgets have all been set to {period} average.',
+  {
+    displayMonth,
+    period: numberOfMonths === 12 ? t('yearly') : t('{n} month', { n: numberOfMonths })
+  }
+),
packages/desktop-client/src/components/modals/EnvelopeBudgetMonthMenuModal.tsx (2)

Line range hint 180-180: Missing translation for average budget message

This message should be translated like the other notification messages:

- message: `${displayMonth} budgets have all been set to ${numberOfMonths === 12 ? 'yearly' : `${numberOfMonths} month`} average.`,
+ message: t(
+   '{displayMonth} budgets have all been set to {period} average.',
+   {
+     displayMonth,
+     period: numberOfMonths === 12 ? t('yearly') : t('{count} month', { count: numberOfMonths })
+   }
+ ),

Line range hint 1-224: Consider implementing i18n type safety

While the translations are correctly implemented, consider enhancing type safety for translations:

  1. Use TypeScript's type system to ensure all translation keys exist
  2. Consider using a tool like typesafe-i18n or similar to catch missing translations at compile time
packages/desktop-client/src/components/reports/reports/NetWorth.tsx (1)

252-254: Consider using a shorter translation key.

While using full text as translation keys works, it's generally recommended to use shorter, more maintainable keys. This makes it easier to manage translations and reduces the risk of inconsistencies when the same text appears in multiple places.

Consider this approach:

-              {t(
-                'Net worth shows the balance of all accounts over time, including all of your investments. Your "net worth" is considered to be the amount you'd have if you sold all your assets and paid off as much debt as possible. If you hover over the graph, you can also see the amount of assets and debt individually.',
-              )}
+              {t('networth.explanation')}

Then add this key-value pair to your translation file:

{
  "networth.explanation": "Net worth shows the balance of all accounts over time, including all of your investments. Your \"net worth\" is considered to be the amount you'd have if you sold all your assets and paid off as much debt as possible. If you hover over the graph, you can also see the amount of assets and debt individually."
}
packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx (1)

397-397: LGTM: Consistent translation of UI elements

All user-facing strings in the RecurringScheduleTooltip have been properly internationalized. However, there's room for improvement.

Consider also translating the static options in the Select components for consistency:

  • 'indefinitely', 'for', 'until' in the end mode dropdown
  • 'before', 'after' in the weekend solve dropdown
  • Day names in DAY_OF_WEEK_OPTIONS
  • 'Day' and 'Last' in the monthly patterns dropdowns

Example for the end mode options:

options={[
-  ['never', 'indefinitely'],
-  ['after_n_occurrences', 'for'],
-  ['on_date', 'until'],
+  ['never', t('indefinitely')],
+  ['after_n_occurrences', t('for')],
+  ['on_date', t('until')],
]}

Also applies to: 434-434, 449-449, 472-472, 508-508, 525-525, 533-533, 539-539

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 58baf74 and 45b8f08.

⛔ Files ignored due to path filters (1)
  • upcoming-release-notes/3832.md is excluded by !**/*.md
📒 Files selected for processing (12)
  • packages/desktop-client/src/components/modals/EnvelopeBalanceMenuModal.tsx (2 hunks)
  • packages/desktop-client/src/components/modals/EnvelopeBudgetMenuModal.tsx (2 hunks)
  • packages/desktop-client/src/components/modals/EnvelopeBudgetMonthMenuModal.tsx (6 hunks)
  • packages/desktop-client/src/components/modals/EnvelopeBudgetSummaryModal.tsx (4 hunks)
  • packages/desktop-client/src/components/modals/LoadBackupModal.tsx (3 hunks)
  • packages/desktop-client/src/components/modals/TrackingBalanceMenuModal.tsx (2 hunks)
  • packages/desktop-client/src/components/modals/TrackingBudgetMenuModal.tsx (2 hunks)
  • packages/desktop-client/src/components/modals/TrackingBudgetMonthMenuModal.tsx (7 hunks)
  • packages/desktop-client/src/components/modals/TrackingBudgetSummaryModal.tsx (2 hunks)
  • packages/desktop-client/src/components/reports/Header.tsx (6 hunks)
  • packages/desktop-client/src/components/reports/reports/NetWorth.tsx (1 hunks)
  • packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx (10 hunks)
🧰 Additional context used
📓 Learnings (2)
packages/desktop-client/src/components/modals/LoadBackupModal.tsx (1)
Learnt from: tlesicka
PR: actualbudget/actual#3689
File: packages/desktop-client/src/components/modals/LoadBackupModal.tsx:162-190
Timestamp: 2024-11-10T16:45:25.627Z
Learning: Adding progress indicators for backup operations in the budget application requires updates to the server backend, and may be beyond the scope of a single PR.
packages/desktop-client/src/components/modals/TrackingBudgetMenuModal.tsx (2)
Learnt from: qedi-r
PR: actualbudget/actual#3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-11-10T16:45:25.627Z
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-11-10T16:45:25.627Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is acceptable and commonly used in the codebase.
🔇 Additional comments (28)
packages/desktop-client/src/components/modals/TrackingBudgetSummaryModal.tsx (3)

2-2: LGTM: Import statement is correctly placed

The useTranslation hook import is properly added and necessary for implementing internationalization.


22-22: LGTM: Hook usage follows React best practices

The useTranslation hook is correctly implemented at the component's top level, following React hooks rules.


Line range hint 34-46: Verify translation coverage in child components

While the modal title is now translatable, ensure that the child components (IncomeTotal, ExpenseTotal, and Saved) also support translations for a consistent user experience.

✅ Verification successful

Translation support is properly implemented in all child components

All three components (IncomeTotal, ExpenseTotal, and Saved) already have proper translation support:

  • They import and use useTranslation hook from react-i18next
  • All visible strings are wrapped in translation functions:
    • IncomeTotal: Uses t('Income')
    • ExpenseTotal: Uses t('Expenses')
    • Saved: Uses Trans component and t() for multiple strings including "Projected Savings", "Overspent", "Saved", etc.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if child components have translation support
echo "Checking translation implementation in child components..."

# Search for translation usage in related components
rg -l "useTranslation|react-i18next" "packages/desktop-client/src/components/budget/tracking/budgetsummary/"

# Search for potential hardcoded strings in these components
echo "Checking for potential hardcoded strings..."
rg -i "total|saved|income|expense" "packages/desktop-client/src/components/budget/tracking/budgetsummary/" \
  --type tsx \
  --glob '!*.test.{ts,tsx}' \
  -C 2

Length of output: 1020


Script:

#!/bin/bash
# Let's check the content of these components to verify translation implementation
echo "Checking IncomeTotal component..."
cat packages/desktop-client/src/components/budget/tracking/budgetsummary/IncomeTotal.tsx

echo -e "\nChecking ExpenseTotal component..."
cat packages/desktop-client/src/components/budget/tracking/budgetsummary/ExpenseTotal.tsx

echo -e "\nChecking Saved component..."
cat packages/desktop-client/src/components/budget/tracking/budgetsummary/Saved.tsx

Length of output: 4612

packages/desktop-client/src/components/modals/TrackingBalanceMenuModal.tsx (3)

6-6: LGTM: Import statement follows i18next conventions

The import of the t function from i18next is correctly implemented.


69-69: LGTM: String translation implementation is correct

The hardcoded "Balance" string has been properly replaced with the translation function call.


Line range hint 1-104: Verify i18next initialization

Since this is one of the first files being marked for translation, we should ensure that i18next is properly initialized in the application.

✅ Verification successful

Let me try a different search pattern to find i18next initialization and configuration.


Let me search for i18next initialization code in the codebase.


i18next is properly initialized and configured

The codebase has proper i18next initialization:

  • i18next is initialized in a dedicated i18n.ts file
  • It uses initReactI18next and i18next-resources-to-backend for loading translations
  • The initialization file is imported at the application entry point (index.tsx)
  • Language files are loaded dynamically from the locale directory
  • Default language is set to 'en' with ability to override via localStorage
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for i18next setup files and initialization
echo "Checking for i18next configuration..."
rg -l "i18next" --type ts --type tsx packages/desktop-client/src/
echo "\nChecking for translation initialization..."
ast-grep --pattern 'i18next.init' 

Length of output: 6063


Script:

#!/bin/bash
# Search for i18next configuration and initialization with proper file type
echo "Checking for i18next configuration..."
rg -l "i18next" packages/desktop-client/src/
echo "\nChecking for i18next initialization code..."
rg -A 5 "i18next" packages/desktop-client/src/

Length of output: 136238


Script:

#!/bin/bash
# Check for i18next initialization in i18n.ts
echo "Checking i18next initialization in i18n.ts..."
cat packages/desktop-client/src/i18n.ts

# Check for i18next usage in app entry points
echo "\nChecking i18next usage in app entry points..."
rg -A 5 "import.*i18n" packages/desktop-client/src/index.tsx

Length of output: 1290

packages/desktop-client/src/components/modals/EnvelopeBalanceMenuModal.tsx (1)

6-7: LGTM: Clean import of translation function

The import of the t function from i18next is properly placed and follows the project's import organization.

packages/desktop-client/src/components/modals/EnvelopeBudgetMenuModal.tsx (1)

8-8: LGTM! Import of i18next translation function.

The import is correctly placed and aligns with the internationalization objective.

packages/desktop-client/src/components/modals/TrackingBudgetMenuModal.tsx (2)

8-8: LGTM: Import follows project conventions

The direct import of t from 'i18next' aligns with the project's established practices.


Line range hint 89-106: Verify number formatting localization

While text translation is being implemented, ensure that number formatting (including currency symbols and decimal separators) is properly handled for different locales. The FocusableAmountInput component and the integerToAmount/amountToInteger utilities should respect the user's locale settings.

✅ Verification successful

Based on the shell script results, I can now generate the final response since I have enough information about the number formatting implementation:

Number formatting is properly localized

The codebase already handles number formatting localization correctly through:

  • A locale-aware Intl.NumberFormat formatter in packages/loot-core/src/shared/util.ts that's used by the currency formatting functions
  • Proper separation between storage (integer cents) and display (decimal amounts) through integerToAmount/amountToInteger utility functions
  • Consistent usage of these utilities throughout the codebase for all amount conversions
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if number formatting utilities handle localization
rg -A 5 "integerToAmount|amountToInteger" "packages/desktop-client/src" "packages/loot-core/src"

# Look for any existing locale-aware number formatting
ast-grep --pattern 'new Intl.NumberFormat'

Length of output: 64370

packages/desktop-client/src/components/modals/EnvelopeBudgetSummaryModal.tsx (2)

2-2: LGTM: Translation setup is correctly implemented

The useTranslation hook is properly imported and initialized at the component level, following React i18n best practices.

Also applies to: 39-39


Line range hint 1-138: Test translations with different locales

The translation implementation looks good overall. To ensure completeness:

  1. Test the component with different locales to verify all strings are properly translated
  2. Verify that dynamic content (numbers, dates, currency) is formatted according to locale conventions
#!/bin/bash
# Check for any remaining hardcoded strings in the file
echo "Checking for potential hardcoded strings..."
rg -n "['\"][A-Za-z\s]+['\"]" packages/desktop-client/src/components/modals/EnvelopeBudgetSummaryModal.tsx

# Look for translation keys in the locale files
echo "Verifying translation keys exist..."
fd -e json -e yaml "translation" | xargs rg "Transfer: To Budget|Cover: Overbudgeted|Budget Summary"
packages/desktop-client/src/components/modals/LoadBackupModal.tsx (3)

1-2: LGTM: Clean import addition

The useTranslation hook is properly imported alongside other React imports.


77-77: LGTM: Proper hook initialization

The useTranslation hook is correctly initialized within the component scope.


100-139: Verify translations for critical warning messages

The translations are properly implemented, especially for the critical warning messages about device sync and backup implications. However, we should ensure these important messages are properly translated across all supported languages to maintain their clarity and impact.

packages/desktop-client/src/components/reports/Header.tsx (3)

2-2: LGTM: Translation hook import added correctly

The useTranslation hook is properly imported from react-i18next.


64-64: LGTM: Translation hook initialized properly

The useTranslation hook is correctly initialized and destructured at the component's top level, following React hooks best practices.


Line range hint 173-177: Verify translations in child components

Please ensure that the text in child components FilterButton and AppliedFilters is also marked for translation.

Also applies to: 195-203

✅ Verification successful

Let me check the FiltersMenu.jsx file since it's in JSX format and verify its content.


No translation issues found in child components

Based on the verification results, all text strings that need translation in the child components are properly handled:

  • FiltersMenu.jsx uses the translation hook (useTranslation) and Trans component from react-i18next
  • FilterExpression.tsx uses the translation hook (useTranslation) and properly translates UI elements including the "Delete filter" button label
  • ConditionsOpMenu.tsx doesn't contain any hardcoded UI strings that need translation
  • AppliedFilters.tsx only contains structural code without user-facing strings
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if FilterButton and AppliedFilters components contain hardcoded strings
echo "Checking FilterButton component..."
rg -l "FilterButton" packages/desktop-client/src/components/filters/
echo "Checking for hardcoded strings in these files..."
rg -i "['\"](.*?)['\"]" packages/desktop-client/src/components/filters/FiltersMenu.tsx

echo "Checking AppliedFilters component..."
rg -l "AppliedFilters" packages/desktop-client/src/components/filters/
echo "Checking for hardcoded strings in these files..."
rg -i "['\"](.*?)['\"]" packages/desktop-client/src/components/filters/AppliedFilters.tsx

Length of output: 1392


Script:

#!/bin/bash
# Check FiltersMenu.jsx for hardcoded strings
echo "Checking FiltersMenu.jsx content..."
rg -i "['\"](.*?)['\"]" packages/desktop-client/src/components/filters/FiltersMenu.jsx

# Also check FilterExpression since it's imported in AppliedFilters
echo "Checking FilterExpression content..."
rg -l "FilterExpression" packages/desktop-client/src/components/filters/
rg -i "['\"](.*?)['\"]" packages/desktop-client/src/components/filters/FilterExpression.tsx

# Check ConditionsOpMenu as well since it's used
echo "Checking ConditionsOpMenu content..."
rg -i "['\"](.*?)['\"]" packages/desktop-client/src/components/filters/ConditionsOpMenu.tsx

Length of output: 7738

packages/desktop-client/src/components/modals/TrackingBudgetMonthMenuModal.tsx (3)

3-3: LGTM: Proper i18n setup

The internationalization setup follows best practices with proper import and hook initialization.

Also applies to: 31-31


Line range hint 1-214: Verify translation key definitions

Let's ensure all translation keys used in this component are defined in the translation files.

#!/bin/bash
# Extract all translation keys used in this file
echo "Translation keys used in TrackingBudgetMonthMenuModal:"
rg "t\(['\"]([^'\"]*)['\"]\)" packages/desktop-client/src/components/modals/TrackingBudgetMonthMenuModal.tsx -o -r '$1'

# Check if these keys exist in the translation files
echo -e "\nSearching for these keys in translation files:"
rg "(['\"]No notes['\"]|['\"]Edit notes['\"]|['\"]Actions['\"])" packages/desktop-client/src/locales/

120-120: Verify translation key consistency across components

The button labels are correctly translated, but we should verify that these translation keys are used consistently across similar modal components.

Also applies to: 150-150

✅ Verification successful

Translation keys are consistently used across modal components

The translation keys "Edit notes" and "Actions" are consistently used across multiple modal components in the codebase:

  • "Edit notes" appears in 5 modal components (TrackingBudgetMonth, EnvelopeBudgetMonth, Account, CategoryGroup, and Category)
  • "Actions" appears in 3 modal components (TrackingBudgetMonth, EnvelopeBudgetMonth, and SelectLinkedAccounts)

This consistent usage across similar components indicates proper translation key reuse and maintainability.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for 'Edit notes' and 'Actions' translations in other modal components
rg "('Edit notes'|\"Edit notes\")" packages/desktop-client/src/components/modals/
rg "('Actions'|\"Actions\")" packages/desktop-client/src/components/modals/

Length of output: 576


Script:

#!/bin/bash
# Let's search for translation keys in the entire codebase
fd . -t f -e ts -e tsx -e js -e jsx | xargs rg "t\(['\"]Edit notes['\"]\)" -l
fd . -t f -e ts -e tsx -e js -e jsx | xargs rg "t\(['\"]Actions['\"]\)" -l

# Also search for potential translation files
fd "translations|locales|i18n" --type f

Length of output: 891

packages/desktop-client/src/components/modals/EnvelopeBudgetMonthMenuModal.tsx (3)

3-3: LGTM: Translation setup is correct

The import and initialization of the translation hook follow React best practices.

Also applies to: 60-60


118-118: LGTM: Button text translations are properly implemented

Button labels are correctly wrapped with the translation function.

Also applies to: 148-148


160-163: LGTM: Notification messages properly handle translations with variables

The notification messages correctly use the translation function with proper variable interpolation for displayMonth.

Also applies to: 170-173, 191-194, 201-204, 211-214

packages/desktop-client/src/components/reports/reports/NetWorth.tsx (1)

249-249: LGTM! Text properly marked for translation.

The heading text is correctly wrapped with the translation function.

packages/desktop-client/src/components/select/RecurringSchedulePicker.tsx (3)

10-10: LGTM: Import for i18next translation function

The addition of the i18next import is appropriate for implementing internationalization.


320-320: LGTM: Translated aria-labels for accessibility

The aria-labels for add/remove buttons have been properly internationalized while maintaining accessibility.

Also applies to: 333-333


575-575: LGTM: Translated default text

The default text for no recurring date has been properly internationalized.

@awaisalee
Copy link
Contributor Author

@youngcw @MikesGlitch, Could you please review PR?

Copy link
Contributor

@MikesGlitch MikesGlitch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@MikesGlitch MikesGlitch merged commit 670419b into actualbudget:master Nov 14, 2024
20 checks passed
This was referenced Nov 14, 2024
@tlesicka
Copy link
Contributor

@awaisalee I know I'm a little late to this, but didn't look at this until I was syncing my PR's with the master. No problems, just a question.
Why did you use the t function {t('text')} instead using the Trans tag <Trans>text</Trans>?
Not saying you did anything wrong, I'm just curious. In my PR (#3689) I had used <Trans> tags in LoadBackupModal.tsx and had quite a few merge errors to resolve.

Again, no issues, just curious as to which way I should code things in the future. I have another PR that I'm working on that will create a couple new modals and want to have consistency with what is already established.

@MikesGlitch or @youngcw if either of you have any thoughts, let me know.

@MikesGlitch
Copy link
Contributor

MikesGlitch commented Nov 15, 2024

My only thoughts about it are we can't enforce a code styling rule if it's not in lint. New people won't know about it (which is important for open source).

I prefer using <Trans> in rendering code but wouldn't turn away a good contribution because it used t.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants