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

Fix/enable edit menu server web #3387

Merged
merged 2 commits into from
Dec 4, 2024
Merged

Fix/enable edit menu server web #3387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd55260
enable edit menu and switch theme on setup
syns2191 Dec 1, 2024
ebe2aee
fix: naming interface runtime server config
syns2191 Dec 1, 2024
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
9 changes: 4 additions & 5 deletions apps/server-web/src/main/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,7 @@ const createWindow = async (type: 'SETTING_WINDOW' | 'LOG_WINDOW' | 'SETUP_WINDO
url = resolveHtmlPath('index.html', 'setup');
setupWindow?.loadURL(url);
mainBindings(ipcMain, setupWindow, fs);
if (process.platform === 'darwin') {
Menu.setApplicationMenu(Menu.buildFromTemplate([]));
} else {
setupWindow.removeMenu();
}
Menu.setApplicationMenu(appMenu.buildDefaultTemplate(appMenu.initialMenu(), i18nextMainBackend));
setupWindow.on('closed', () => {
setupWindow = null;
})
Expand Down Expand Up @@ -270,6 +266,8 @@ const onInitApplication = () => {
trayMenuItems = trayMenuItems.length ? trayMenuItems : defaultTrayMenuItem(eventEmitter);
updateTrayMenu('none', {}, eventEmitter, tray, trayMenuItems, i18nextMainBackend);
Menu.setApplicationMenu(appMenu.buildDefaultTemplate(appMenuItems, i18nextMainBackend))
} else {
Menu.setApplicationMenu(appMenu.buildDefaultTemplate(appMenu.initialMenu(), i18nextMainBackend))
}
}, 250));

Expand Down Expand Up @@ -378,6 +376,7 @@ const onInitApplication = () => {
})
logWindow?.webContents.send('themeSignal', { type: SettingPageTypeMessage.themeChange, data });
settingWindow?.webContents.send('themeSignal', { type: SettingPageTypeMessage.themeChange, data });
setupWindow?.webContents.send('themeSignal', { type: SettingPageTypeMessage.themeChange, data });
})

