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

Missing internationalization #287

Closed
arbrandes opened this issue Feb 9, 2024 · 4 comments
Closed

Missing internationalization #287

arbrandes opened this issue Feb 9, 2024 · 4 comments
Labels
bug Report of or fix for something that isn't working as intended

Comments

@arbrandes
Copy link
Contributor

As per https://discuss.openedx.org/t/translations-issues-with-the-frontend-app-learner-dashboard/12287, @sambapete writes:

(...) not all string ids in the various messages.js files found in the repository are properly harvested, collected and sent to Transifex for translations.

If you look at the course card on the dashboard, you will see for example that the string “Begin Course” and other strings remain in English for all languages. You can easily check that by switching languages in edx.org or in your own Open edX instances running Quince for example.

That’s because the strings in src/containers/CourseCard/components/CourseCardActions/messages.js never made it to src/i18n/messages/fr_CA.json or any other files that would be used for translating various languages in the MFE. Check for the id learner-dash.courseCard.actions.beginCourse and you will not find it in the strings that the Translations Working Group needs to translate in Transifex.

@arbrandes arbrandes added the bug Report of or fix for something that isn't working as intended label Feb 9, 2024
@OmarIthawi
Copy link
Member

OmarIthawi commented Feb 10, 2024

Thanks @sambapete and @arbrandes for reporting this issue.

I've checked the file and it's a bug in the messages.js:

import { StrictDict } from 'utils';
export const messages = StrictDict({
upgrade: {
id: 'learner-dash.courseCard.actions.upgrade',
description: 'Course card upgrade button text',
defaultMessage: 'Upgrade',
},
beginCourse: {
id: 'learner-dash.courseCard.actions.beginCourse',
description: 'Course card begin-course button text',

It's using the StrictDict instead of the defineMessage as seen in other files:

import { defineMessages } from '@edx/frontend-platform/i18n';
const messages = defineMessages({
dashboard: {
id: 'learnerVariantDashboard.menu.dashboard.label',
defaultMessage: 'Dashboard',
description: 'The text for the user menu Dashboard navigation link.',
},
dashboardPersonal: {
id: 'learnerVariantDashboard.menu.dashboardPersonal.label',
defaultMessage: 'Personal',

To fix the issue, the CourseCard/components/CourseCardActions/messages.js should be refactored to use defineMessage and it'll make it to transifex shortly (the following day or so).

@sambapete
Copy link

sambapete commented Feb 10, 2024

This will fix the problem with CourseCardActions but not all problems.

The same issue exists in other messages.js files in frontend-app-learner-dashboard.

A full review may be needed.

Look at src/containers/SelectSessionModal/messages.js, ./src/containers/CourseCard/components/CourseCardMenu/messages.js and a few others that are also using StrictDict.

And it may be the case in other directories for that MFE and a few other MFEs.

I guess the Translations Working Group and the Frontend Working Group should make sure the translations of MFEs is properly setup for all MFEs. @arbrandes @OmarIthawi

As I am no longer a core contributor to the Open edX project, I don't have the time and I don't know how long it would take me to make a PR to fix all issues, have it reviewed and implemented to fix all the issues with translations in this particular MFE and maybe in other MFEs. Yes, I am passing the ball to the MFEs owners.

I don't know how the community could implement it but a test to make sure that all strings defined in a particular MFE are added to the strings that are sent to Transifex for translations, but I assume this is something that needs to be done now after what we found with frontend-app-communications previously and now with frontend-app-learner-dashboard. My 2 cents.

@ghassanmas
Copy link
Member

@sambapete I have made a quick serach across openedx org in github, and it seems this only used learner-dashbaord,
Search link: https://github.com/search?q=org%3Aopenedx+%22export+const+messages+%3D+StrictDict%28%22&type=code

For other MFEs I think the problem we had before that the resources werent synced to TX in the first place.

Regarding a test I agree it would be ideal, so far to my knowledge there are two reason a resource is not TX:( 1. The repo is not synced and 2. The sytanx used to define a message ).

We will discuss this topic in next TWG call CC:@ehuthmacher

ghassanmas added a commit to ghassanmas/frontend-app-learner-dashboard that referenced this issue Feb 16, 2024
@sambapete
Copy link

sambapete commented Feb 16, 2024

Thanks @ghassanmas

At least we know it is restricted to this MFE. Since the problem also affects edx.org, maybe it could be given a higher priority to resolve?

Unfortunately, I cannot attend the TWG meetings anymore as we are shutting down our Open edX instance on April 30, 2024. I am no longer a core contributor since we announced it. I found this problem while migrating our courses on edx.org.

yuopenedx added a commit to yuopenedx/frontend-app-learner-dashboard that referenced this issue Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Report of or fix for something that isn't working as intended
Projects
Status: Closed
Development

No branches or pull requests

4 participants