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

feat: Korean language support #7727

Merged
merged 1 commit into from
Jan 15, 2025
Merged

feat: Korean language support #7727

merged 1 commit into from
Jan 15, 2025

Conversation

lan-yonghui
Copy link
Member

Refs #7363

Copy link

f2c-ci-robot bot commented Jan 15, 2025

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

...message,
...xpackJpLocale,
...xpackJaLocale,
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no significant issues or optimization opportunities detected in this code snippet.

...message,
...xpackEnLocale,
...xpackMsLocale,
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided diff shows several changes in localization file lang/ms.ts between Fit2Cloud UI Plus version dated September 1st, 2029, and July 7th, 2034:

Changes:

  1. Changed Import Path

    • From:
      import fit2cloudEnLocale from 'fit2cloud-ui-plus/src/locale/lang/ms';
    • To:
      import fit2cloudMsLocale from 'fit2cloud-ui-plus/src/locale/lang/ms';
  2. Updated Locale Variables Names:

    • Changed xpackEnLocale to xpackMsLocale.
    • Updated keys in the message object to reflect MS locale (for example, translated labels like "fullscreen" vs. capitalized "fullscreen").

Suggestions and Potential Issues:

  1. Consistency of File Name: Ensure that the imported component names match exactly across different versions if they refer to files with similar names but potentially different contents.

  2. Capitalization Consistency: In some places, consistent capitalization is applied (e.g., "Fullscreen", "Quit") which enhances readability.

  3. Localization Coverage: Verify that all key translations are complete and correct, especially critical messages for usability and end-user experience.

Optimization Suggestion:

Although not immediately relevant given this change alone, consider using ES6 imports when necessary, such as replacing import.meta.glob() with static imports where appropriate, especially as more modules become available dynamically.

This change set appears to be focused on adjusting specific languages and maintaining consistency within the UI component locales, ensuring compatibility with future updates or regional requirements without conflicts or inconsistencies.

...message,
...xpackEnLocale,
...xpackRuLocale,
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code differences have some potential issues and can be optimized to improve readability and maintainability:

  1. Variable Names: The variable names xpackEnLocale and yexpackRuLocale should be consistent with their purpose (en refers to English, not Yandex). Update them to:

    let fit2cloudXpackEnLocale = {};
    let fit2cloudXpackRuLocale = {};
  2. Error Handling: Ensure that import.meta.glob resolves successfully without errors. If the file doesn't exist, it should handle this case gracefully.

  3. Simplified Code Structure: Remove unnecessary comments and simplify the structure of the imports if possible.

  4. Imported Data: Since you're importing all .ts files within a specific directory using glob, ensure that the paths are correct and there aren't multiple identical modules being imported. Use destructuring assignment to extract data more efficiently.

Here is an improved version of the code based on these considerations:

// Importing the necessary locales
const { default: fit2cloudRusLocale } = await import('fit2cloud-ui-plus/src/locale/lang/ru');
let fit2cloudXpackRusLocale = {};

try {
    const yextPackModules = import.meta.glob('../../xpack/lang/en.ts', { eager: true });

    // Populate Russian localization
    if (yextPackModules['../../xpack/lang/en.ts']) {
        fit2cloudXpackRusLocale = yextPackModules['../../xpack/lang/en.ts']['default'] || {};
    }
} catch (error) {
    console.error("Failed to load X-Pack locale:", error);
}

export default {
    // Merging the locales
    ...fit2cloudRusLocale,
    ...(fit2cloudXpackRusLocale || {}),
};

This version ensures that only one attempt to import the locale module is made per language context, reducing overhead while still ensuring robustness against missing resources.

Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@wanghe-fit2cloud
Copy link
Member

/approve

Copy link

f2c-ci-robot bot commented Jan 15, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wanghe-fit2cloud

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@f2c-ci-robot f2c-ci-robot bot merged commit 9052386 into dev Jan 15, 2025
6 checks passed
@f2c-ci-robot f2c-ci-robot bot deleted the pr@dev@feat_i18n_ko branch January 15, 2025 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants