source-academy / frontend

Frontend of Source Academy, an online experiential environment for computational thinking (React, Redux, Saga, Blueprint)
https://sourceacademy.org
Apache License 2.0
103 stars 168 forks source link

i18n framework #2946

Closed cheehongw closed 5 months ago

cheehongw commented 5 months ago

Description

Partially addresses #2426, #2468

This PR adds an i18n framework that we can work with.

Things to note

Some translations were done for NavigationBar and Dropdown and Login to demonstrate how the strings might be internationalized over time and some of the potential problems we might encounter.

  1. Structure for translation files

    /locales
      /en
        commons.json     //for all the components in commons
        academy.json
        achievement.json
        ...        //one json for each page
  2. Applying translation at the correct level

    In Dropdown, the useTranslation hook and the translate function was applied directly to the strings since these are located within the component itself.

    const Dropdown: React.FC = () => {
      ...
      const { t } = useTranslation('translation', { keyPrefix: 'dropdown' })
      ...
      const myCourses = isLoggedIn ? (
        <MenuItem icon={IconNames.PROPERTIES} onClick={toggleMyCoursesOpen} text={ t('My Courses') } />
      ) : null;

    However, the strings in NavigationBar are not located in the component. For example, some of the strings are found hardcoded as part of the many list entries in AcademyNavigationBar.tsx > getStaffNavLinkInfo. Furthermore, these entries are then passed into filterNotificationsByType, where a string comparison is performed between the entry's string and a hardcoded string. If we applied translation directly to the entry's string, a translated string would always fail this comparison, causing some logic to break. Therefore, translation is only applied at the rendering level.

    <div className={classNames(navbarEntry.hiddenInBreakpoints?.map(bp => `hidden-${bp}`))}>
      <Translation keyPrefix={"navigationBar"}>
        {(t) => t(navbarEntry.text), { defaultValue: navbarEntry.text } 
      </Translation>
    </div>
      export function filterNotificationsByType(
        assessmentType: AssessmentType
      ): NotificationFilterFunction {
        return (notifications: Notification[]) =>
          notifications.filter(n => {
            if (assessmentType === 'Grading') {
              return n.submission_id !== undefined;
            }
            return !n.submission_id && assessmentType === n.assessment_type;
          });
      }

https://github.com/source-academy/frontend/assets/72195240/847f3196-4bb6-4df0-a22f-3e11008d2734

Type of change

Checklist

coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 8755851977

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
src/commons/navigationBar/NavigationBar.tsx 0 2 0.0%
src/commons/dropdown/LocaleSelector.tsx 2 8 25.0%
<!-- Total: 11 19 57.89% -->
Totals Coverage Status
Change from base Build 8738989806: 0.02%
Covered Lines: 5288
Relevant Lines: 14861

💛 - Coveralls
cheehongw commented 5 months ago

thanks for the review @RichDom2185

we plan to bring more than just basic UI strings in the long run (see https://github.com/source-academy/frontend/issues/1943 for an example). Or even the sublanguage intro tab (with dynamic links):

i18next supports complex UI strings with the Trans component . The next step would definitely be identifying and externalizing all the English strings in the repo to follow i18n.

Actual translation can take place alongside this process.

design of using separate namespaces for different parts

Originally, i wanted to have a json file for each component containing strings, however, combining them into a nested structure is tricky and each separate json file is considered a namespace by i18next. Furthermore, the location of a string and where the translation is applied might be different, so there is less meaning to having one json file to each component.

Currently, the idea i was working towards is to have the namespace follow the structure of how components are nested in their directory, for example externalising the strings in a deeply nested component like commons/achievement may look like this, but we should also consider squashing some levels to prevent over-nesting

//commons.json
{
   "achievement" : {
      "card": { ... },
      "control": { 
         "achievementEditor" : { 
              "achievementSettings": { ... },
              "key1" : "string1",
              "key2" : "string2",
           }
         ...
        }
    }
   "dropdown" : { ... },
   "navigationBar": { ... }
}

I think namespacing will become clearer as more strings get externalized. My only concern is that the "commons" namespace will contain majority of the translated strings since most components are located under the commons directory, which might make it difficult for a translator to track down a string belonging to which component.