-
Notifications
You must be signed in to change notification settings - Fork 306
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
Standardize Translation Method #1354
Standardize Translation Method #1354
Conversation
Everything looks good to me. Please convert the draft to PR. |
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.
Thanks to @lawsj for another contribution. Overall, this works as desired. I made one comment in a file to address. In addition, here are comments that could not be placed in a file:
- src/client/app/utils/translate.ts has the previously used translation function. I could not find it used (which is good). If so, I think this file should be removed to avoid unused code and to stop someone from using the old translate method.
- OED is crashing when I go to a number of admin pages. For example, going to the conversion page and then try to create a conversion does this. What is strange is I cannot see why any change made in this PR would cause this to happen. Thus, I removed the new translate change in this file but it still happened. Next I tried changing to the development branch and it no longer crashed. This seems to imply it was a change in the pull request but why is unclear. Modest debugging did not find the reason. Given this, I wanted to know if you (or anyone else) sees this same type of failure? I want to know if it is a general problem or something in my setup.
This is just for the record: We are continuing to try to determine the cause of the issue. It may well be something outside the direct changes made in this PR. |
@huss , the code has been updated with the discussed changes. Can you please review this PR |
While two functions where in the components directory, they were not in the function component so the React Hooks version of translate should not be used. Before the change it was getting errors and crashing. now it seems fine.
I pushed commits to do the following:
|
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.
Thanks to @Rakesh-Ranga-Buram for the updated code. Review and testing found it works as desired at this time. Congratulations to @lawsj on their first accepted contribution to OED.
Description
Made changes to use the new "useTranslate()" instead of the old "translate()".
Partly Addresses #1204
Type of change
(Check the ones that apply by placing an "x" instead of the space in the [ ] so it becomes [x])
Checklist
(Note what you have done by placing an "x" instead of the space in the [ ] so it becomes [x]. It is hoped you do all of them.)
##Limitations
N/a