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.

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

feat: Save custom theme color scheme #7690

Merged
merged 1 commit into from
Jan 10, 2025
Merged

feat: Save custom theme color scheme #7690

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
10 changes: 8 additions & 2 deletions frontend/src/views/setting/panel/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,10 @@ const mobile = computed(() => {
interface ThemeColor {
light: string;
dark: string;
themePredefineColors: {
light: string[];
dark: string[];
};
}

const form = reactive({
Expand Down Expand Up @@ -366,8 +370,10 @@ const search = async () => {
const xpackRes = await getXpackSetting();
if (xpackRes) {
form.theme = xpackRes.data.theme || globalStore.themeConfig.theme || 'light';
form.themeColor = JSON.parse(xpackRes.data.themeColor);
globalStore.themeConfig.themeColor = xpackRes.data.themeColor;
form.themeColor = JSON.parse(xpackRes.data.themeColor || '{"light":"#005eeb","dark":"#F0BE96"}');
globalStore.themeConfig.themeColor = xpackRes.data.themeColor
? xpackRes.data.themeColor
: '{"light":"#005eeb","dark":"#F0BE96"}';
globalStore.themeConfig.theme = form.theme;
form.proxyDocker = xpackRes.data.proxyDocker;
}
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 has several improvements:

  1. Default Theme Color Initialization: Added default values for themeColor and themePredefineColors. If no data is received from the server, default colors are used to prevent errors.

  2. Simplified Default Values Setting: Removed unnecessary brackets around the {} syntax when initializing defaultThemeConfig.

  3. Null Coalescing Operator: Used null coalescing operator (?) to simplify the assignment of default theme colors, ensuring that if no color information is available, the default values are still applied without error.

  4. Avoiding Empty Object Check: Instead of checking if 'light' or 'dark' keys exist in an empty object before accessing them, directly assign default values assuming there will be color configurations stored correctly.

These changes improve readability and robustness by ensuring that default values are always set consistently, even if no dynamic data retrieval occurs.

Expand Down
28 changes: 21 additions & 7 deletions frontend/src/views/setting/panel/theme-color/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,24 @@ const drawerVisible = ref();
const loading = ref();

interface DialogProps {
themeColor: { light: string; dark: string };
themeColor: {
light: string;
dark: string;
themePredefineColors: {
light: string[];
dark: string[];
};
};
theme: '';
}

interface ThemeColor {
light: string;
dark: string;
themePredefineColors: {
light: string[];
dark: string[];
};
}

const form = reactive({
Expand All @@ -124,6 +135,7 @@ const lightPredefineColors = ref([
'#ff4500',
'#ff8c00',
'#ffd700',
'#333539',
]);
const darkPredefineColors = ref([
'#238636',
Expand All @@ -135,6 +147,7 @@ const darkPredefineColors = ref([
'#ff4500',
'#ff8c00',
'#ffd700',
'#333539',
]);

const defaultDarkColors = [
Expand Down Expand Up @@ -191,9 +204,13 @@ const acceptParams = (params: DialogProps): void => {
form.theme = params.theme;
form.dark = form.themeColor.dark;
form.light = form.themeColor.light;
if (form.themeColor.themePredefineColors) {
localStorage.setItem(STORAGE_KEY, JSON.stringify(form.themeColor.themePredefineColors));
}
initThemeColors();
lightPredefineColors.value = themeColors.value['light'];
darkPredefineColors.value = themeColors.value['dark'];
lightColors = defaultLightColors;
lightPredefineColors.value.slice(0, 2).forEach((color) => {
const exists = lightColors.some((item) => item.color === color);
if (!exists) {
Expand All @@ -203,6 +220,7 @@ const acceptParams = (params: DialogProps): void => {
});
}
});
darkColors = defaultDarkColors;
darkPredefineColors.value.slice(0, 2).forEach((color) => {
const exists = darkColors.some((item) => item.color === color);
if (!exists) {
Expand Down Expand Up @@ -259,7 +277,7 @@ const onSave = async (formEl: FormInstance | undefined) => {
}).then(async () => {
await formEl.validate(async (valid) => {
if (!valid) return;
form.themeColor = { light: form.light, dark: form.dark };
form.themeColor = { light: form.light, dark: form.dark, themePredefineColors: themeColors.value };
if (globalStore.isProductPro) {
await updateXpackSettingByKey('ThemeColor', JSON.stringify(form.themeColor));
MsgSuccess(i18n.global.t('commons.msg.operationSuccess'));
Expand Down Expand Up @@ -288,12 +306,8 @@ const onReSet = async () => {
cancelButtonText: i18n.global.t('commons.button.cancel'),
type: 'info',
}).then(async () => {
form.themeColor = { light: '#005eeb', dark: '#F0BE96' };
form.themeColor = { light: '#005eeb', dark: '#F0BE96', themePredefineColors: themeColors.value };
if (globalStore.isProductPro) {
localStorage.setItem(STORAGE_KEY, JSON.stringify(defaultColors));
themeColors.value = { ...defaultColors };
darkColors = [...defaultDarkColors];
lightColors = [...defaultLightColors];
await updateXpackSettingByKey('ThemeColor', JSON.stringify(form.themeColor));
MsgSuccess(i18n.global.t('commons.msg.operationSuccess'));
loading.value = false;
Copy link
Member

Choose a reason for hiding this comment

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

The code you've provided is mostly correct but lacks some minor improvements and optimizations. Here are some points to consider:

  1. Code Casing Consistency: Ensure that constants like STORAGE_KEY match their usage consistently across the file.

  2. Optional Chaining for Theme Predefined Colors: The use of optional chaining (?.) can make the code cleaner by handling cases where themePredefineColors might not be defined.

  3. Initialization Order: Consider ensuring the initialization order of variables, especially if there's any dependency between them.

  4. Local Storage Usage: Be mindful of how local storage works with asynchronous operations, as it might lead to unexpected behavior if accessed during state changes.

Here's an optimized version with these considerations:

const drawerVisible = ref();

interface DialogProps {
    themeColor: {
        light: string;
        dark: string;
        themePredefineColors?: { // Optional property
            light: string[];
            dark: string[];
        };
    };
}

interface ThemeColor extends DialogProps;

const form = reactive<DialogProps>({
    themeColor: {} as DialogProps,
});

const themeNames = ['light', 'dark'];
const defaultDarkColors = ['#238636', '#dcdcdc'];

// Optimize the initial load by using defaults without checking localStorage first
initializeDefaultThemeColors(themeNames);

function initializeDefaultThemeColors(names) {
    names.forEach(name => {
        form.themeColor[name] = defaultColors[name] || '';
    });
}

Explanation:

  • Casing Consistency: Ensured form, dialogProps, etc., follow camelCase convention.
  • Optional Chaining: Used to safely access properties within themePredefineColors.
  • Initialize Defaults Early: Moved default initialization before accessing local storage to avoid unnecessary checks when no predefined colors are set.

These adjustments should improve readability and maintainability while potentially reducing runtime overhead.

Make sure to adjust the logic according to your specific application requirements and handle any potential bugs based on real-world testing.

Expand Down
Loading