From 1ea3ec19a8759cd04e4e09e0f9f9e0a5cb50dc35 Mon Sep 17 00:00:00 2001 From: ChayaLau Date: Thu, 5 Aug 2021 16:41:06 +0300 Subject: [PATCH] fixed PR according to reviewer comments --- CHANGELOG.md | 4 ++ .../browser/common-frontend-contribution.ts | 64 ++++++++++--------- packages/core/src/browser/core-preferences.ts | 4 +- .../src/browser/menu/browser-menu-plugin.ts | 3 - packages/core/src/browser/style/sidepanel.css | 2 +- .../menu/electron-main-menu-factory.ts | 5 +- 6 files changed, 42 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53b42736e9425..17a7965a22dae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,10 @@ - [debug] `DebugSession` and `PluginDebugSession` constructors accept a `parentSession` of type `DebugSession | undefined` as their 3rd parameter, offsetting every subsequent parameter by one [#9613](https://github.com/eclipse-theia/theia/pull/9613) - [monaco] upgraded to monaco 0.23.0 including replacement of `quickOpen` API (0.20.x) with `quickInput` API (0.23.x) [#9154](https://github.com/eclipse-theia/theia/pull/9154) - [workspace] `WorkspaceCommandContribution.addFolderToWorkspace` no longer accepts `undefined`. `WorkspaceService.addRoot` now accepts a `URI` or a `URI[]` [#9684](https://github.com/eclipse-theia/theia/pull/9684) +-[core] `SidePanelHandler.addMenu` and `SidePanelHandler.removeMenu` no longer exists, instead added `addBottomMenu` and `addTopMenu` for adding menu, `removeTopMenu` and `removeBottomMenu` for removing menu. +`SidebarBottomMenu` interface is renamed `SidebarMenu` and handles not only bottom menu's. +`packages/core/src/browser/shell/sidebar-bottom-menu-widget.tsx` file is renamed `packages/core/src/browser/shell/sidebar-menu-widget.tsx`. `SidebarBottomMenuWidget` class is renamed `SidebarMenuWidget`. + [#9830](https://github.com/eclipse-theia/theia/pull/9830) ## v1.15.0 - 6/30/2021 diff --git a/packages/core/src/browser/common-frontend-contribution.ts b/packages/core/src/browser/common-frontend-contribution.ts index d764ca7dc9c81..4b9a7c58c5d8e 100644 --- a/packages/core/src/browser/common-frontend-contribution.ts +++ b/packages/core/src/browser/common-frontend-contribution.ts @@ -43,9 +43,9 @@ import { environment } from '@theia/application-package/lib/environment'; import { IconThemeService } from './icon-theme-service'; import { ColorContribution } from './color-application-contribution'; import { ColorRegistry, Color } from './color-registry'; -import { CorePreferences } from './core-preferences'; +import { CoreConfiguration, CorePreferences } from './core-preferences'; import { ThemeService } from './theming'; -import { PreferenceService, PreferenceScope } from './preferences'; +import { PreferenceService, PreferenceScope, PreferenceChangeEvent } from './preferences'; import { ClipboardService } from './clipboard-service'; import { EncodingRegistry } from './encoding-registry'; import { UTF8 } from '../common/encodings'; @@ -353,35 +353,7 @@ export class CommonFrontendContribution implements FrontendApplicationContributi this.updateStyles(); this.updateThemeFromPreference('workbench.colorTheme'); this.updateThemeFromPreference('workbench.iconTheme'); - this.preferences.onPreferenceChanged(e => { - switch (e.preferenceName) { - case 'workbench.editor.highlightModifiedTabs': { - this.updateStyles(); - break; - } - case 'workbench.colorTheme': - case 'workbench.iconTheme': { - this.updateThemeFromPreference(e.preferenceName); - break; - } - case 'window.menuBarVisibility': { - const { newValue } = e; - const mainMenuId = 'main-menu'; - if (newValue === 'compact') { - app.shell.leftPanelHandler.addTopMenu({ - id: mainMenuId, - iconClass: 'codicon codicon-menu', - title: 'Application Menu', - menuPath: ['menubar'], - order: 0, - }); - } else { - app.shell.leftPanelHandler.removeTopMenu(mainMenuId); - } - break; - } - } - }); + this.preferences.onPreferenceChanged(e => this.handlePreferenceChange(e, app)); this.themeService.onDidColorThemeChange(() => this.updateThemePreference('workbench.colorTheme')); this.iconThemes.onDidChangeCurrent(() => this.updateThemePreference('workbench.iconTheme')); @@ -442,6 +414,36 @@ export class CommonFrontendContribution implements FrontendApplicationContributi } } + protected handlePreferenceChange(e: PreferenceChangeEvent, app: FrontendApplication): void { + switch (e.preferenceName) { + case 'workbench.editor.highlightModifiedTabs': { + this.updateStyles(); + break; + } + case 'workbench.colorTheme': + case 'workbench.iconTheme': { + this.updateThemeFromPreference(e.preferenceName); + break; + } + case 'window.menuBarVisibility': { + const { newValue } = e; + const mainMenuId = 'main-menu'; + if (newValue === 'compact') { + app.shell.leftPanelHandler.addTopMenu({ + id: mainMenuId, + iconClass: 'codicon codicon-menu', + title: 'Application Menu', + menuPath: ['menubar'], + order: 0, + }); + } else { + app.shell.leftPanelHandler.removeTopMenu(mainMenuId); + } + break; + } + } + } + onStart(): void { this.storageService.getData<{ recent: Command[] }>(RECENT_COMMANDS_STORAGE_KEY, { recent: [] }) .then(tasks => this.commandRegistry.recent = tasks.recent); diff --git a/packages/core/src/browser/core-preferences.ts b/packages/core/src/browser/core-preferences.ts index 1505725da3f50..64a853938b3ef 100644 --- a/packages/core/src/browser/core-preferences.ts +++ b/packages/core/src/browser/core-preferences.ts @@ -104,11 +104,11 @@ export const corePreferenceSchema: PreferenceSchema = { 'Menu is displayed at the top of the window and only hidden in full screen mode.', 'Menu is always visible at the top of the window even in full screen mode.', 'Menu is always hidden.', - 'Menu is displayed as a compact button in the sidebar. This value is ignored when `#window.titleBarStyle#` is `native`.' + 'Menu is displayed as a compact button in the sidebar.' ], default: 'classic', scope: 'application', - markdownDescription: `Control the visibility of the menu bar. A setting of 'toggle' means that the menu bar is hidden and a single press of the Alt key will show it. + markdownDescription: `Control the visibility of the menu bar. A setting of 'compact' will move the menu into the sidebar.`, }, } diff --git a/packages/core/src/browser/menu/browser-menu-plugin.ts b/packages/core/src/browser/menu/browser-menu-plugin.ts index f7990bb20e5fa..3549ba9b66a5f 100644 --- a/packages/core/src/browser/menu/browser-menu-plugin.ts +++ b/packages/core/src/browser/menu/browser-menu-plugin.ts @@ -368,9 +368,6 @@ export class BrowserMenuBarContribution implements FrontendApplicationContributi @inject(ApplicationShell) protected readonly shell: ApplicationShell; - @inject(CorePreferences) - protected readonly corePreferences: CorePreferences; - constructor( @inject(BrowserMainMenuFactory) protected readonly factory: BrowserMainMenuFactory ) { } diff --git a/packages/core/src/browser/style/sidepanel.css b/packages/core/src/browser/style/sidepanel.css index 1ccdc62b838cd..9bf6061c0d406 100644 --- a/packages/core/src/browser/style/sidepanel.css +++ b/packages/core/src/browser/style/sidepanel.css @@ -211,7 +211,7 @@ color: var(--theia-activityBar-foreground); } -.theia-sidebar-menu > i.codicon-menu{ +.theia-sidebar-menu > i.codicon-menu { font-size: calc(var(--theia-private-sidebar-icon-size)*0.7); } diff --git a/packages/core/src/electron-browser/menu/electron-main-menu-factory.ts b/packages/core/src/electron-browser/menu/electron-main-menu-factory.ts index 74625511f0976..ef37c832d3ee0 100644 --- a/packages/core/src/electron-browser/menu/electron-main-menu-factory.ts +++ b/packages/core/src/electron-browser/menu/electron-main-menu-factory.ts @@ -99,9 +99,8 @@ export class ElectronMainMenuFactory { } createMenuBar(): Electron.Menu | null { - let preference = this.preferencesService.get('window.menuBarVisibility'); - preference = preference ? preference : 'classic'; - if (preference && ['classic', 'visible'].includes(preference)) { + const preference = this.preferencesService.get('window.menuBarVisibility') || 'classic'; + if (['classic', 'visible'].includes(preference)) { const menuModel = this.menuProvider.getMenu(MAIN_MENU_BAR); const template = this.fillMenuTemplate([], menuModel); if (isOSX) {