eventEmitter.on(EventLists.gotoAbout, async () => {
Expand Down
25 changes: 24 additions & 1 deletion apps/server-web/src/main/menu.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export default class MenuBuilder {
this.eventEmitter = eventEmitter
}

defaultMenu(): AppMenu[] {
initialMenu(): AppMenu[] {
const isDarwin = process.platform === 'darwin';
return [
{
Expand All @@ -41,6 +41,25 @@ export default class MenuBuilder {
},
],
},
{
id: 'MENU_APP_EDIT',
label: 'MENU_APP.APP_EDIT',
submenu: [
{ label: 'MENU_APP.APP_SUBMENU.APP_UNDO', accelerator: "CmdOrCtrl+Z", role: "undo" },
{ label: "MENU_APP.APP_SUBMENU.APP_REDO", accelerator: "Shift+CmdOrCtrl+Z", role: "redo" },
{ type: "separator" },
{ label: "MENU_APP.APP_SUBMENU.APP_CUT", accelerator: "CmdOrCtrl+X", role: "cut" },
{ label: "MENU_APP.APP_SUBMENU.APP_COPY", accelerator: "CmdOrCtrl+C", role: "copy" },
{ label: "MENU_APP.APP_SUBMENU.APP_PASTE", accelerator: "CmdOrCtrl+V", role: "paste" },
{ label: "MENU_APP.APP_SUBMENU.APP_SELECT_ALL", accelerator: "CmdOrCtrl+A", role: "selectAll" }
]
}
]
}

defaultMenu(): AppMenu[] {
return [
...this.initialMenu(),
{
id: 'MENU_APP_WINDOW',
label: 'MENU_APP.APP_WINDOW',
Expand Down Expand Up @@ -110,6 +129,10 @@ export default class MenuBuilder {
return Menu.buildFromTemplate(this.translateAppMenu(i18nextMainBackend, menuItems));
}

buildInitialTemplate(menuItems: any, i18nextMainBackend: typeof i18n) {
return Menu.buildFromTemplate(this.translateAppMenu(i18nextMainBackend, menuItems));
}
Comment on lines +132 to +134
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove duplicate method implementation

buildInitialTemplate appears to be identical to buildDefaultTemplate. Consider removing this duplication and reusing buildDefaultTemplate instead.

-buildInitialTemplate(menuItems: any, i18nextMainBackend: typeof i18n) {
-  return Menu.buildFromTemplate(this.translateAppMenu(i18nextMainBackend, menuItems));
-}

Committable suggestion skipped: line range outside the PR's 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) {
Expand Down
2 changes: 1 addition & 1 deletion apps/server-web/src/renderer/components/Select.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const SelectComponent = ({
onValueChange={onValueChange}
>
<Select.Trigger
className="inline-flex items-center justify-center rounded-lg px-[15px] text-[13px] leading-none h-[35px] gap-[5px] bg-white dark:bg-[#25272D] text-violet11 dark:text-white shadow-[0_2px_10px] shadow-black/10 hover:bg-mauve3 focus:shadow-[0_0_0_2px] focus:shadow-black data-[placeholder]:text-violet9 outline-none"
className="inline-flex items-center justify-center rounded-lg px-[15px] text-[13px] leading-none h-[35px] gap-[5px] bg-white dark:bg-[#25272D] text-black dark:text-white shadow-[0_2px_10px] shadow-black/10 hover:bg-mauve3 focus:shadow-[0_0_0_2px] focus:shadow-black data-[placeholder]:text-violet9 outline-none"
aria-label="Food"
>
<Select.Value placeholder={title} />
Expand Down
4 changes: 2 additions & 2 deletions apps/server-web/src/renderer/components/Updater.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export const UpdaterComponent = (props: IUpdaterComponent) => {
setToastShow(false);
};

const onSelectPeriode = (value: string) => {
const onSelectPeriod = (value: string) => {
props.changeAutoUpdate({
autoUpdate: props.data.autoUpdate,
updateCheckPeriod: value,
Expand Down Expand Up @@ -152,7 +152,7 @@ export const UpdaterComponent = (props: IUpdaterComponent) => {
value={props.data.updateCheckPeriod}
defaultValue={props.data.updateCheckPeriod}
disabled={!props.data.autoUpdate}
onValueChange={onSelectPeriode}
onValueChange={onSelectPeriod}
/>
</div>
</div>
Expand Down
14 changes: 12 additions & 2 deletions apps/server-web/src/renderer/pages/setup/Landing.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { config } from '../../../configs/config';
import { useTranslation } from 'react-i18next';
import LanguageSelector from '../../components/LanguageSelector';
import { useEffect, useState } from 'react';
import { ThemeToggler } from '../../components/Toggler';
type props = {
nextAction: () => void;
};
Expand All @@ -25,8 +26,17 @@ const Landing = (props: props) => {
}, []);
return (
<div className="w-full">
<div className="mb-6 ml-10">
<LanguageSelector lang={defaultLang} />
<div className="flex w-full mb-6 ml-10">
<div className="flex flex-col w-6/12">
<div>
<LanguageSelector lang={defaultLang} />
</div>
</div>
<div className="flex w-6/12 flex-row-reverse mr-10">
<div className="flex flex-col w-2/8">
<ThemeToggler />
</div>
</div>
Comment on lines +29 to +39
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve layout structure and accessibility

The current implementation has several issues that should be addressed:

  1. The width class w-2/8 is invalid in Tailwind CSS
  2. The nested structure could be simplified
  3. Missing accessibility attributes for language and theme controls

Consider applying these improvements:

-      <div className="flex w-full mb-6 ml-10">
-        <div className="flex flex-col w-6/12">
-          <div>
-            <LanguageSelector lang={defaultLang} />
-          </div>
-        </div>
-        <div className="flex w-6/12 flex-row-reverse mr-10">
-          <div className="flex flex-col w-2/8">
-            <ThemeToggler />
-          </div>
-        </div>
+      <nav className="flex justify-between items-center w-full px-10 mb-6" aria-label="Site settings">
+        <div className="flex-shrink-0" role="region" aria-label="Language selection">
+          <LanguageSelector lang={defaultLang} />
+        </div>
+        <div className="flex-shrink-0" role="region" aria-label="Theme selection">
+          <ThemeToggler />
+        </div>
+      </nav>

Changes made:

  • Replaced invalid w-2/8 with a more flexible layout using flex-shrink-0
  • Simplified the nested structure
  • Added semantic nav element and ARIA labels
  • Used consistent padding with px-10 instead of ml-10 and mr-10
📝 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.

Suggested change
<div className="flex w-full mb-6 ml-10">
<div className="flex flex-col w-6/12">
<div>
<LanguageSelector lang={defaultLang} />
</div>
</div>
<div className="flex w-6/12 flex-row-reverse mr-10">
<div className="flex flex-col w-2/8">
<ThemeToggler />
</div>
</div>
<nav className="flex justify-between items-center w-full px-10 mb-6" aria-label="Site settings">
<div className="flex-shrink-0" role="region" aria-label="Language selection">
<LanguageSelector lang={defaultLang} />
</div>
<div className="flex-shrink-0" role="region" aria-label="Theme selection">
<ThemeToggler />
</div>
</nav>

</div>

<div className="mb-8 w-full text-center">
Expand Down
2 changes: 1 addition & 1 deletion apps/web/app/interfaces/IRuntimeServerConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export interface ServerRuntimeConfig {
export interface IServerRuntimeConfig {
GAUZY_API_SERVER_URL?: string;
NEXT_PUBLIC_GAUZY_API_SERVER_URL?: string;
[key: string]: any;
Expand Down
4 changes: 2 additions & 2 deletions apps/web/app/services/server/requests/desktop-source.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import getConfig from 'next/config';
import { ServerRuntimeConfig } from '@app/interfaces/IRuntimeServerConfig';
import { IServerRuntimeConfig } from '@app/interfaces/IRuntimeServerConfig';
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Interface renaming needs to be completed across the codebase

The verification reveals inconsistent interface naming:

  • The interface has been renamed to IServerRuntimeConfig in IRuntimeServerConfig.ts
  • However, getServerRuntimeConfig() function in env.ts and its usage in constants.ts still use the old naming convention
  • This creates a mix of old and new naming patterns that should be standardized

Locations needing updates:

  • apps/web/app/env.ts: Function name should be updated to getIServerRuntimeConfig()
  • apps/web/app/constants.ts: Variable name should be updated to match the new interface naming
🔗 Analysis chain

Verify consistent interface renaming across the codebase

The interface renaming from ServerRuntimeConfig to IServerRuntimeConfig follows TypeScript naming conventions. Let's ensure this change is consistent throughout the codebase.

Let me analyze the results further by checking the implementation details of these files.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining occurrences of the old interface name
# and verify the new interface is used consistently

# Search for any remaining instances of the old interface name
echo "Checking for remaining instances of ServerRuntimeConfig..."
rg "ServerRuntimeConfig" --type ts

# Verify the new interface usage
echo "Verifying IServerRuntimeConfig usage..."
rg "IServerRuntimeConfig" --type ts

Length of output: 1213

Script:

#!/bin/bash
# Check the implementation of getServerRuntimeConfig and its usage
echo "Checking env.ts implementation..."
cat apps/web/app/env.ts

echo -e "\nChecking constants.ts implementation..."
cat apps/web/app/constants.ts

echo -e "\nChecking IRuntimeServerConfig interface..."
cat apps/web/app/interfaces/IRuntimeServerConfig.ts

Length of output: 12996

import { GAUZY_API_SERVER_URL, GAUZY_API_BASE_SERVER_URL } from '@app/constants';

export function getDesktopConfig(): Partial<ServerRuntimeConfig> {
export function getDesktopConfig(): Partial<IServerRuntimeConfig> {
try {
const { serverRuntimeConfig } = getConfig();
return serverRuntimeConfig;
Expand Down