Skip to content

feat: Some adjustments to internationalization #7332

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.

Sign up for GitHub

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

Merged
f2c-ci-robot merged 1 commit into from
Dec 11, 2024
Merged

feat: Some adjustments to internationalization #7332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion frontend/src/layout/components/Sidebar/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ function getCheckedLabels(json: Node): string[] {

const search = async () => {
await checkIsSystemIntl();
let checkedLabels: string | any[];
let checkedLabels: any[] = [];
if (!globalStore.isIntl) {
const res = await getSettingInfo();
const json: Node = JSON.parse(res.data.xpackHideMenu);
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Dec 11, 2024

Choose a reason for hiding this comment

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

The code change seems mostly correct, but here's a quick review:

  1. Type of checkedLabels: The line let checkedLabels: string | any[]; specifies that checkedLabels can be either a string or an array of any type (any[]). Ensure that this is intentional as it might lead to runtime errors if you expect checkedLabels to always be a string when used.

  2. Potential Issue with globalStore.isIntl: Before assigning the value from getSettingInfo, ensure there isn't a race condition where globalStore.isIntl hasn't been initialized yet (e.g., using asynchronous operations outside this block).

  3. Suggested Change: Consider initializing checkedLabels within the same scope as its definition without specifying types first, especially if types are not strictly necessary for all use cases (although they can help improve readability):

    const search = async () => {
    await checkIsSystemIntl();
    let checkedLabels;

    if (!globalStore.isIntl) {
        const res = await getSettingInfo();
        globalStore.setLocale(res.data.xpackHideMenu); // Assuming setLocale updates isCheckedLabels based on xpackHideMenu data

        checkedLabels = globalStore.isCheckedLabels || [];
    } else {
        checkedLabels = []; // Alternatively set labels based on intl system settings or default values
    }

    doSomethingWith(checkedLabels);
};

This approach assumes setCheckedLabels method exists in globalStore.

  1. If isCheckedLabels doesn’t exist, consider setting its initial value in init function before calling search. This would prevent undefined behavior:

    import { init } from "./store";

init() {
    globalStore.isCheckedLabels = []; // Set initial state if needed
}

async search() {
    await ...
}

These optimizations focus on clarity and ensuring the flow is smooth without introducing unnecessary complexity or runtime error risks.

Expand Down
6 changes: 6 additions & 0 deletions frontend/src/routers/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ router.beforeEach((to, from, next) => {
axiosCanceler.removeAllPending();
const globalStore = GlobalStore();

if (globalStore.isIntl && to.path.includes('xpack')) {
next({ name: '404' });
NProgress.done();
return;
}

if (to.name !== 'entrance' && !globalStore.isLogin) {
next({
name: 'entrance',
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Dec 11, 2024

Choose a reason for hiding this comment

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

There are no apparent errors or issues with this code. However, there is an opportunity for further optimization:

  1. Code DRYing: The next({ name: '404' }); line appears twice in the same branch. Consider extracting it into a separate function call to make the code cleaner.

Here's how you can optimize it:

@@ -10,6 +10,9 @@ router.beforeEach((to, from, next) => {
   axiosCanceler.removeAllPending();
   const globalStore = GlobalStore();

+  handleUnauthorizedRoute(next);
+
   if (!globalStore.isLogin && to.name !== 'entrance') {
       next({
           name: 'entrance',

Create this helper function:

function handleUnauthorizedRoute(next) {

    nprogress.done()

    if(to.path.includes('xpack')){
        return 
    }

    console.log("Unauthorized path")
    
    // You may want to log more specific information here, like the current route
}

This way, the duplicate code block is removed and organized under its own named function, improving readability and maintainability.

Expand Down
3 changes: 2 additions & 1 deletion frontend/src/views/setting/panel/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,10 @@
@change="onSave('Language', form.language)"
v-model="form.language"
>
<el-radio v-if="globalStore.isIntl" value="en">English</el-radio>
<el-radio value="zh">中文(简体)</el-radio>
<el-radio value="tw">中文(繁體)</el-radio>
<el-radio value="en">English</el-radio>
<el-radio v-if="!globalStore.isIntl" value="en">English</el-radio>
</el-radio-group>
</el-form-item>

Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Dec 11, 2024

Choose a reason for hiding this comment

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

The provided code snippet appears to have duplication of "English" radio options. It should be cleaned up to avoid this repetition:

@@ -77,13 +77,12 @@
                                     @change="onSave('Language', form.language)"
                                     v-model="form.language"
                                 >
-                                    <el-radio value="en">English</el-radio>
+                                    <!-- Remove duplicate English radio option -->
                                 </el-radio-group>
                             </el-form-item>

Additionally, you may want to consider using conditional rendering within the el-radio components based on some logic rather than hardcoding them like this. For example, if globalStore.isIntl determines whether certain fields need certain visibility or not, it might make more sense to use conditional statements instead of repeating the same HTML twice with different values.

If there's additional context about what these changes will achieve (like improving accessibility by hiding one of the radios), feel free to include that information for further clarification.

Expand Down
Loading