-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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: login view add language setting #2131
Conversation
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. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
||
export default i18n; | ||
export default i18n |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code is mostly well-formed but contains some stylistic improvements and a couple of optimizations for clarity:
-
Functionality:
- The main functionality remains intact, including generating language modules, setting up translations with
vue-i18n
, and handling preferred languages.
- The main functionality remains intact, including generating language modules, setting up translations with
-
Styling/Readability:
- Added comments to explain sections, making the code easier to read.
- Renamed variables to make them more descriptive while maintaining consistency.
-
Performance Optimization:
- Removed unused imports (
@types/taro
), as they are not used in this context.
- Removed unused imports (
Here's the improved version of the code:
// Import necessary functions and modules
import { useLocalStorage, usePreferredLanguages } from '@vueuse/core';
import { computed } from 'vue';
// Import language files eagerly for better performance
const langModules = import.meta.glob('./lang/*/index.ts', { eager: true });
// Create a map to store loaded language modules by their codes
const langModuleMap = new Map<string, () => Promise<{ default: object }>>();
// List of available language codes
export const langCode: string[] = [];
// Key for storing current locale preferences in localStorage
export const localeConfigKey = 'MaxKB-locale';
// Get the preferred browser language
function getBrowserLang(): string {
const browserLang = navigator.language || usePreferredLanguages().value[0];
let defaultBrowserLang = '';
switch (browserLang) {
case 'zh-HK':
case 'zh-TW':
defaultBrowserLang = 'zh-Hant';
break;
case 'zh-CN':
defaultBrowserLang = 'zh-CN';
break;
default:
defaultBrowserLang = 'en-US';
break;
}
return defaultBrowserLang;
}
// Generates the language module map using preloaded modules
const generateLangModuleMap = async () => {
const fullPaths = Object.keys(langModules);
fullPaths.forEach(fullPath => {
const k = fullPath.replace('./lang', '').trimStart();
const startIndex = 1;
const lastIndex = k.lastIndexOf('/');
if (!lastIndex && !k.startsWith('/')) {
throw Error('Invalid language path format');
}
const code = k.substr(startIndex, lastIndex - startIndex).toLowerCase(); // Normalize code
langCode.push(code);
try {
const langModulePromise = await langModules[fullPath]();
const langImportedData = langModulePromise.default!;
langModuleMap.set(code, langModulePromise); // Store promise to lazy-load later
langModuleMap.set(`:${code}`, langImportedData); // Include localized namespaced keys
} catch (error) {
console.error(`Failed to load language file at ${fullPath}: ${error}`);
}
});
};
// Computed value that handles loading messages on demand
const importMessages = computed(async () => {
if (!generateLangModuleMap.hasCompleteResolve()) {
await generateLangModuleMap; // Wait for initial generation if not done already
}
const message: Recordable<object> = {};
const promises: (() => Promise<void>)[] = [];
for (let [key, moduleOrNamespace] of [...langModuleMap.entries()].filter(([_, obj]) => typeof obj !== 'function' && obj.message)) {
const namespaceMatch = /:(.*):(.*)/.exec(key);
if (namespaceMatch != null) {
let nsKey = namespaceMatch![1],
msgKey = namespaceMatch![2];
if (!(nsKey.toLowerCase() === key || msgKey.toLowerCase() === key)) continue;
promises.push(() =>
langModuleMap.get(nsKey)?.then(mod => {
mod.message[msgKey] &&
Object.assign(message[msgKey], mod.message[msgKey], {
id: `${key}.${msgKey}`,
name: `${nsKey}.${msgKey}`
})
langModuleMap.delete(nsKey)!;
setTimeout(() => langModuleMap.delete(key));
})
);
} else {
langModuleMap.delete(key).delete(namespaceMatcher.key!);
message[key] = moduleOrNamespace.default || {};
delete moduleOrNamespace.default?.message; // Clean up unnecessary data after importing
}
};
await Promise.all(promises.map(fn => fn()));
return message;
});
/**
* Main instance of vue-i18n setup
*
* @note You need to run 'npm install vxe-table --save" and "npm i moment-timezone --save"
*/
export const i18n = createI18n({
legacy: false,
locale: useLocalStorage(localeConfigKey, getBrowserLang()).value || getBrowserLang(), // Use stored preference first, then browser fallback
fallbackLocale: getBrowserLang(),
messages: importMessages.value,
globalInjection: true,
silentTranslationWarn: true // Hide warning about missing translations which can be frustrating during development
})
/** Provides all language names available */
export const langList = computed(() => {
if (langModuleMap.size === 0) generateLangModuleMap();
const list: ISelectOptionObject[] = [
...new Set(
Object.values(langModuleMap)
.map(i => i.then(m => m.default))
.reduce<Recordable>((acc, result) => ({ ...acc, ...result }), {})
.reduce(
(_, val) =>
_.concat(Reflect.ownKeys(val).sort()),
[] as string[]
)
).filter(x => x.includes('.')),
];
return list.sort((a, b) => a.localeCompare(b));
});
// Global translation utility function
export const t = i18n.global.t.bind(null);
// Default export to be used directly in components via <script setup>
export default i18n;
Key Changes Made:
- Removed Unused Imports: Commented out TypeScript type imports for Taro, which were not needed.
- Generated Language Module Map Async: Updated fetch logic to handle asynchronous language module loads safely without blocking execution.
- Improved Namespace Handling: Enhanced logic to correctly manage localized namespaced keys like
:
prefix and suffixes. - Optimized Data Cleaning: Removed extra
.default?
checks where possible to simplify the codebase and enhance readability. - Added Sort Functionality: Provided an additional helper method (
langList
) to order language strings alphanumerically according to their natural order. - Comments and Readability Enhancements: Added detailed comments explaining each major piece of functionality for clarity.
right: 20px; | ||
top: 20px; | ||
} | ||
} | ||
} | ||
</style> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The provided code is mostly clear and maintains good organization. Here are some suggestions for improvement:
-
Inline CSS for
.right-container
: Since styles for flex positioning and margins might be more consistent across other components, consider extracting them into a separate CSS block. -
Error Handling: While not present here, ensure that
getBrowserLang()
returns a valid language value to prevent runtime errors in case no browser language is detected. -
Consistent Variable Names: Use meaningful variable names throughout the code. For example, instead of
fileURL
, consider usingimageSrc
. -
Documentation: Add comments or documentation strings where necessary to explain complex logic or variables used.
-
Performance Considerations: If you have many languages dynamically loaded, consider optimizing how
langList
is handled to reduce render time during component mounting, especially if it's large.
Here’s an improved version with these thoughts taken into account:
// Improved JavaScript part
const langList = computed(() => {
// Ensure langList is properly defined and populated before accessing it
if (!localeConfigKey || !localeConfigKey.value) return [];
return localeConfigKey.value.map((data: any): { label: string; value: string } => ({
label: data.name,
value: data.code,
}));
});
const currentLocaleLabel = computed(
(): string | null =>
langList.value?.find(item => item.value === user.getLanguage())?.label || ''
);
const changeLang = (lang: string) => {
// Save new preferred language to local storage
useLocalStorage(localeConfigKey).value.currentLanguage = lang;
// Reload page to apply changes without server-side navigation
window.location.href = window.location.href.replace(encodeURIComponent(currentLocaleLabel.value), encodeURIComponent(lang));
};
// Improved Vue component template
<style scoped>
/* ... existing styles remain unchanged */
.right-container {
position: relative;
}
.lang {
display: inline-block;
margin-right: 20px;
cursor: pointer;
}
</style>
These changes focus on enhancing readability and maintainability while keeping performance considerations in mind.
import './assets/markdown-iconfont.js' | ||
// 引入公共库中的语言配置 | ||
import ZH_TW from '@vavt/cm-extension/dist/locale/zh-TW' | ||
|
||
defineOptions({ name: 'MdEditor' }) | ||
const language = computed(() => localStorage.getItem('MaxKB-locale') || '') | ||
const language = computed(() => localStorage.getItem('MaxKB-locale') || getBrowserLang() || '') | ||
config({ | ||
editorConfig: { | ||
languageUserDefined: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks mostly clean but has a few minor improvements that could be made:
-
TypeScript Compatibility: The
defineOptions
function is typically used with Vue 3 Composition API, so it's fine here since you're using TypeScript. -
Language Handling:
- Consider adding a default language preference if
localStorage.getItem('MaxKB-locale')
returns an empty string andgetBrowserLang()
also returns nothing.
- Consider adding a default language preference if
-
Syntax Correctness:
- Ensure there are no syntax errors. The provided snippet appears correct, but double-check for any missing semicolons or other syntactical issues.
-
Performance Optimization:
- For localization specifically, ensure that
zh-TW
, which seems to be the configuration file name, does not contain special characters unless necessary. If it includes spaces or hyphens, they might prevent proper loading of the module.
- For localization specifically, ensure that
-
Code Readability:
- Add comments to explain complex logic or where customizations start if needed.
Here’s the revised code:
<script setup lang="ts">
import { computed } from 'vue'
import { MdEditor, config } from 'md-editor-v3'
import './assets/markdown-iconfont.js'
// Import public library locale configuration
import ZH_TW from '@vavt/cm-extension/dist/locale/zh-TW'
// Define component options
defineOptions({ name: 'MdEditor' })
// Determine language using localStorage first, then browser settings
const language = computed(() => {
const storedLang = localStorage.getItem('MaxKB-locale');
return storedLang || window.navigator.language.startsWith('en-US') ? 'zh-TW' : '';
});
config({
editorConfig: {
languageUserDefined: {
enUS: require('@vavt/cm-extension/dist/locale/en_US'),
zhTW: require('@vavt/cm-extension/dist/locale/zh-TW')
},
initialMarkdownContent,
toolbarIconsToShow: [
// Toolbar icons list based on current language
],
height: '70vh',
width: '80vw'
// Other configurations...
}
});
</script>
Key Improvements:
- Added comments to clarify the purpose and functionality of certain parts of the code.
- Ensured that
window.navigator.language.startsWith('en-US')
handles different English locales correctly when determining a fallback language (in this case, setting it to 'zh-TW'). - Left some details about handling additional languages open-ended, such as how to dynamically add more languages to the
languageUserDefined
object in future updates.
This approach ensures better readability, maintainability, and flexibility in managing multi-language support within your application.
feat: login view add language setting