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

Settings - Error message is not translated dynamically to Spanish #23008

Closed
2 of 6 tasks
kbecciv opened this issue Jul 17, 2023 · 7 comments
Closed
2 of 6 tasks

Settings - Error message is not translated dynamically to Spanish #23008

kbecciv opened this issue Jul 17, 2023 · 7 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2

Comments

@kbecciv
Copy link

kbecciv commented Jul 17, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

  1. Open the app and login with user A (device 1)
  2. Open the app and login with user A (device 2)
  3. From device 1, Change your image and select unsupported type like .ico to get the error
  4. From device 2, Change language to Spanish

Expected Result:

Error message should be translated dynamically to Spanish

Actual Result:

Error message is not translated dynamically to Spanish

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.3.40-5
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

24.New.Expensify.-.16.July.2023.1.mp4
Screen_Recording_20230717_111243_Chrome.mp4
Recording.3697.mp4

Expensify/Expensify Issue URL:
Issue reported by: @mejed-alkoutaini
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1689465033493809

View all open jobs on GitHub

@ahmedGaber93
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

Error message is not translated dynamically to Spanish

What is the root cause of that problem?

all errors text is translated once and saved to state, so when language is changed the state not translate again.

showAvatarCropModal(image) {
if (!this.isValidExtension(image)) {
this.showErrorModal(
this.props.translate('avatarWithImagePicker.imageUploadFailed'),
this.props.translate('avatarWithImagePicker.notAllowedExtension', {allowedExtensions: CONST.AVATAR_ALLOWED_EXTENSIONS}),

What changes do you think we should make in order to solve the problem?

we should save translationKey in all the state instead of translated value

// state
this.state = {
    errorModalPromptTranslationKey: '',
    errorModalTitleTranslationKey: '',
};


showErrorModal(titleTranslationKey, promptTranslationKey) {
    this.setState({isErrorModalVisible: true, errorModalTitleTranslationKey: titleTranslationKey, errorModalPromptTranslationKey: promptTranslationKey});
}

this.showErrorModal(
   // this.props.translate('avatarWithImagePicker.imageUploadFailed')
   'avatarWithImagePicker.imageUploadFailed',
   ...
);


<ConfirmModal
    title={Boolean(this.state.errorModalTitleTranslationKey) ? props.translate(this.state.errorModalTitleTranslationKey) : ''}
    prompt={Boolean(this.state.errorModalPromptTranslationKey) ? props.translate(this.state.errorModalPromptTranslationKey) : ''}

We need to do the same for all error type in this component

What alternative solutions did you explore? (Optional)

we can search for similar cases like this and also fix it.

@kbecciv kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 17, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 17, 2023

Triggered auto assignment to @kevinksullivan (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Jul 17, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@c3024
Copy link
Contributor

c3024 commented Jul 17, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

When we upload an unsupported file in profile picture an error modal appears. But its title and prompt do not change on changing language from another device

What is the root cause of that problem?

Root cause of the problem is in the AvatarWithImagePicker.js

showAvatarCropModal(image) {
        if (!this.isValidExtension(image)) {
            this.showErrorModal(
                this.props.translate('avatarWithImagePicker.imageUploadFailed'),
                this.props.translate('avatarWithImagePicker.notAllowedExtension', {allowedExtensions: CONST.AVATAR_ALLOWED_EXTENSIONS}),
            );

Here we are translating it in the functions like this. So these do not get translated dynamically when language is changed from another device.

What changes do you think we should make in order to solve the problem?

We can observe that the word close is translated properly.
So, we follow similar solution for these as well.
We set the errors to the keys of the message instead of translating there in the above showAvatarCropModal function and other functions.
Also we will add another state variable called say reason and pass it to showErrorModal.
So showErrorModal function changes to

showErrorModal(title, prompt, reason) {
        this.setState({isErrorModalVisible: true, errorModalTitle: title, errorModalPrompt: prompt, reason});
    }

and
the above function in RCA can be changed to

showAvatarCropModal(image) {
        if (!this.isValidExtension(image)) {
            this.showErrorModal(
                'avatarWithImagePicker.imageUploadFailed',
                'avatarWithImagePicker.notAllowedExtension',
                {allowedExtensions: CONST.AVATAR_ALLOWED_EXTENSIONS}
            );

and translate it in the render statement like this

<ConfirmModal
                    title={this.state.errorModalTitle ? this.props.translate(this.state.errorModalTitle, this.state.reason) : ‘’}
                    onConfirm={this.hideErrorModal}
                    onCancel={this.hideErrorModal}
                    isVisible={this.state.isErrorModalVisible}
                    prompt={this.state.errorModalTitle ? this.props.translate(this.state.errorModalPrompt, this.state.reason) : ‘’}
                    confirmText={this.props.translate('common.close')}
                    shouldShowCancelButton={false}
                />

We change similarly at other showErrorModal uses in the file.

Further there are other files like AttachmentModal as well where errors are not translated dynamically and there also this is the root cause. All files can be identified and they can be fixed together.

What alternative solutions did you explore? (Optional)

@hungvu193
Copy link
Contributor

hungvu193 commented Jul 17, 2023

Dupe of #22665.
Similar #22665
Please reopen if this is considered as a bug

@adelekennedy
Copy link

@kevinksullivan would love your thoughts on this - I think this issue and this are pretty edge case behavior (changing your language in the middle of taking an action, I don't see a lot of users doing that) but if it's the same root cause I think we should keep one issue open and fix

@kevinksullivan
Copy link
Contributor

Let's keep your issue if it was opened first @adelekennedy . FWIW many of our bugs are edge cases, but I'd agree that this is not the behavior I'd expect so I'd consider it a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2
Projects
None yet
Development

No branches or pull requests

6 participants