-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
π Fix parameter formatting issues #3841
π Fix parameter formatting issues #3841
Conversation
β Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats β desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged
|
Bundle Stats β loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged No assets were unchanged |
WalkthroughThe pull request introduces changes to three files related to modal components and error handling within the application. In the Possibly related PRs
Suggested labels
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
packages/desktop-client/src/components/modals/EnvelopeBudgetMonthMenuModal.tsxOops! 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:
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? πͺ§ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
π§Ή Outside diff range and nitpick comments (4)
packages/loot-core/src/shared/errors.ts (2)
70-72
: Consider standardizing error message formatWhile the i18next syntax is correct, the error message format differs from others in the file. Consider standardizing the format:
- 'Something went wrong trying to download that file, sorry! Visit https://actualbudget.org/contact/ for support. reason: {{reason}}{{info}}', + 'Something went wrong trying to download that file, sorry! Visit https://actualbudget.org/contact/ for support. (ref: {{reason}}{{info}})',This matches the format used in
getUploadError
where "ref:" precedes the technical details.
Line range hint
103-108
: Fix inconsistent string interpolation in getSyncErrorThere's an inconsistency in the string interpolation syntax within the same function:
- 'Budget "{id}" not found. Check the id of your budget in the Advanced section of the settings page.', + 'Budget "{{id}}" not found. Check the id of your budget in the Advanced section of the settings page.',Line 104 uses single quotes around
id
while line 108 correctly uses double curly braces. This should be updated for consistency and proper i18next functionality.packages/desktop-client/src/components/modals/TrackingBudgetMonthMenuModal.tsx (1)
Line range hint
184-184
: Consider using i18next for consistencyThis message uses template literals instead of i18next translations, unlike other notification messages in this file.
Consider updating to use i18next translation:
- 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 }) + } + ),packages/desktop-client/src/components/modals/EnvelopeBudgetMonthMenuModal.tsx (1)
Line range hint
178-181
: Inconsistent translation handling detected.This notification message is using direct string interpolation instead of the translation system. This should be consistent with other notification messages to support proper localization.
Consider updating to use the translation system:
- 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 }) + } + ),
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
β Files ignored due to path filters (1)
upcoming-release-notes/3841.md
is excluded by!**/*.md
π Files selected for processing (3)
packages/desktop-client/src/components/modals/EnvelopeBudgetMonthMenuModal.tsx
(5 hunks)packages/desktop-client/src/components/modals/TrackingBudgetMonthMenuModal.tsx
(4 hunks)packages/loot-core/src/shared/errors.ts
(3 hunks)
π Additional comments (9)
packages/loot-core/src/shared/errors.ts (2)
37-39
: LGTM! Correct i18next string interpolation syntax
The update to double curly braces {{reason}}
follows i18next's proper variable interpolation syntax.
Line range hint 37-108
: Verify consistent i18next syntax across the codebase
Let's ensure all translation strings use the correct double curly braces syntax.
β Verification successful
Translation interpolation syntax is consistent across the codebase
The verification shows that all variable interpolations in translation strings correctly use the double curly braces syntax {{variable}}
. The few instances with single braces found are either:
- Template literals using
${}
for string interpolation - JSX markup with curly braces for embedding expressions
- Not actual variable interpolations
π Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for potential single-brace i18next interpolation
rg "t\(['\"].*?[^{]{[^{].*?['\"]" --type ts
# Search for translation calls with variables to verify
rg "t\(['\"].*?{.*?}.*?['\"].*?,.*?{.*?}" --type ts
Length of output: 359
Script:
#!/bin/bash
# Search for t() function calls with single curly brace interpolation
rg "t\('.*\{[^{].*'\)" --type ts
rg 't\(".*\{[^{].*"\)' --type ts
# Search for t() function calls with variables
rg "t\(['\"].*\{\{.*\}\}.*['\"].*,.*\{.*\}" --type ts
Length of output: 2051
π§° Tools
πͺ Biome
[error] 68-68: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 68-68: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/desktop-client/src/components/modals/TrackingBudgetMonthMenuModal.tsx (4)
163-166
: LGTM: Correct translation string format
The updated string interpolation syntax using double curly braces is correct for i18next translations.
173-176
: LGTM: Consistent translation format
The translation string format is consistent with the previous change and follows i18next conventions.
194-197
: LGTM: Proper translation string format
The translation string format is correct for i18next.
204-207
: LGTM: Translation format is correct
The translation string format follows i18next conventions consistently.
Let's verify if there are any other translation strings that need similar updates:
β Verification successful
Translation format is consistent throughout the modal component
After reviewing the entire modal component and similar patterns across other modal components, I can confirm that:
- The translation string format
{{displayMonth}}
is used consistently throughout the file - All notification messages in the component follow the same i18next convention
- The specific translation string for overwriting templates follows the same pattern as other similar notifications in the same component
- No template literals are used for notification messages that should be translated
π Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for potential single-brace translation patterns that might need updating
rg "t\(['\"].*\{[^{].*['\"]" --type ts
# Search for template literals in notification messages that could be converted to translations
rg "showUndoNotification.*message:\s+`.*`" --type ts
Length of output: 6412
Script:
#!/bin/bash
# Let's search more specifically for translation patterns in the modal component
rg -A 2 "t\(['\"].*\{\{.*\}\}.*['\"]" "packages/desktop-client/src/components/modals/TrackingBudgetMonthMenuModal.tsx"
# Also search for similar patterns in other modal components
rg -A 2 "t\(['\"].*\{\{.*\}\}.*['\"]" "packages/desktop-client/src/components/modals/"
# Check for any template literals in the same modal file
rg "`.*`" "packages/desktop-client/src/components/modals/TrackingBudgetMonthMenuModal.tsx"
Length of output: 8348
packages/desktop-client/src/components/modals/EnvelopeBudgetMonthMenuModal.tsx (3)
161-163
: LGTM! Correct translation string format.
The updated string interpolation syntax using double curly braces {{displayMonth}}
is the correct format for i18next translations.
171-173
: LGTM! Consistent translation format.
The string interpolation syntax is correctly updated and maintains consistency with other translation strings.
192-194
: LGTM! Translation format consistently applied.
All notification messages are correctly using the double curly braces syntax for variable interpolation, maintaining consistency throughout the file.
Also applies to: 202-204, 212-214
Some translation string formatting fixes that slipped through my review. π€¦ββοΈ