-
Notifications
You must be signed in to change notification settings - Fork 0
fix: missing i18n translation for Trans #42
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
base: greptile_combined_100_qodo_grep_sentry_1_base_fix_missing_i18n_translation_for_trans_pr137
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -34,10 +34,11 @@ const EmptyElement: FC<{ appDetail: App }> = ({ appDetail }) => { | |||||
| </span> | ||||||
| <div className="system-sm-regular mt-2 text-text-tertiary"> | ||||||
| <Trans | ||||||
| i18nKey="appLog.table.empty.element.content" | ||||||
| i18nKey="table.empty.element.content" | ||||||
| ns="appLog" | ||||||
| components={{ | ||||||
| shareLink: <Link href={`${appDetail.site.app_base_url}${basePath}/${getWebAppType(appDetail.mode)}/${appDetail.site.access_token}`} className="text-util-colors-blue-blue-600" target="_blank" rel="noopener noreferrer" />, | ||||||
| testLink: <Link href={getRedirectionPath(true, appDetail)} className="text-util-colors-blue-blue-600" />, | ||||||
| testLink: <Link href={getRedirectionPath(false, appDetail)} className="text-util-colors-blue-blue-600" />, | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the boolean parameter passed to Looking at the diff, the original code was: testLink: <Link href={getRedirectionPath(true, appDetail)} ...And it was changed to: testLink: <Link href={getRedirectionPath(false, appDetail)} ...Without seeing the implementation of This change needs verification - if this was intentional, it should be documented. If unintentional, it should be reverted:
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: web/app/components/app/log/empty-element.tsx
Line: 41:41
Comment:
the boolean parameter passed to `getRedirectionPath` was changed from `true` to `false`. This change inverts the logic and will cause users to be redirected to a different location.
Looking at the diff, the original code was:
```typescript
testLink: <Link href={getRedirectionPath(true, appDetail)} ...
```
And it was changed to:
```typescript
testLink: <Link href={getRedirectionPath(false, appDetail)} ...
```
Without seeing the implementation of `getRedirectionPath`, this appears to be an unintended logic change that was bundled with the i18n translation fixes. The first parameter likely controls some aspect of the redirection behavior (possibly whether to redirect to the test/preview page vs the production page).
This change needs verification - if this was intentional, it should be documented. If unintentional, it should be reverted:
```suggestion
testLink: <Link href={getRedirectionPath(true, appDetail)} className="text-util-colors-blue-blue-600" />,
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||
| }} | ||||||
| /> | ||||||
| </div> | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -92,8 +92,8 @@ const TransferOwnershipModal = ({ onClose, show }: Props) => { | |||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| const sendCodeToOriginEmail = async () => { | ||||||||||||||||||
| await sendEmail() | ||||||||||||||||||
| setStep(STEP.verify) | ||||||||||||||||||
| await sendEmail() | ||||||||||||||||||
|
Comment on lines
94
to
+96
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the order of operations has been changed, causing the UI step to be updated BEFORE the email is sent. This can lead to a poor user experience if the email sending fails. Original code: await sendEmail()
setStep(STEP.verify)Changed to: setStep(STEP.verify)
await sendEmail()Problem: If The original order was correct - it ensured the email was successfully sent before showing the verification UI. The fix should revert this change:
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: web/app/components/header/account-setting/members-page/transfer-ownership-modal/index.tsx
Line: 94:96
Comment:
the order of operations has been changed, causing the UI step to be updated BEFORE the email is sent. This can lead to a poor user experience if the email sending fails.
Original code:
```typescript
await sendEmail()
setStep(STEP.verify)
```
Changed to:
```typescript
setStep(STEP.verify)
await sendEmail()
```
**Problem**: If `sendEmail()` throws an error or fails, the user will already see the verification screen (STEP.verify), but no email was actually sent. This creates a confusing user experience.
The original order was correct - it ensured the email was successfully sent before showing the verification UI. The fix should revert this change:
```suggestion
const sendCodeToOriginEmail = async () => {
await sendEmail()
setStep(STEP.verify)
}
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| const handleVerifyOriginEmail = async () => { | ||||||||||||||||||
|
|
@@ -140,7 +140,8 @@ const TransferOwnershipModal = ({ onClose, show }: Props) => { | |||||||||||||||||
| <div className="body-md-regular text-text-secondary">{t('members.transferModal.warningTip', { ns: 'common' })}</div> | ||||||||||||||||||
| <div className="body-md-regular text-text-secondary"> | ||||||||||||||||||
| <Trans | ||||||||||||||||||
| i18nKey="common.members.transferModal.sendTip" | ||||||||||||||||||
| i18nKey="members.transferModal.sendTip" | ||||||||||||||||||
| ns="common" | ||||||||||||||||||
| components={{ email: <span className="body-md-medium text-text-primary"></span> }} | ||||||||||||||||||
| values={{ email: userProfile.email }} | ||||||||||||||||||
| /> | ||||||||||||||||||
|
|
@@ -170,7 +171,8 @@ const TransferOwnershipModal = ({ onClose, show }: Props) => { | |||||||||||||||||
| <div className="pb-2 pt-1"> | ||||||||||||||||||
| <div className="body-md-regular text-text-secondary"> | ||||||||||||||||||
| <Trans | ||||||||||||||||||
| i18nKey="common.members.transferModal.verifyContent" | ||||||||||||||||||
| i18nKey="members.transferModal.verifyContent" | ||||||||||||||||||
| ns="common" | ||||||||||||||||||
| components={{ email: <span className="body-md-medium text-text-primary"></span> }} | ||||||||||||||||||
| values={{ email: userProfile.email }} | ||||||||||||||||||
| /> | ||||||||||||||||||
|
|
||||||||||||||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -21,7 +21,7 @@ | |||||
| "resolveJsonModule": true, | ||||||
| "types": ["vitest/globals", "node"], | ||||||
| "allowJs": true, | ||||||
| "strict": true, | ||||||
| "strict": false, | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. disabling TypeScript's strict mode removes important type safety guarantees and is a significant regression. Strict mode includes critical checks like:
This change appears unrelated to the stated PR goal of "fixing missing i18n translation for Trans". Disabling strict mode to make code compile is not a proper fix - it masks underlying type issues that should be resolved. Impact: This will allow type errors to slip through that could cause runtime bugs, especially with the new i18next v25 and react-i18next v16 which may have different type signatures. Recommendation: Keep strict mode enabled and fix any type errors properly:
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: web/tsconfig.json
Line: 24:24
Comment:
disabling TypeScript's strict mode removes important type safety guarantees and is a significant regression. Strict mode includes critical checks like:
- `strictNullChecks`: prevents null/undefined errors
- `strictFunctionTypes`: ensures function parameter type safety
- `noImplicitAny`: catches missing type annotations
- `strictBindCallApply`: ensures correct function binding
This change appears unrelated to the stated PR goal of "fixing missing i18n translation for Trans". Disabling strict mode to make code compile is not a proper fix - it masks underlying type issues that should be resolved.
**Impact**: This will allow type errors to slip through that could cause runtime bugs, especially with the new i18next v25 and react-i18next v16 which may have different type signatures.
**Recommendation**: Keep strict mode enabled and fix any type errors properly:
```suggestion
"strict": true,
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||
| "noEmit": true, | ||||||
| "esModuleInterop": true, | ||||||
| "forceConsistentCasingInFileNames": true, | ||||||
|
|
||||||
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.
the
sendEmailfunction is being called with the wrong email address. WhenisOriginisfalse(indicating this is for the new email), the function should receive the new email address (mail), not the old email address (email).Looking at the function signature on line 61:
And the call on line 107-110 for the origin email:
But on line 163-167, for the NEW email step:
This will send the verification code to the OLD email address instead of the NEW email address the user just entered. The correct code should be:
Prompt To Fix With AI