From e994ea716c54d8364bd89573a35a5a065ee94c24 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Thu, 8 Dec 2022 12:31:17 +0100 Subject: [PATCH] fix: menu item enablement issues Signed-off-by: Akos Kitta --- .../src/browser/contributions/contribution.ts | 4 ++ .../src/browser/contributions/examples.ts | 4 -- .../browser/contributions/interface-scale.ts | 71 +++---------------- .../browser/contributions/new-cloud-sketch.ts | 9 +-- .../browser/contributions/open-settings.ts | 3 - .../contributions/upload-certificate.ts | 18 +++-- .../browser/contributions/upload-firmware.ts | 2 + .../browser/contributions/upload-sketch.ts | 2 + .../src/browser/contributions/user-fields.ts | 37 +++------- .../browser/contributions/verify-sketch.ts | 39 ++++++---- .../core/common-frontend-contribution.ts | 3 +- 11 files changed, 64 insertions(+), 128 deletions(-) diff --git a/arduino-ide-extension/src/browser/contributions/contribution.ts b/arduino-ide-extension/src/browser/contributions/contribution.ts index ba57a8587..0a3f93d92 100644 --- a/arduino-ide-extension/src/browser/contributions/contribution.ts +++ b/arduino-ide-extension/src/browser/contributions/contribution.ts @@ -61,6 +61,7 @@ import { BoardsDataStore } from '../boards/boards-data-store'; import { NotificationManager } from '../theia/messages/notifications-manager'; import { MessageType } from '@theia/core/lib/common/message-service-protocol'; import { WorkspaceService } from '../theia/workspace/workspace-service'; +import { MainMenuManager } from '../../common/main-menu-manager'; export { Command, @@ -106,6 +107,9 @@ export abstract class Contribution @inject(FrontendApplicationStateService) protected readonly appStateService: FrontendApplicationStateService; + @inject(MainMenuManager) + protected readonly menuManager: MainMenuManager; + @postConstruct() protected init(): void { this.appStateService.reachedState('ready').then(() => this.onReady()); diff --git a/arduino-ide-extension/src/browser/contributions/examples.ts b/arduino-ide-extension/src/browser/contributions/examples.ts index 1f818a217..b51dddf8c 100644 --- a/arduino-ide-extension/src/browser/contributions/examples.ts +++ b/arduino-ide-extension/src/browser/contributions/examples.ts @@ -12,7 +12,6 @@ import { } from '@theia/core/lib/common/disposable'; import { OpenSketch } from './open-sketch'; import { ArduinoMenus, PlaceholderMenuNode } from '../menu/arduino-menus'; -import { MainMenuManager } from '../../common/main-menu-manager'; import { BoardsServiceProvider } from '../boards/boards-service-provider'; import { ExamplesService } from '../../common/protocol/examples-service'; import { @@ -39,9 +38,6 @@ export abstract class Examples extends SketchContribution { @inject(MenuModelRegistry) private readonly menuRegistry: MenuModelRegistry; - @inject(MainMenuManager) - protected readonly menuManager: MainMenuManager; - @inject(ExamplesService) protected readonly examplesService: ExamplesService; diff --git a/arduino-ide-extension/src/browser/contributions/interface-scale.ts b/arduino-ide-extension/src/browser/contributions/interface-scale.ts index eefd1ab6e..be862ff84 100644 --- a/arduino-ide-extension/src/browser/contributions/interface-scale.ts +++ b/arduino-ide-extension/src/browser/contributions/interface-scale.ts @@ -1,31 +1,17 @@ -import { inject, injectable } from '@theia/core/shared/inversify'; +import { injectable } from '@theia/core/shared/inversify'; import { Contribution, Command, MenuModelRegistry, KeybindingRegistry, } from './contribution'; -import { ArduinoMenus, PlaceholderMenuNode } from '../menu/arduino-menus'; -import { - CommandRegistry, - DisposableCollection, - MaybePromise, - nls, -} from '@theia/core/lib/common'; - +import { ArduinoMenus } from '../menu/arduino-menus'; +import { CommandRegistry, MaybePromise, nls } from '@theia/core/lib/common'; import { Settings } from '../dialogs/settings/settings'; -import { MainMenuManager } from '../../common/main-menu-manager'; import debounce = require('lodash.debounce'); @injectable() export class InterfaceScale extends Contribution { - @inject(MenuModelRegistry) - private readonly menuRegistry: MenuModelRegistry; - - @inject(MainMenuManager) - private readonly mainMenuManager: MainMenuManager; - - private readonly menuActionsDisposables = new DisposableCollection(); private fontScalingEnabled: InterfaceScale.FontScalingEnabled = { increase: true, decrease: true, @@ -62,63 +48,22 @@ export class InterfaceScale extends Contribution { } override registerMenus(registry: MenuModelRegistry): void { - this.menuActionsDisposables.dispose(); - const increaseFontSizeMenuAction = { + registry.registerMenuAction(ArduinoMenus.EDIT__FONT_CONTROL_GROUP, { commandId: InterfaceScale.Commands.INCREASE_FONT_SIZE.id, label: nls.localize( 'arduino/editor/increaseFontSize', 'Increase Font Size' ), order: '0', - }; - const decreaseFontSizeMenuAction = { + }); + registry.registerMenuAction(ArduinoMenus.EDIT__FONT_CONTROL_GROUP, { commandId: InterfaceScale.Commands.DECREASE_FONT_SIZE.id, label: nls.localize( 'arduino/editor/decreaseFontSize', 'Decrease Font Size' ), order: '1', - }; - - if (this.fontScalingEnabled.increase) { - this.menuActionsDisposables.push( - registry.registerMenuAction( - ArduinoMenus.EDIT__FONT_CONTROL_GROUP, - increaseFontSizeMenuAction - ) - ); - } else { - this.menuActionsDisposables.push( - registry.registerMenuNode( - ArduinoMenus.EDIT__FONT_CONTROL_GROUP, - new PlaceholderMenuNode( - ArduinoMenus.EDIT__FONT_CONTROL_GROUP, - increaseFontSizeMenuAction.label, - { order: increaseFontSizeMenuAction.order } - ) - ) - ); - } - if (this.fontScalingEnabled.decrease) { - this.menuActionsDisposables.push( - this.menuRegistry.registerMenuAction( - ArduinoMenus.EDIT__FONT_CONTROL_GROUP, - decreaseFontSizeMenuAction - ) - ); - } else { - this.menuActionsDisposables.push( - this.menuRegistry.registerMenuNode( - ArduinoMenus.EDIT__FONT_CONTROL_GROUP, - new PlaceholderMenuNode( - ArduinoMenus.EDIT__FONT_CONTROL_GROUP, - decreaseFontSizeMenuAction.label, - { order: decreaseFontSizeMenuAction.order } - ) - ) - ); - } - this.mainMenuManager.update(); + }); } private updateFontScalingEnabled(): void { @@ -153,7 +98,7 @@ export class InterfaceScale extends Contribution { ); if (isChanged) { this.fontScalingEnabled = fontScalingEnabled; - this.registerMenus(this.menuRegistry); + this.menuManager.update(); } } diff --git a/arduino-ide-extension/src/browser/contributions/new-cloud-sketch.ts b/arduino-ide-extension/src/browser/contributions/new-cloud-sketch.ts index 430821c03..55fabeaca 100644 --- a/arduino-ide-extension/src/browser/contributions/new-cloud-sketch.ts +++ b/arduino-ide-extension/src/browser/contributions/new-cloud-sketch.ts @@ -17,7 +17,6 @@ import { nls } from '@theia/core/lib/common/nls'; import { inject, injectable } from '@theia/core/shared/inversify'; import { WorkspaceInputDialogProps } from '@theia/workspace/lib/browser/workspace-input-dialog'; import { v4 } from 'uuid'; -import { MainMenuManager } from '../../common/main-menu-manager'; import type { AuthenticationSession } from '../../node/auth/types'; import { AuthenticationClientService } from '../auth/authentication-client-service'; import { CreateApi } from '../create/create-api'; @@ -41,8 +40,6 @@ export class NewCloudSketch extends Contribution { private readonly widgetContribution: SketchbookWidgetContribution; @inject(AuthenticationClientService) private readonly authenticationService: AuthenticationClientService; - @inject(MainMenuManager) - private readonly mainMenuManager: MainMenuManager; private readonly toDispose = new DisposableCollection(); private _session: AuthenticationSession | undefined; @@ -54,7 +51,7 @@ export class NewCloudSketch extends Contribution { const oldSession = this._session; this._session = session; if (!!oldSession !== !!this._session) { - this.mainMenuManager.update(); + this.menuManager.update(); } }), this.preferences.onPreferenceChanged(({ preferenceName, newValue }) => { @@ -62,7 +59,7 @@ export class NewCloudSketch extends Contribution { const oldEnabled = this._enabled; this._enabled = Boolean(newValue); if (this._enabled !== oldEnabled) { - this.mainMenuManager.update(); + this.menuManager.update(); } } }), @@ -70,7 +67,7 @@ export class NewCloudSketch extends Contribution { this._enabled = this.preferences['arduino.cloud.enabled']; this._session = this.authenticationService.session; if (this._session) { - this.mainMenuManager.update(); + this.menuManager.update(); } } diff --git a/arduino-ide-extension/src/browser/contributions/open-settings.ts b/arduino-ide-extension/src/browser/contributions/open-settings.ts index 0cbab70c0..e3dff44df 100644 --- a/arduino-ide-extension/src/browser/contributions/open-settings.ts +++ b/arduino-ide-extension/src/browser/contributions/open-settings.ts @@ -1,6 +1,5 @@ import { nls } from '@theia/core/lib/common/nls'; import { inject, injectable } from '@theia/core/shared/inversify'; -import { MainMenuManager } from '../../common/main-menu-manager'; import type { Settings } from '../dialogs/settings/settings'; import { SettingsDialog } from '../dialogs/settings/settings-dialog'; import { ArduinoMenus } from '../menu/arduino-menus'; @@ -16,8 +15,6 @@ import { export class OpenSettings extends SketchContribution { @inject(SettingsDialog) private readonly settingsDialog: SettingsDialog; - @inject(MainMenuManager) - private readonly menuManager: MainMenuManager; private settingsOpened = false; diff --git a/arduino-ide-extension/src/browser/contributions/upload-certificate.ts b/arduino-ide-extension/src/browser/contributions/upload-certificate.ts index 91292eb49..7e1400932 100644 --- a/arduino-ide-extension/src/browser/contributions/upload-certificate.ts +++ b/arduino-ide-extension/src/browser/contributions/upload-certificate.ts @@ -12,7 +12,6 @@ import { PreferenceScope, PreferenceService, } from '@theia/core/lib/browser/preferences/preference-service'; -import { ArduinoPreferences } from '../arduino-preferences'; import { arduinoCert, certificateList, @@ -31,22 +30,29 @@ export class UploadCertificate extends Contribution { @inject(PreferenceService) protected readonly preferenceService: PreferenceService; - @inject(ArduinoPreferences) - protected readonly arduinoPreferences: ArduinoPreferences; - @inject(ArduinoFirmwareUploader) protected readonly arduinoFirmwareUploader: ArduinoFirmwareUploader; protected dialogOpened = false; + override onStart(): void { + this.preferences.onPreferenceChanged(({ preferenceName }) => { + if (preferenceName === 'arduino.board.certificates') { + this.menuManager.update(); + } + }); + } + override registerCommands(registry: CommandRegistry): void { registry.registerCommand(UploadCertificate.Commands.OPEN, { execute: async () => { try { this.dialogOpened = true; + this.menuManager.update(); await this.dialog.open(); } finally { this.dialogOpened = false; + this.menuManager.update(); } }, isEnabled: () => !this.dialogOpened, @@ -54,7 +60,7 @@ export class UploadCertificate extends Contribution { registry.registerCommand(UploadCertificate.Commands.REMOVE_CERT, { execute: async (certToRemove) => { - const certs = this.arduinoPreferences.get('arduino.board.certificates'); + const certs = this.preferences.get('arduino.board.certificates'); this.preferenceService.set( 'arduino.board.certificates', @@ -75,7 +81,6 @@ export class UploadCertificate extends Contribution { .join(' ')}` ); }, - isEnabled: () => true, }); registry.registerCommand(UploadCertificate.Commands.OPEN_CERT_CONTEXT, { @@ -89,7 +94,6 @@ export class UploadCertificate extends Contribution { args: [args.cert], }); }, - isEnabled: () => true, }); } diff --git a/arduino-ide-extension/src/browser/contributions/upload-firmware.ts b/arduino-ide-extension/src/browser/contributions/upload-firmware.ts index 6fc566904..cd6afb9cb 100644 --- a/arduino-ide-extension/src/browser/contributions/upload-firmware.ts +++ b/arduino-ide-extension/src/browser/contributions/upload-firmware.ts @@ -21,9 +21,11 @@ export class UploadFirmware extends Contribution { execute: async () => { try { this.dialogOpened = true; + this.menuManager.update(); await this.dialog.open(); } finally { this.dialogOpened = false; + this.menuManager.update(); } }, isEnabled: () => !this.dialogOpened, diff --git a/arduino-ide-extension/src/browser/contributions/upload-sketch.ts b/arduino-ide-extension/src/browser/contributions/upload-sketch.ts index 2868d341c..735379d84 100644 --- a/arduino-ide-extension/src/browser/contributions/upload-sketch.ts +++ b/arduino-ide-extension/src/browser/contributions/upload-sketch.ts @@ -106,6 +106,7 @@ export class UploadSketch extends CoreServiceContribution { // toggle the toolbar button and menu item state. // uploadInProgress will be set to false whether the upload fails or not this.uploadInProgress = true; + this.menuManager.update(); this.boardsServiceProvider.snapshotBoardDiscoveryOnUpload(); this.onDidChangeEmitter.fire(); this.clearVisibleNotification(); @@ -150,6 +151,7 @@ export class UploadSketch extends CoreServiceContribution { this.handleError(e); } finally { this.uploadInProgress = false; + this.menuManager.update(); this.boardsServiceProvider.attemptPostUploadAutoSelect(); this.onDidChangeEmitter.fire(); } diff --git a/arduino-ide-extension/src/browser/contributions/user-fields.ts b/arduino-ide-extension/src/browser/contributions/user-fields.ts index 195ab07da..62bef9748 100644 --- a/arduino-ide-extension/src/browser/contributions/user-fields.ts +++ b/arduino-ide-extension/src/browser/contributions/user-fields.ts @@ -1,9 +1,9 @@ import { inject, injectable } from '@theia/core/shared/inversify'; -import { DisposableCollection, nls } from '@theia/core/lib/common'; +import { nls } from '@theia/core/lib/common'; import { BoardUserField, CoreError } from '../../common/protocol'; import { BoardsServiceProvider } from '../boards/boards-service-provider'; import { UserFieldsDialog } from '../dialogs/user-fields/user-fields-dialog'; -import { ArduinoMenus, PlaceholderMenuNode } from '../menu/arduino-menus'; +import { ArduinoMenus } from '../menu/arduino-menus'; import { MenuModelRegistry, Contribution } from './contribution'; import { UploadSketch } from './upload-sketch'; @@ -12,7 +12,6 @@ export class UserFields extends Contribution { private boardRequiresUserFields = false; private userFieldsSet = false; private readonly cachedUserFields: Map = new Map(); - private readonly menuActionsDisposables = new DisposableCollection(); @inject(UserFieldsDialog) private readonly userFieldsDialog: UserFieldsDialog; @@ -20,42 +19,22 @@ export class UserFields extends Contribution { @inject(BoardsServiceProvider) private readonly boardsServiceProvider: BoardsServiceProvider; - @inject(MenuModelRegistry) - private readonly menuRegistry: MenuModelRegistry; - protected override init(): void { super.init(); this.boardsServiceProvider.onBoardsConfigChanged(async () => { const userFields = await this.boardsServiceProvider.selectedBoardUserFields(); this.boardRequiresUserFields = userFields.length > 0; - this.registerMenus(this.menuRegistry); + this.menuManager.update(); }); } override registerMenus(registry: MenuModelRegistry): void { - this.menuActionsDisposables.dispose(); - if (this.boardRequiresUserFields) { - this.menuActionsDisposables.push( - registry.registerMenuAction(ArduinoMenus.SKETCH__MAIN_GROUP, { - commandId: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.id, - label: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.label, - order: '2', - }) - ); - } else { - this.menuActionsDisposables.push( - registry.registerMenuNode( - ArduinoMenus.SKETCH__MAIN_GROUP, - new PlaceholderMenuNode( - ArduinoMenus.SKETCH__MAIN_GROUP, - // commandId: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.id, - UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.label, - { order: '2' } - ) - ) - ); - } + registry.registerMenuAction(ArduinoMenus.SKETCH__MAIN_GROUP, { + commandId: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.id, + label: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.label, + order: '2', + }); } private selectedFqbnAddress(): string | undefined { diff --git a/arduino-ide-extension/src/browser/contributions/verify-sketch.ts b/arduino-ide-extension/src/browser/contributions/verify-sketch.ts index fe4de6f6a..b616d6169 100644 --- a/arduino-ide-extension/src/browser/contributions/verify-sketch.ts +++ b/arduino-ide-extension/src/browser/contributions/verify-sketch.ts @@ -21,11 +21,18 @@ export interface VerifySketchParams { */ readonly exportBinaries?: boolean; /** - * If `true`, there won't be any UI indication of the verify command. It's `false` by default. + * If `true`, there won't be any UI indication of the verify command in the toolbar. It's `false` by default. */ readonly silent?: boolean; } +/** + * - `"idle"` when neither verify, not upload is running, + * - `"explicit-verify"` when only verify is running triggered by the user, and + * - `"automatic-verify"` is when the automatic verify phase is running as part of an upload triggered by the user. + */ +type VerifyProgress = 'idle' | 'explicit-verify' | 'automatic-verify'; + @injectable() export class VerifySketch extends CoreServiceContribution { @inject(CoreErrorHandler) @@ -33,22 +40,24 @@ export class VerifySketch extends CoreServiceContribution { private readonly onDidChangeEmitter = new Emitter(); private readonly onDidChange = this.onDidChangeEmitter.event; - private verifyInProgress = false; + private verifyProgress: VerifyProgress = 'idle'; override registerCommands(registry: CommandRegistry): void { registry.registerCommand(VerifySketch.Commands.VERIFY_SKETCH, { execute: (params?: VerifySketchParams) => this.verifySketch(params), - isEnabled: () => !this.verifyInProgress, + isEnabled: () => this.verifyProgress === 'idle', }); registry.registerCommand(VerifySketch.Commands.EXPORT_BINARIES, { execute: () => this.verifySketch({ exportBinaries: true }), - isEnabled: () => !this.verifyInProgress, + isEnabled: () => this.verifyProgress === 'idle', }); registry.registerCommand(VerifySketch.Commands.VERIFY_SKETCH_TOOLBAR, { isVisible: (widget) => ArduinoToolbar.is(widget) && widget.side === 'left', - isEnabled: () => !this.verifyInProgress, - isToggled: () => this.verifyInProgress, + isEnabled: () => this.verifyProgress !== 'explicit-verify', + // toggled only when verify is running, but not toggled when automatic verify is running before the upload + // https://github.com/arduino/arduino-ide/pull/1750#pullrequestreview-1214762975 + isToggled: () => this.verifyProgress === 'explicit-verify', execute: () => registry.executeCommand(VerifySketch.Commands.VERIFY_SKETCH.id), }); @@ -99,15 +108,16 @@ export class VerifySketch extends CoreServiceContribution { private async verifySketch( params?: VerifySketchParams ): Promise { - if (this.verifyInProgress) { + if (this.verifyProgress !== 'idle') { return undefined; } try { - if (!params?.silent) { - this.verifyInProgress = true; - this.onDidChangeEmitter.fire(); - } + this.verifyProgress = params?.silent + ? 'automatic-verify' + : 'explicit-verify'; + this.onDidChangeEmitter.fire(); + this.menuManager.update(); this.clearVisibleNotification(); this.coreErrorHandler.reset(); @@ -139,10 +149,9 @@ export class VerifySketch extends CoreServiceContribution { this.handleError(e); return undefined; } finally { - this.verifyInProgress = false; - if (!params?.silent) { - this.onDidChangeEmitter.fire(); - } + this.verifyProgress = 'idle'; + this.onDidChangeEmitter.fire(); + this.menuManager.update(); } } diff --git a/arduino-ide-extension/src/browser/theia/core/common-frontend-contribution.ts b/arduino-ide-extension/src/browser/theia/core/common-frontend-contribution.ts index 29d092017..3444f0ce1 100644 --- a/arduino-ide-extension/src/browser/theia/core/common-frontend-contribution.ts +++ b/arduino-ide-extension/src/browser/theia/core/common-frontend-contribution.ts @@ -46,7 +46,8 @@ export class CommonFrontendContribution extends TheiaCommonFrontendContribution CommonCommands.SELECT_ICON_THEME, CommonCommands.SELECT_COLOR_THEME, CommonCommands.ABOUT_COMMAND, - CommonCommands.SAVE_WITHOUT_FORMATTING, // Patched for https://github.com/eclipse-theia/theia/pull/8877 + CommonCommands.SAVE_WITHOUT_FORMATTING, // Patched for https://github.com/eclipse-theia/theia/pull/8877, + CommonCommands.NEW_UNTITLED_FILE, ]) { registry.unregisterMenuAction(command); }