Fix/server web application menu #3341
Conversation
WalkthroughThe pull request introduces significant changes across multiple files, primarily focusing on enhancing localization through the addition of a new "MENU_APP" section in translation files for both Bulgarian and English. Additionally, modifications to configuration management and menu handling in the Electron application are made, including the introduction of a Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/server-web/src/main/helpers/services/libs/desktop-store.tsOops! Something went wrong! :( ESLint: 8.46.0 ESLint couldn't find the config "erb" to extend from. Please check that the name of the config is correct. The config "erb" was referenced from the config file in "/apps/server-web/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (11)
apps/server-web/src/main/helpers/services/libs/desktop-store.ts (1)
Line range hint
1-49: Consider architectural improvements for configuration managementAs this file handles critical configuration management, consider these architectural improvements:
- Add configuration validation on application startup
- Implement a configuration migration system for handling version updates
- Add logging for configuration changes
- Consider implementing a configuration schema validator
These improvements would help maintain configuration integrity and make the system more maintainable. Would you like me to provide implementation details for any of these suggestions?
apps/server-web/src/locales/i18n/bg/translation.json (1)
17-31: Consider consolidating duplicate menu translations.There are overlapping translations between the "MENU" and "MENU_APP" sections:
- "ABOUT" appears in both sections
- "SERVER_WINDOW" appears in both sections
This duplication might lead to maintenance challenges. Consider:
- Consolidating these translations under a single section
- Using translation key references if the same translation is needed in multiple places
Example consolidation:
{ "MENU": { "SERVER": "сървър", "UPDATER": "Актуализация", - "ABOUT": "относно", "SERVER_START": "Започнете", "SERVER_STOP": "Спри се", "APP_SETTING": "Настройка", - "APP_ABOUT": "относно", "APP_QUIT": "Откажете се", "GENERAL": "Общ", "SERVER_STATUS_STOPPED": "Статус: Спряна", "SERVER_STATUS_STARTED": "Статус: започна", "OPEN_WEB": "Отворете уеб в браузъра", - "SERVER_WINDOW": "Прозорец на сървъра" }, "MENU_APP": { // Keep all translations here as they are more consistently formatted // Other parts of the app should reference these keys } }apps/web/app/constants.ts (2)
320-321: Add documentation for the IS_DESKTOP_APP constantWhile the constant name is self-explanatory, it would be helpful to add a comment explaining its purpose and impact on the application's behavior, particularly its role in authentication configuration.
Add a comment above the constant:
+// Flag to identify if the application is running in desktop mode +// Used to configure authentication behavior for desktop environment export const IS_DESKTOP_APP = process.env.IS_DESKTOP_APP === 'true';
320-321: Consider handling undefined environment variable explicitlyThe current implementation will return
falsefor both undefined and non-'true' values. While this might be the intended behavior, it's better to be explicit about handling undefined cases.Consider using a more explicit check:
-export const IS_DESKTOP_APP = process.env.IS_DESKTOP_APP === 'true'; +export const IS_DESKTOP_APP = process.env.IS_DESKTOP_APP ? process.env.IS_DESKTOP_APP === 'true' : false;apps/server-web/src/main/main.ts (2)
185-185: Optimize menu rebuilding strategy.The menu is being rebuilt for each window creation which could be inefficient. Consider building the menu once and reusing it.
Consider centralizing menu creation:
+ // At the top level + const buildApplicationMenu = () => { + Menu.setApplicationMenu(appMenu.buildDefaultTemplate(appMenuItems, i18nextMainBackend)); + }; case 'SETTING_WINDOW': settingWindow = new BrowserWindow(defaultOptionWindow); - Menu.setApplicationMenu(appMenu.buildDefaultTemplate(appMenuItems, i18nextMainBackend)) + buildApplicationMenu(); break; case 'LOG_WINDOW': logWindow = new BrowserWindow(defaultOptionWindow); - Menu.setApplicationMenu(appMenu.buildDefaultTemplate(appMenuItems, i18nextMainBackend)) + buildApplicationMenu(); break;Also applies to: 196-196
435-435: Consider making server window auto-open configurable.Auto-opening the server window on initialization might not be desired in all scenarios.
Consider making this configurable through settings:
- eventEmitter.emit(EventLists.SERVER_WINDOW); + const config = LocalStore.getStore('config'); + if (config?.general?.autoOpenServerWindow) { + eventEmitter.emit(EventLists.SERVER_WINDOW); + }apps/server-web/src/main/menu.ts (5)
23-23: Consider localizing the application menu labelThe
labelon line 23 usesconfig.DESCRIPTION || app.getName(), which may not be localized. For consistency and to support internationalization, consider using an i18n key similar to other labels.Apply this diff to update the label:
-label: config.DESCRIPTION || app.getName(), +label: 'MENU_APP.APP_NAME',And add the corresponding translation in your localization files.
128-136: Refactor to eliminate code duplication in label translationThe code for translating labels in both menu and submenu items is duplicated. Consider creating a helper function to translate labels recursively, which will improve maintainability and readability.
Here's a refactored version:
translateAppMenu(i18nextMainBackend: typeof i18n, contextMenu: any) { + const translateItem = (item: any): any => { + const translatedItem = { ...item }; + if (translatedItem.label) { + translatedItem.label = i18nextMainBackend.t(translatedItem.label); + } + if (translatedItem.submenu && translatedItem.submenu.length) { + translatedItem.submenu = translatedItem.submenu.map(translateItem); + } + return translatedItem; + }; + - return contextMenu.map((menu: any) => { - const menuCopied = { ...menu }; - if (menuCopied.label) { - menuCopied.label = i18nextMainBackend.t(menuCopied.label); - } - if (menuCopied.submenu && menuCopied.submenu.length) { - menuCopied.submenu = menuCopied.submenu.map((sm: any) => { - const submenu = { ...sm }; - if (submenu.label) { - submenu.label = i18nextMainBackend.t(submenu.label); - } - return submenu; - }); - } - return menuCopied; - }); + return contextMenu.map(translateItem); }🧰 Tools
🪛 Biome
[error] 131-131: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
131-131: Simplify submenu existence check using optional chainingYou can simplify the condition on line 131 by using optional chaining to make the code more concise.
Apply this diff:
-if (menuCopied.submenu && menuCopied.submenu.length) { +if (menuCopied.submenu?.length) {🧰 Tools
🪛 Biome
[error] 131-131: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
113-123: Simplify theupdateAppMenumethod logicThe
updateAppMenumethod contains redundant code in both theifandelseblocks when setting the application menu. Refactoring can improve readability and reduce duplication.Here's a refactored version:
updateAppMenu( menuItem: string, context: { label?: string; enabled?: boolean }, contextMenuItems: any, i18nextMainBackend: typeof i18n ) { const menuIdx: number = contextMenuItems.findIndex( (item: any) => item.id === menuItem ); if (menuIdx > -1) { contextMenuItems[menuIdx] = { ...contextMenuItems[menuIdx], ...context, }; } const newMenu = [...contextMenuItems]; Menu.setApplicationMenu( Menu.buildFromTemplate( this.translateAppMenu(i18nextMainBackend, newMenu) ) ); }
12-12: Specify an access modifier for the class propertyFor clarity and to adhere to TypeScript best practices, consider explicitly specifying an access modifier (e.g.,
public,private, orprotected) for theeventEmitterproperty.Apply this diff:
-eventEmitter: EventEmitter +private eventEmitter: EventEmitter;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
apps/server-web/src/locales/i18n/bg/translation.json(1 hunks)apps/server-web/src/locales/i18n/en/translation.json(1 hunks)apps/server-web/src/main/helpers/services/libs/desktop-store.ts(2 hunks)apps/server-web/src/main/main.ts(8 hunks)apps/server-web/src/main/main_.ts(0 hunks)apps/server-web/src/main/menu.ts(2 hunks)apps/server-web/src/renderer/components/svgs/EverTeamsLogo.tsx(1 hunks)apps/web/app/constants.ts(1 hunks)apps/web/auth.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/server-web/src/main/main_.ts
✅ Files skipped from review due to trivial changes (1)
- apps/server-web/src/renderer/components/svgs/EverTeamsLogo.tsx
🧰 Additional context used
🪛 Biome
apps/server-web/src/main/menu.ts
[error] 131-131: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (4)
apps/web/auth.ts (2)
5-5: LGTM!
Clean import of the desktop app configuration constant.
9-9: Verify security implications of trustHost configuration
Setting trustHost: IS_DESKTOP_APP bypasses hostname verification in desktop mode. While this might be necessary for desktop applications, it could potentially expose security vulnerabilities if not properly controlled.
Let's verify the implementation and usage of IS_DESKTOP_APP:
Please ensure:
- The desktop app has additional security measures in place when hostname verification is bypassed
- The IS_DESKTOP_APP flag is properly controlled and can't be manipulated by end-users
- Consider documenting the security implications and any compensating controls in place
apps/server-web/src/locales/i18n/en/translation.json (1)
17-31: Verify Bulgarian translations for new menu items.
Please ensure that corresponding Bulgarian translations exist for all new menu items in the "MENU_APP" section.
✅ Verification successful
All menu items have corresponding Bulgarian translation keys ✅
The comparison shows that all keys in the "MENU_APP" section, including the new menu items, have matching entries in both English and Bulgarian translation files. The keys are identical and complete in both languages:
- All main menu items (ABOUT, QUIT, WINDOW, DEV, HELP)
- All submenu items (SETTING, SERVER_WINDOW, LEARN_MORE, DOC, SETTING_DEV, SERVER_DEV)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all English translations have corresponding Bulgarian translations
# Get the Bulgarian translation file and compare keys
# Find the Bulgarian translation file
bg_file=$(fd -t f "translation.json" -p "bg")
if [ -n "$bg_file" ]; then
# Extract MENU_APP keys from both files and compare
echo "Comparing translation keys between English and Bulgarian files..."
# Extract and format English keys
en_keys=$(cat apps/server-web/src/locales/i18n/en/translation.json | jq -r '.MENU_APP | .. | objects | keys[]' 2>/dev/null | sort)
# Extract and format Bulgarian keys
bg_keys=$(cat "$bg_file" | jq -r '.MENU_APP | .. | objects | keys[]' 2>/dev/null | sort)
# Compare keys
diff <(echo "$en_keys") <(echo "$bg_keys")
fi
Length of output: 150
Script:
#!/bin/bash
# Let's try a different approach to find the Bulgarian translation file
echo "Looking for translation files..."
fd -t f "translation.json" --full-path
echo -e "\nChecking content of locales directory..."
ls -R apps/server-web/src/locales/
# If we find the files, we'll compare them
if [ -f "apps/server-web/src/locales/i18n/bg/translation.json" ]; then
echo -e "\nComparing translation keys..."
# Extract and format English keys
echo "English keys:"
cat apps/server-web/src/locales/i18n/en/translation.json | jq -r '.MENU_APP | .. | objects | keys[]' 2>/dev/null | sort
echo -e "\nBulgarian keys:"
cat apps/server-web/src/locales/i18n/bg/translation.json | jq -r '.MENU_APP | .. | objects | keys[]' 2>/dev/null | sort
fi
Length of output: 1224
apps/server-web/src/locales/i18n/bg/translation.json (1)
17-31: LGTM! Bulgarian translations are accurate and well-structured.
The new "MENU_APP" section provides proper Bulgarian translations that follow correct grammar and spelling. The hierarchical structure with submenu items is logically organized.
| const defaultConfig: WebServer | any = store.get('config') || {}; | ||
| Object.keys(DEFAULT_CONFIG).forEach((key) => { | ||
| Object.keys(DEFAULT_CONFIG[key]).forEach((keySub) => { | ||
| defaultConfig[key] = defaultConfig[key] || {}; | ||
| defaultConfig[key][keySub] = defaultConfig[key][keySub] || DEFAULT_CONFIG[key][keySub]; | ||
| }) | ||
| }) | ||
| store.set({ | ||
| config: defaultConfig | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance robustness and efficiency of configuration management
The current implementation has several potential improvements:
- The nested iteration could be replaced with a more efficient deep merge
- Add error handling for malformed configurations
- Add validation for required fields
- Add type safety to the configuration merging process
Consider refactoring to:
-setDefaultServerConfig: () => {
- const defaultConfig: WebServer | any = store.get('config') || {};
- Object.keys(DEFAULT_CONFIG).forEach((key) => {
- Object.keys(DEFAULT_CONFIG[key]).forEach((keySub) => {
- defaultConfig[key] = defaultConfig[key] || {};
- defaultConfig[key][keySub] = defaultConfig[key][keySub] || DEFAULT_CONFIG[key][keySub];
- })
- })
- store.set({
- config: defaultConfig
- });
-}
+setDefaultServerConfig: () => {
+ try {
+ const storedConfig = store.get('config') as Partial<AppConfig> || {};
+
+ // Deep merge with type safety
+ const mergedConfig = deepMerge<AppConfig>(DEFAULT_CONFIG, storedConfig);
+
+ // Validate required fields
+ validateConfig(mergedConfig);
+
+ store.set({ config: mergedConfig });
+ } catch (error) {
+ console.error('Failed to set default configuration:', error);
+ // Fallback to DEFAULT_CONFIG if stored config is corrupted
+ store.set({ config: DEFAULT_CONFIG });
+ }
+}Add these helper functions:
function deepMerge<T>(target: T, source: Partial<T>): T {
const result: T = { ...target };
Object.keys(source).forEach(key => {
const value = source[key as keyof T];
if (value && typeof value === 'object') {
result[key as keyof T] = deepMerge(
target[key as keyof T],
value as any
);
} else if (value !== undefined) {
result[key as keyof T] = value as any;
}
});
return result;
}
function validateConfig(config: AppConfig): void {
const required = ['PORT', 'GAUZY_API_SERVER_URL'];
required.forEach(field => {
if (!config.server[field as keyof ServerConfig]) {
throw new Error(`Missing required field: ${field}`);
}
});
}| const DEFAULT_CONFIG:any = { | ||
| server: { | ||
| PORT: 3002, | ||
| GAUZY_API_SERVER_URL: 'http://localhost:3000', | ||
| NEXT_PUBLIC_GAUZY_API_SERVER_URL: 'http://localhost:3000', | ||
| DESKTOP_WEB_SERVER_HOSTNAME: '0.0.0.0' | ||
| }, | ||
| general: { | ||
| lang: 'en', | ||
| autoUpdate: true, | ||
| updateCheckPeriode: '1140' | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety and configuration management
The DEFAULT_CONFIG constant has several areas for improvement:
- Replace 'any' type with a proper interface
- Consider moving sensitive configuration to environment variables
- Add validation for configuration values
- Add documentation for each configuration option
Consider applying these changes:
-const DEFAULT_CONFIG:any = {
+interface ServerConfig {
+ PORT: number;
+ GAUZY_API_SERVER_URL: string;
+ NEXT_PUBLIC_GAUZY_API_SERVER_URL: string;
+ DESKTOP_WEB_SERVER_HOSTNAME: string;
+}
+
+interface GeneralConfig {
+ lang: string;
+ autoUpdate: boolean;
+ updateCheckPeriode: string;
+}
+
+interface AppConfig {
+ server: ServerConfig;
+ general: GeneralConfig;
+}
+
+/**
+ * Default configuration for the application
+ * @property {ServerConfig} server - Server-related configuration
+ * @property {GeneralConfig} general - General application settings
+ */
+const DEFAULT_CONFIG: AppConfig = {
server: {
- PORT: 3002,
+ PORT: process.env.SERVER_PORT ? parseInt(process.env.SERVER_PORT) : 3002,
- GAUZY_API_SERVER_URL: 'http://localhost:3000',
+ GAUZY_API_SERVER_URL: process.env.GAUZY_API_SERVER_URL || 'http://localhost:3000',
- NEXT_PUBLIC_GAUZY_API_SERVER_URL: 'http://localhost:3000',
+ NEXT_PUBLIC_GAUZY_API_SERVER_URL: process.env.NEXT_PUBLIC_GAUZY_API_SERVER_URL || 'http://localhost:3000',
DESKTOP_WEB_SERVER_HOSTNAME: '0.0.0.0'
},
general: {
lang: 'en',
autoUpdate: true,
- updateCheckPeriode: '1140'
+ updateCheckPeriode: '1140' // Time in minutes
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const DEFAULT_CONFIG:any = { | |
| server: { | |
| PORT: 3002, | |
| GAUZY_API_SERVER_URL: 'http://localhost:3000', | |
| NEXT_PUBLIC_GAUZY_API_SERVER_URL: 'http://localhost:3000', | |
| DESKTOP_WEB_SERVER_HOSTNAME: '0.0.0.0' | |
| }, | |
| general: { | |
| lang: 'en', | |
| autoUpdate: true, | |
| updateCheckPeriode: '1140' | |
| } | |
| } | |
| interface ServerConfig { | |
| PORT: number; | |
| GAUZY_API_SERVER_URL: string; | |
| NEXT_PUBLIC_GAUZY_API_SERVER_URL: string; | |
| DESKTOP_WEB_SERVER_HOSTNAME: string; | |
| } | |
| interface GeneralConfig { | |
| lang: string; | |
| autoUpdate: boolean; | |
| updateCheckPeriode: string; | |
| } | |
| interface AppConfig { | |
| server: ServerConfig; | |
| general: GeneralConfig; | |
| } | |
| /** | |
| * Default configuration for the application | |
| * @property {ServerConfig} server - Server-related configuration | |
| * @property {GeneralConfig} general - General application settings | |
| */ | |
| const DEFAULT_CONFIG: AppConfig = { | |
| server: { | |
| PORT: process.env.SERVER_PORT ? parseInt(process.env.SERVER_PORT) : 3002, | |
| GAUZY_API_SERVER_URL: process.env.GAUZY_API_SERVER_URL || 'http://localhost:3000', | |
| NEXT_PUBLIC_GAUZY_API_SERVER_URL: process.env.NEXT_PUBLIC_GAUZY_API_SERVER_URL || 'http://localhost:3000', | |
| DESKTOP_WEB_SERVER_HOSTNAME: '0.0.0.0' | |
| }, | |
| general: { | |
| lang: 'en', | |
| autoUpdate: true, | |
| updateCheckPeriode: '1140' // Time in minutes | |
| } | |
| } |
| "MENU_APP": { | ||
| "ABOUT": "About", | ||
| "QUIT": "Quit", | ||
| "WINDOW": "Window", | ||
| "SUBMENU": { | ||
| "SETTING": "Setting", | ||
| "SERVER_WINDOW": "Server Window", | ||
| "LEARN_MORE": "Learn More", | ||
| "DOC": "Documentation", | ||
| "SETTING_DEV": "Setting Dev.", | ||
| "SERVER_DEV": "Server Dev." | ||
| }, | ||
| "DEV": "Developer", | ||
| "HELP": "Help" | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider consolidating duplicate menu translations and standardizing naming patterns.
There appear to be overlapping translations between the existing "MENU" section and the new "MENU_APP" section:
- "ABOUT" duplicates existing translation
- "QUIT" duplicates "APP_QUIT"
- "SERVER_WINDOW" exists in both sections
- "SETTING" duplicates "APP_SETTING"
Additionally, the naming patterns are inconsistent:
- The "MENU" section uses prefixes like "APP_" and "SERVER_"
- The "MENU_APP" section doesn't follow this pattern
Consider consolidating these sections and following a consistent naming pattern:
"MENU_APP": {
- "ABOUT": "About",
- "QUIT": "Quit",
- "WINDOW": "Window",
+ "APP_ABOUT": "About",
+ "APP_QUIT": "Quit",
+ "APP_WINDOW": "Window",
"SUBMENU": {
- "SETTING": "Setting",
- "SERVER_WINDOW": "Server Window",
+ "APP_SETTING": "Setting",
+ "APP_SERVER_WINDOW": "Server Window",
"LEARN_MORE": "Learn More",
"DOC": "Documentation",
- "SETTING_DEV": "Setting Dev.",
- "SERVER_DEV": "Server Dev."
+ "DEV_SETTING": "Setting Dev.",
+ "DEV_SERVER": "Server Dev."
},
"DEV": "Developer",
"HELP": "Help"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "MENU_APP": { | |
| "ABOUT": "About", | |
| "QUIT": "Quit", | |
| "WINDOW": "Window", | |
| "SUBMENU": { | |
| "SETTING": "Setting", | |
| "SERVER_WINDOW": "Server Window", | |
| "LEARN_MORE": "Learn More", | |
| "DOC": "Documentation", | |
| "SETTING_DEV": "Setting Dev.", | |
| "SERVER_DEV": "Server Dev." | |
| }, | |
| "DEV": "Developer", | |
| "HELP": "Help" | |
| }, | |
| "MENU_APP": { | |
| "APP_ABOUT": "About", | |
| "APP_QUIT": "Quit", | |
| "APP_WINDOW": "Window", | |
| "SUBMENU": { | |
| "APP_SETTING": "Setting", | |
| "APP_SERVER_WINDOW": "Server Window", | |
| "LEARN_MORE": "Learn More", | |
| "DOC": "Documentation", | |
| "DEV_SETTING": "Setting Dev.", | |
| "DEV_SERVER": "Server Dev." | |
| }, | |
| "DEV": "Developer", | |
| "HELP": "Help" | |
| }, |
| @@ -1,5 +1,5 @@ | |||
| import path from 'path'; | |||
| import { app, ipcMain, Tray, dialog, BrowserWindow, shell } from 'electron'; | |||
| import { app, ipcMain, Tray, dialog, BrowserWindow, shell, Menu } from 'electron'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving type safety and menu initialization.
The menu system changes could benefit from better type safety and centralization:
- The
appMenuItemsarray is typed asanywhich reduces type safety - Menu initialization is scattered across multiple places
Consider these improvements:
- let appMenuItems: any = [];
+ interface MenuItem {
+ label: string;
+ click?: () => void;
+ submenu?: MenuItem[];
+ }
+ let appMenuItems: MenuItem[] = [];Also applies to: 44-44, 95-95
| @@ -218,7 +212,7 @@ | |||
| const runServer = async () => { | |||
| console.log('Run the Server...'); | |||
| try { | |||
| const envVal = getEnvApi(); | |||
| const envVal: any = getEnvApi(); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Strengthen type safety throughout the application.
While adding type annotation for envVal is good, consider extending type safety to other areas of the application.
Consider adding proper types instead of using any:
- const envVal: any = getEnvApi();
+ interface EnvConfig {
+ PORT: number;
+ NEXT_PUBLIC_GAUZY_API_SERVER_URL: string;
+ // Add other environment variables
+ }
+ const envVal: EnvConfig = getEnvApi();Committable suggestion skipped: line range outside the PR's diff.
| eventEmitter.on(EventLists.SETTING_WINDOW_DEV, () => { | ||
| settingWindow?.webContents.toggleDevTools(); | ||
| }) | ||
|
|
||
| eventEmitter.on(EventLists.SERVER_WINDOW_DEV, () => { | ||
| logWindow?.webContents.toggleDevTools(); | ||
| }) |
There was a problem hiding this comment.
Add production safeguards for development features.
The dev tools toggle functionality should be restricted to development environments.
Consider adding environment checks:
eventEmitter.on(EventLists.SETTING_WINDOW_DEV, () => {
+ if (process.env.NODE_ENV === 'development') {
settingWindow?.webContents.toggleDevTools();
+ }
})
eventEmitter.on(EventLists.SERVER_WINDOW_DEV, () => {
+ if (process.env.NODE_ENV === 'development') {
logWindow?.webContents.toggleDevTools();
+ }
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| eventEmitter.on(EventLists.SETTING_WINDOW_DEV, () => { | |
| settingWindow?.webContents.toggleDevTools(); | |
| }) | |
| eventEmitter.on(EventLists.SERVER_WINDOW_DEV, () => { | |
| logWindow?.webContents.toggleDevTools(); | |
| }) | |
| eventEmitter.on(EventLists.SETTING_WINDOW_DEV, () => { | |
| if (process.env.NODE_ENV === 'development') { | |
| settingWindow?.webContents.toggleDevTools(); | |
| } | |
| }) | |
| eventEmitter.on(EventLists.SERVER_WINDOW_DEV, () => { | |
| if (process.env.NODE_ENV === 'development') { | |
| logWindow?.webContents.toggleDevTools(); | |
| } | |
| }) |
| // check and set default config | ||
| LocalStore.setDefaultServerConfig(); | ||
| createIntervalAutoUpdate() | ||
| trayMenuItems = trayMenuItems.length ? trayMenuItems : defaultTrayMenuItem(eventEmitter); | ||
| appMenuItems = appMenuItems.length ? appMenuItems : appMenu.defaultMenu(); | ||
| tray = _initTray(trayMenuItems, getAssetPath('icons/icon.png')); | ||
| i18nextMainBackend.on('languageChanged', (lng) => { | ||
| if (i18nextMainBackend.isInitialized) { | ||
|
|
||
| trayMenuItems = trayMenuItems.length ? trayMenuItems : defaultTrayMenuItem(eventEmitter); | ||
| updateTrayMenu('none', {}, eventEmitter, tray, trayMenuItems, i18nextMainBackend); | ||
| Menu.setApplicationMenu(appMenu.buildDefaultTemplate(appMenuItems, i18nextMainBackend)) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling in initialization sequence.
The initialization sequence lacks proper error handling and could benefit from debouncing frequent operations.
Consider adding error handling and debouncing:
+ import debounce from 'lodash/debounce';
const onInitApplication = () => {
+ try {
LocalStore.setDefaultServerConfig();
createIntervalAutoUpdate()
trayMenuItems = trayMenuItems.length ? trayMenuItems : defaultTrayMenuItem(eventEmitter);
appMenuItems = appMenuItems.length ? appMenuItems : appMenu.defaultMenu();
tray = _initTray(trayMenuItems, getAssetPath('icons/icon.png'));
+ } catch (error) {
+ console.error('Failed to initialize application:', error);
+ dialog.showErrorBox('Initialization Error', 'Failed to initialize application');
+ }
- i18nextMainBackend.on('languageChanged', (lng) => {
+ i18nextMainBackend.on('languageChanged', debounce((lng) => {
if (i18nextMainBackend.isInitialized) {
trayMenuItems = trayMenuItems.length ? trayMenuItems : defaultTrayMenuItem(eventEmitter);
updateTrayMenu('none', {}, eventEmitter, tray, trayMenuItems, i18nextMainBackend);
Menu.setApplicationMenu(appMenu.buildDefaultTemplate(appMenuItems, i18nextMainBackend))
}
- });
+ }, 250));Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (8)
apps/server-web/src/renderer/components/svgs/EverTeamsLogo.tsx (1)
7-8: Consider using responsive width/height attributesThe hardcoded width and height values might not scale well across different screen sizes. Consider using responsive values or Tailwind's responsive utilities.
- width="200" - height="100" + className="h-[100px] w-[200px] md:h-[120px] md:w-[240px] lg:h-[150px] lg:w-[300px]"apps/server-web/src/locales/i18n/en/translation.json (1)
29-29: Maintain consistent casing in menu itemsThe translation "Developer" uses sentence case while other menu items use title case.
Consider changing to "Developer" to maintain consistency:
- "DEV": "Developer", + "DEV": "Developer",apps/server-web/src/main/main.ts (4)
2-2: Consider using proper TypeTypes for menu items.The menu-related variables and imports are correctly structured, but using
anytype for menu items reduces type safety. Consider creating proper interfaces for menu items.-let appMenuItems: any = []; +interface MenuItemType { + label: string; + click?: () => void; + submenu?: MenuItemType[]; +} +let appMenuItems: MenuItemType[] = [];Also applies to: 44-44, 95-95
259-270: Improve code comment clarity.The comment "check and set default config" could be more descriptive about what configuration is being set and why.
-// check and set default config +// Initialize server configuration with default values if not already setLGTM: Robust menu initialization and language handling.
The menu initialization and language change handling are well-implemented, ensuring both tray and application menus stay in sync with the selected language.
435-435: Consider making server window auto-open configurable.Automatically opening the server window at startup might not be desired by all users. Consider making this behavior configurable through settings.
- eventEmitter.emit(EventLists.SERVER_WINDOW); + const config = LocalStore.getStore('config'); + if (config?.general?.autoOpenServerWindow) { + eventEmitter.emit(EventLists.SERVER_WINDOW); + }
215-215: Add proper typing for environment values.Using
anytype for environment values reduces type safety. Consider creating an interface that describes the expected environment structure.- const envVal: any = getEnvApi(); + interface EnvConfig { + PORT: number; + // Add other expected environment variables + } + const envVal: EnvConfig = getEnvApi();apps/server-web/src/main/menu.ts (2)
133-133: Simplify condition using optional chainingThe condition
if (menuCopied.submenu && menuCopied.submenu.length)can be simplified with optional chaining for better readability and conciseness.Apply this diff to simplify the condition:
- if (menuCopied.submenu && menuCopied.submenu.length) { + if (menuCopied.submenu?.length) {🧰 Tools
🪛 Biome
[error] 133-133: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
115-125: Optimize menu update to prevent unnecessary resetsWhen the specified
menuItemis not found, the application menu is being reset without any changes. To improve performance, consider skipping the menu reset when no modifications are made.Apply this diff:
updateAppMenu(menuItem: string, context: { label?: string, enabled?: boolean}, contextMenuItems: any, i18nextMainBackend: typeof i18n) { const menuIdx: number = contextMenuItems.findIndex((item: any) => item.id === menuItem); if (menuIdx > -1) { contextMenuItems[menuIdx] = { ...contextMenuItems[menuIdx], ...context }; const newMenu = [...contextMenuItems]; Menu.setApplicationMenu(Menu.buildFromTemplate(this.translateAppMenu(i18nextMainBackend, newMenu))); - } else { - const newMenu = [...contextMenuItems]; - Menu.setApplicationMenu(Menu.buildFromTemplate(this.translateAppMenu(i18nextMainBackend, newMenu))); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
apps/server-web/src/locales/i18n/bg/translation.json(1 hunks)apps/server-web/src/locales/i18n/en/translation.json(1 hunks)apps/server-web/src/main/helpers/services/libs/desktop-store.ts(2 hunks)apps/server-web/src/main/main.ts(8 hunks)apps/server-web/src/main/main_.ts(0 hunks)apps/server-web/src/main/menu.ts(2 hunks)apps/server-web/src/renderer/components/svgs/EverTeamsLogo.tsx(1 hunks)apps/web/app/constants.ts(1 hunks)apps/web/auth.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/server-web/src/main/main_.ts
🧰 Additional context used
🪛 Biome
apps/server-web/src/main/menu.ts
[error] 133-133: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (8)
apps/web/auth.ts (1)
5-5: LGTM!
The import statement is properly placed and follows the project's import conventions.
apps/server-web/src/main/helpers/services/libs/desktop-store.ts (2)
40-49: 🛠️ Refactor suggestion
Improve configuration merging robustness and error handling
The current implementation of configuration merging has several potential issues:
- No validation of the merged configuration
- No error handling for malformed configurations
- Custom deep merge implementation could be replaced with a proven solution
Consider using a library like deepmerge and adding validation:
+import deepmerge from 'deepmerge';
+
setDefaultServerConfig: () => {
- const defaultConfig: WebServer | any = store.get('config') || {};
- Object.keys(DEFAULT_CONFIG).forEach((key) => {
- Object.keys(DEFAULT_CONFIG[key]).forEach((keySub) => {
- defaultConfig[key] = defaultConfig[key] || {};
- defaultConfig[key][keySub] = defaultConfig[key][keySub] || DEFAULT_CONFIG[key][keySub];
- })
- })
+ try {
+ const existingConfig = store.get('config') || {};
+ const mergedConfig = deepmerge(DEFAULT_CONFIG, existingConfig);
+
+ // Validate configuration
+ if (!mergedConfig.server?.PORT || !mergedConfig.server?.GAUZY_API_SERVER_URL) {
+ throw new Error('Invalid configuration: Missing required server settings');
+ }
+
+ store.set({ config: mergedConfig });
+ } catch (error) {
+ console.error('Failed to set default configuration:', error);
+ // Fallback to DEFAULT_CONFIG if merge fails
+ store.set({ config: DEFAULT_CONFIG });
+ }
- store.set({
- config: defaultConfig
- });
}Let's verify the current usage of configuration merging:
#!/bin/bash
# Search for other instances of configuration merging that might need similar updates
ast-grep --pattern 'store.set({ config: $_ })'4-16: 🛠️ Refactor suggestion
Consider improving type safety and configuration management
The current implementation has several areas for improvement:
- The
anytype should be replaced with a proper interface - Sensitive configuration values should be moved to environment variables
- Configuration validation should be added
Consider applying these changes:
-const DEFAULT_CONFIG:any = {
+interface ServerConfig {
+ PORT: number;
+ GAUZY_API_SERVER_URL: string;
+ NEXT_PUBLIC_GAUZY_API_SERVER_URL: string;
+ DESKTOP_WEB_SERVER_HOSTNAME: string;
+}
+
+interface GeneralConfig {
+ lang: string;
+ autoUpdate: boolean;
+ updateCheckPeriode: string;
+}
+
+interface AppConfig {
+ server: ServerConfig;
+ general: GeneralConfig;
+}
+
+const DEFAULT_CONFIG: AppConfig = {
server: {
- PORT: 3002,
- GAUZY_API_SERVER_URL: 'http://localhost:3000',
- NEXT_PUBLIC_GAUZY_API_SERVER_URL: 'http://localhost:3000',
- DESKTOP_WEB_SERVER_HOSTNAME: '0.0.0.0'
+ PORT: process.env.DEFAULT_PORT || 3002,
+ GAUZY_API_SERVER_URL: process.env.DEFAULT_API_URL || 'http://localhost:3000',
+ NEXT_PUBLIC_GAUZY_API_SERVER_URL: process.env.DEFAULT_PUBLIC_API_URL || 'http://localhost:3000',
+ DESKTOP_WEB_SERVER_HOSTNAME: process.env.DEFAULT_HOSTNAME || '0.0.0.0'
},Let's verify if these configuration values are used consistently across the codebase:
apps/server-web/src/locales/i18n/en/translation.json (1)
17-31: Verify the necessity of two separate menu sections
The presence of both "MENU" and "MENU_APP" sections with overlapping translations suggests a potential architectural issue.
Let's verify the usage of these sections:
✅ Verification successful
The two menu sections serve different purposes and are correctly separated
The search results show a clear distinction in usage:
MENU_APPis used exclusively inmenu.tsfor the application's main window menuMENUis used intray.tsand other components for the system tray menu and UI elements
This separation is intentional and architecturally sound as it distinguishes between:
- Application window menus (
MENU_APP) - System tray and component-specific menus (
MENU)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usages of both menu sections to understand their distinct purposes
echo "Searching for MENU usages:"
rg "MENU\." -t ts -t tsx
echo -e "\nSearching for MENU_APP usages:"
rg "MENU_APP\." -t ts -t tsx
Length of output: 253
Script:
#!/bin/bash
# Search for usages of both menu sections with correct file type flags
echo "Searching for MENU usages:"
rg "MENU\." --type-add 'ts:*.{ts,tsx}' -t ts
echo -e "\nSearching for MENU_APP usages:"
rg "MENU_APP\." --type-add 'ts:*.{ts,tsx}' -t ts
# Also search in JavaScript files as they might contain these strings
echo -e "\nSearching in JS files:"
rg "MENU(_APP)?\." -t js
Length of output: 2772
apps/server-web/src/locales/i18n/bg/translation.json (1)
17-31: Verify the transition from "MENU" to "MENU_APP".
The addition of "MENU_APP" alongside existing "MENU" suggests a transition. Let's verify this change across the codebase.
✅ Verification successful
Let me gather more information about the codebase to understand if this is a new addition or a replacement.
Based on the git history output, I can now generate the final response as we have clear evidence of the transition.
The transition from "MENU" to "MENU_APP" is a deliberate refactoring of the menu structure
The git history shows this is a significant menu restructuring where:
- The old menu used hardcoded English labels ('&File', 'Help', etc.)
- The new "MENU_APP" structure introduces proper i18n support with translation keys
- Both "MENU" and "MENU_APP" coexist because they serve different purposes:
- "MENU" handles server-related actions (start/stop)
- "MENU_APP" handles application-level menu items (about/quit/window)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage of MENU vs MENU_APP keys in the codebase
# Search for MENU usage
echo "=== Searching for MENU key usage ==="
rg -g '*.{ts,tsx,js,jsx}' '"MENU\.' -A 2
# Search for MENU_APP usage
echo -e "\n=== Searching for MENU_APP key usage ==="
rg -g '*.{ts,tsx,js,jsx}' '"MENU_APP\.' -A 2
# Check other translation files for similar structure
echo -e "\n=== Checking other translation files ==="
fd 'translation.json' | xargs rg '"MENU"' -A 5
Length of output: 1345
Script:
#!/bin/bash
# Check git history for changes related to MENU and MENU_APP
echo "=== Checking git history for MENU_APP changes ==="
git log -p | rg -A 5 -B 5 "MENU_APP"
# Check for any menu-related component usage
echo -e "\n=== Checking for menu-related component usage ==="
rg -g '*.{ts,tsx,js,jsx}' -A 3 "useTranslation|t\(['\"]MENU|t\(['\"]MENU_APP"
# Check if both MENU and MENU_APP exist in the same translation files
echo -e "\n=== Checking translation files for coexistence ==="
fd translation.json | xargs cat
Length of output: 3732
apps/server-web/src/main/main.ts (1)
185-185: LGTM: Consistent menu application across windows.
The menu application is implemented correctly and consistently across different window types. The menu is properly rebuilt when the language changes.
Also applies to: 196-196
apps/server-web/src/main/menu.ts (2)
9-11: Imports are correctly added for new dependencies
The imported modules EventEmitter, EventLists, and i18n are appropriately added and used within the class.
14-14: Update class to use EventEmitter
The MenuBuilder class now uses EventEmitter instead of BrowserWindow, promoting a decoupled design that handles events without direct dependency on the window instance.
Also applies to: 16-17
| <img | ||
| src={Logo} | ||
| alt="EverTeams Logo" | ||
| width="200" | ||
| height="100" | ||
| className="object-contain brightness-0 dark:invert invert-0 fill-black dark:fill-white" | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review component organization and styling approach
There are a few inconsistencies to address:
- The component is in an
svgsfolder but uses a PNG image - The
fill-blackandfill-whiteclasses won't affect PNG images as they're meant for SVG elements
Consider either:
- Moving this component to a more appropriate folder (e.g.,
components/commonorcomponents/brand) - Converting the logo to SVG if vector format is desired, which would then make the fill colors work as intended
For the current PNG implementation, you can simplify the classes:
- className="object-contain brightness-0 dark:invert invert-0 fill-black dark:fill-white"
+ className="object-contain brightness-0 dark:invert invert-0"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <img | |
| src={Logo} | |
| alt="EverTeams Logo" | |
| width="200" | |
| height="100" | |
| className="object-contain brightness-0 dark:invert invert-0 fill-black dark:fill-white" | |
| /> | |
| <img | |
| src={Logo} | |
| alt="EverTeams Logo" | |
| width="200" | |
| height="100" | |
| className="object-contain brightness-0 dark:invert invert-0" | |
| /> |
apps/web/auth.ts
Outdated
|
|
||
| export const { handlers, signIn, signOut, auth } = NextAuth((request) => ({ | ||
| providers: filteredProviders, | ||
| trustHost: IS_DESKTOP_APP, |
There was a problem hiding this comment.
💡 Codebase verification
Security concern: IS_DESKTOP_APP environment variable needs runtime validation
The current implementation of IS_DESKTOP_APP lacks proper runtime validation and security controls:
- It's simply defined as
process.env.IS_DESKTOP_APP === 'true'inapps/web/app/constants.ts - No middleware or runtime checks ensure this flag is only set in genuine desktop environments
- Package scripts show desktop builds are handled differently, but no security validation is present
- Setting this to
truein a non-desktop environment would bypass CSRF protection
Recommendations:
- Add runtime validation to ensure the app is actually running in Electron context
- Implement middleware to verify the desktop environment before trusting the host
- Consider using Electron's IPC for secure host verification instead of an environment variable
🔗 Analysis chain
Verify security implications of trustHost setting
Setting trustHost to true bypasses CSRF protection in NextAuth.js. Please ensure that IS_DESKTOP_APP is properly controlled and validated to prevent security vulnerabilities.
Let's verify the implementation of IS_DESKTOP_APP:
Let me gather more information about the environment variable validation and its usage in the build process:
Let me check one final thing about the desktop app setup and environment configuration:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation and usage of IS_DESKTOP_APP constant
# Check the constant definition
echo "Checking IS_DESKTOP_APP constant definition:"
rg -p "export.*IS_DESKTOP_APP.*=" --type ts
# Check all usages of this constant
echo "\nChecking all usages of IS_DESKTOP_APP:"
rg "IS_DESKTOP_APP" --type ts
# Check for any environment variable validation
echo "\nChecking for environment variable validation:"
rg "process\.env\.IS_DESKTOP_APP" --type ts
Length of output: 816
Script:
#!/bin/bash
# Check for environment variable configuration in build files
echo "Checking for environment variable configuration:"
rg -p "IS_DESKTOP_APP" --type json --type env --type yaml
# Check for any validation middleware or security checks
echo "\nChecking for security middleware and validation:"
rg -p "middleware|security|csrf" --type ts -A 5 -B 5
# Check for desktop-specific code or electron setup
echo "\nChecking for desktop app setup:"
fd -e ts -e js -e json electron
Length of output: 7074
Script:
#!/bin/bash
# Check the desktop app configuration
echo "Checking desktop app setup:"
cat apps/server-web/electronmon.js
# Check for any environment configuration files
echo "\nChecking environment files:"
fd -e env -e env.example -e env.local
# Check package.json for desktop-related scripts
echo "\nChecking package.json for desktop configuration:"
rg -p "desktop|electron" package.json
Length of output: 1509
| "MENU_APP": { | ||
| "ABOUT": "About", | ||
| "QUIT": "Quit", | ||
| "WINDOW": "Window", | ||
| "SUBMENU": { | ||
| "SETTING": "Setting", | ||
| "SERVER_WINDOW": "Server Window", | ||
| "LEARN_MORE": "Learn More", | ||
| "DOC": "Documentation", | ||
| "SETTING_DEV": "Setting Dev.", | ||
| "SERVER_DEV": "Server Dev." | ||
| }, | ||
| "DEV": "Developer", | ||
| "HELP": "Help" | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider consolidating duplicate menu translations
There are several duplicate translations between the existing "MENU" section and the new "MENU_APP" section:
- "About" (MENU.ABOUT/APP_ABOUT vs MENU_APP.ABOUT)
- "Setting" (MENU.APP_SETTING vs MENU_APP.SUBMENU.SETTING)
- "Server Window" (MENU.SERVER_WINDOW vs MENU_APP.SUBMENU.SERVER_WINDOW)
- "Quit" (MENU.APP_QUIT vs MENU_APP.QUIT)
This duplication could lead to maintenance issues if translations need to be updated.
Consider consolidating these translations into a single section or clarifying the distinct purposes of "MENU" vs "MENU_APP" through comments.
| eventEmitter.on(EventLists.SETTING_WINDOW_DEV, () => { | ||
| settingWindow?.webContents.toggleDevTools(); | ||
| }) | ||
|
|
||
| eventEmitter.on(EventLists.SERVER_WINDOW_DEV, () => { | ||
| logWindow?.webContents.toggleDevTools(); | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider restricting dev tools in production.
Developer tools events should only be registered in development mode to prevent unintended access in production.
- eventEmitter.on(EventLists.SETTING_WINDOW_DEV, () => {
- settingWindow?.webContents.toggleDevTools();
- })
-
- eventEmitter.on(EventLists.SERVER_WINDOW_DEV, () => {
- logWindow?.webContents.toggleDevTools();
- })
+ if (isDebug) {
+ eventEmitter.on(EventLists.SETTING_WINDOW_DEV, () => {
+ settingWindow?.webContents.toggleDevTools();
+ })
+
+ eventEmitter.on(EventLists.SERVER_WINDOW_DEV, () => {
+ logWindow?.webContents.toggleDevTools();
+ })
+ }Committable suggestion skipped: line range outside the PR's diff.
apps/server-web/src/main/menu.ts
Outdated
| { | ||
| id: 'MENU_APP_DEV', | ||
| label: 'MENU_APP.DEV', | ||
| submenu: [ | ||
| { | ||
| id: 'SUBMENU_SETTING_DEV', | ||
| label: 'MENU_APP.SUBMENU.SETTING_DEV', | ||
| click: () => { | ||
| this.eventEmitter.emit(EventLists.SETTING_WINDOW_DEV); | ||
| }, | ||
| }, | ||
| { | ||
| id: 'SUBMENU_SERVER_DEV', | ||
| label: 'MENU_APP.SUBMENU.SERVER_DEV', | ||
| click: () => { | ||
| this.eventEmitter.emit(EventLists.SERVER_WINDOW_DEV); | ||
| }, | ||
| }, | ||
| ] | ||
| }, |
There was a problem hiding this comment.
Conditionally include 'DEV' menu items based on environment
Including the MENU_APP_DEV menu in production builds may expose development features to end-users, which could be a security risk or cause confusion. Consider conditionally adding this menu only in development mode.
Apply this diff to include 'DEV' menu items only in development:
defaultMenu() {
const isDarwin = process.platform === 'darwin';
+ const isDev = !app.isPackaged;
return [
// ... other menu items
+ ...(isDev ? [{
id: 'MENU_APP_DEV',
label: 'MENU_APP.DEV',
submenu: [
{
id: 'SUBMENU_SETTING_DEV',
label: 'MENU_APP.SUBMENU.SETTING_DEV',
click: () => {
this.eventEmitter.emit(EventLists.SETTING_WINDOW_DEV);
},
},
{
id: 'SUBMENU_SERVER_DEV',
label: 'MENU_APP.SUBMENU.SERVER_DEV',
click: () => {
this.eventEmitter.emit(EventLists.SERVER_WINDOW_DEV);
},
},
]
+ }] : []),
];
}Committable suggestion skipped: line range outside the PR's diff.
|
@syns2191 I am merging this PR now, please create new PR based on |
Description
Please include a summary of the changes and the related issue.
Type of Change
Checklist
Previous screenshots
Please add here videos or images of previous status
Current screenshots
Please add here videos or images of previous status
Summary by CodeRabbit
Release Notes
New Features
IS_DESKTOP_APPto determine if the application is running as a desktop app, affecting authentication logic.Improvements
Bug Fixes