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

fix: avoid overriding of translations data from previous state #901

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

ullasgowda
Copy link
Contributor

@ullasgowda ullasgowda commented Jan 4, 2024

Avoid overriding previous state translations data over new translations.

File

frontend/src/redux/translate/reducer.js

Please provide a brief description of the changes or additions made in this pull request.

return { result: { ...state.result, ...payload }, langCode: langCode.toLowerCase(), langDirection: isRtl ? 'rtl' : 'ltr', isLoading: false }

This would make the translations data in the result dirty.

Unless if there was a specific use case, due to which this is written like this { ...state.result, ...payload }, we should avoid destructuring of new translations over previous state translations.

Because, when there are keys that are present in Hindi translation but are not present in English translation file, it would retain such keys and its values when we change locale from Hindi to English due to this destructuring { ...state.result, ...payload }

Related Issues

If this pull request is related to any issue(s), please list them here.
#900

Steps to Test

  • Navigate to /admin
  • Check selected locale
  • Click on Add new Admin, and check strings for Role dropdown

image

  • Change locale to Hindi, and then again click on Add new Admin and verify Role dropdown

image

  • Now, go back and change locale to English and check the Role dropdown again
  • Ideally all the strings should have translated back to English. But some strings are still in Hindi

image

This is basically because, there are some strings missing in then en_us translation file, due to which the logic { ...state.result, ...payload } retained previous states translations.

Although it is suggested to maintain translations data similar across locale, it is also a better practice to avoid merging previous translation object and new translation object.

Provide steps on how to test the changes introduced in this pull request.

  • Follow the same steps above.

Screenshots (if applicable)

If your changes include visual updates, it would be helpful to provide screenshots of the before and after.

Checklist

  • I have tested these changes
  • [] I have updated the relevant documentation
  • [] I have commented my code, particularly in hard-to-understand areas
  • [] I have made corresponding changes to the codebase
  • My changes generate no new warnings or errors
  • The title of my pull request is clear and descriptive

@salahlalami salahlalami merged commit c2b7032 into idurar:dev Jan 8, 2024
1 check passed
@ullasgowda ullasgowda deleted the fix/translation-state branch January 8, 2024 15:42
awesomedev82 pushed a commit to awesomedev82/idurar-erp-crm that referenced this pull request May 20, 2024
fix: avoid overriding of translations data from previous state
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants