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

standardize translation method #1204

Open
huss opened this issue Apr 4, 2024 · 7 comments
Open

standardize translation method #1204

huss opened this issue Apr 4, 2024 · 7 comments
Labels
i-good-first-issue This issue is probably a good starting point for people new to coding or the OED project. p-low-priority reserved Expected to be assigned to developer by project so others should not work on without prior approval. t-enhancement This issues tracks a potential improvement to the software
Milestone

Comments

@huss
Copy link
Member

huss commented Apr 4, 2024

Is your feature request related to a problem? Please describe.

In many places OED uses translate() and in some it uses FormatMessage(). They both appear to return a string.

Describe the solution you'd like

OED should pick the better of these two and use it consistently in the code. Maybe research the two and propose one to the project. Then make the needed edits once everyone agrees.

Describe alternatives you've considered

Leave as is

Additional context

None

@huss huss added t-enhancement This issues tracks a potential improvement to the software p-low-priority i-good-first-issue This issue is probably a good starting point for people new to coding or the OED project. labels Apr 4, 2024
@huss huss added this to the 1.x milestone Apr 4, 2024
@huss
Copy link
Member Author

huss commented Sep 9, 2024

This seems to relate to issue #1189 and can probably be done together.

@huss huss added the reserved Expected to be assigned to developer by project so others should not work on without prior approval. label Sep 9, 2024
@Rakesh-Ranga-Buram Rakesh-Ranga-Buram moved this to Todo in SLU Sep 21, 2024
@lawsj
Copy link

lawsj commented Sep 23, 2024

Please assign this issue to me.

@lawsj
Copy link

lawsj commented Sep 30, 2024

I found 15 instances of formatMessage() across the files.
I found 66 instances of translate() across the files.

General uses:

Both in react-intl library

translate():
Convert strings from one language to another, using a file/directory as a key.
ex) translate(“greeting”) returns “hello” for english or “hola” for Spanish

formatMessage():
Formatting strings from variables
Good for displaying messages that use variables, such as:error messages, user notifications, etc.
https://formatjs.io/docs/react-intl/api/
ex) FormatMessage({id: ‘welcome’, defaultMessage: ‘Welcome, {name}!’}, {name: ‘Alice’}) returns “Welcome, Alice!”

https://react.i18next.com/latest/usetranslation-hook

Experimenting:
The changes from translate() to useTranslate() did not show errors, but all of the changes from formatMessage() to useTranslate() showed errors. These were the errors encountered when trying to implement this change:

  1. Only allow translate() instead of useTranslate()
  2. Intl would have to be removed from each instance, since the library is not used to call the useTranslate() function.

I think that it may be better to use the formatMessage() across the program if we want to continue to use the react-intl library. Translate() seems simpler, but it does not seem to be able to handle as many variables like the formatMessage() can. I would even propose to use translate() in some areas and use FormatMessage() strictly for error handling, which is how the instances seem to be used currently.

@nneni
Copy link
Contributor

nneni commented Sep 30, 2024

I like how thorough your analysis of the two functions was, it was very enlightening and helped me form a better, more factual opinion of the efficiency of the use of either function. I agree with your statement at the end as well, the functions seem to each work better in different situations.

@huss
Copy link
Member Author

huss commented Oct 1, 2024

@lawsj Thanks for looking into this and your analysis. Would it be possible to meet to discuss your two options? (You can DM/email me to find a time.) I'd like to understand the tradeoff a little better so a final decision can be made. If not, I can ask questions here.

@RaiuXL
Copy link

RaiuXL commented Dec 3, 2024

I would like to take on this issue.

@huss
Copy link
Member Author

huss commented Dec 3, 2024

@RaiuXL Please note that PR #1354 made significant progress on this issue. At this point the item left is to get it two work outside components. There has been a little discussion on how that might be done. If you still want to work on this then please indicate that and it might be good to discuss how to move forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i-good-first-issue This issue is probably a good starting point for people new to coding or the OED project. p-low-priority reserved Expected to be assigned to developer by project so others should not work on without prior approval. t-enhancement This issues tracks a potential improvement to the software
Projects
Status: In Progress
Development

No branches or pull requests

4 participants