From b62f3dec84a677af9dad7b66c0516905d62f3803 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Fri, 22 Jul 2022 12:02:31 +0200 Subject: [PATCH] #714: UX improvements of the Arduino LS in IDE2 - Debounced the connectivity status update. - Silent the output channel for the Arduino LS. - Delay the problem markers update with 500ms. - Do not update the status bar on every `keypress` event. - Debounced the tab-bar toolbar updates when typing in editor. - Fixed electron menu contribution binding. - Aligned the editor widget factory's API to Theia. - Set the zoom level when the app is ready (Closes #1244) - Fixed event listener leak (Closes #1062) Signed-off-by: Akos Kitta --- .../browser/arduino-frontend-contribution.tsx | 2 +- .../browser/arduino-ide-frontend-module.ts | 18 ++++- .../src/browser/contributions/ino-language.ts | 1 + .../browser/theia/core/application-shell.ts | 65 ++++++++++++++----- .../theia/core/connection-status-service.ts | 6 +- .../src/browser/theia/core/status-bar.ts | 13 ++++ .../src/browser/theia/core/tab-bars.ts | 22 ++++++- .../theia/editor/editor-widget-factory.ts | 16 ++--- .../browser/theia/markers/problem-manager.ts | 15 ++++- .../theia/core/electron-menu-module.ts | 2 +- electron/build/template-package.json | 2 +- package.json | 2 +- 12 files changed, 129 insertions(+), 35 deletions(-) create mode 100644 arduino-ide-extension/src/browser/theia/core/status-bar.ts diff --git a/arduino-ide-extension/src/browser/arduino-frontend-contribution.tsx b/arduino-ide-extension/src/browser/arduino-frontend-contribution.tsx index a8a5f4d4..14320039 100644 --- a/arduino-ide-extension/src/browser/arduino-frontend-contribution.tsx +++ b/arduino-ide-extension/src/browser/arduino-frontend-contribution.tsx @@ -104,7 +104,7 @@ export class ArduinoFrontendContribution } } }); - this.appStateService.reachedState('initialized_layout').then(() => + this.appStateService.reachedState('ready').then(() => this.arduinoPreferences.ready.then(() => { const webContents = remote.getCurrentWebContents(); const zoomLevel = this.arduinoPreferences.get( diff --git a/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts b/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts index 29224191..1e9a8209 100644 --- a/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts +++ b/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts @@ -50,13 +50,17 @@ import { ApplicationShell as TheiaApplicationShell, ShellLayoutRestorer as TheiaShellLayoutRestorer, CommonFrontendContribution as TheiaCommonFrontendContribution, + DockPanelRenderer as TheiaDockPanelRenderer, TabBarRendererFactory, ContextMenuRenderer, createTreeContainer, TreeWidget, } from '@theia/core/lib/browser'; import { MenuContribution } from '@theia/core/lib/common/menu'; -import { ApplicationShell } from './theia/core/application-shell'; +import { + ApplicationShell, + DockPanelRenderer, +} from './theia/core/application-shell'; import { FrontendApplication } from './theia/core/frontend-application'; import { BoardsConfigDialog, @@ -315,6 +319,8 @@ import { OpenBoardsConfig } from './contributions/open-boards-config'; import { SketchFilesTracker } from './contributions/sketch-files-tracker'; import { MonacoThemeServiceIsReady } from './utils/window'; import { Deferred } from '@theia/core/lib/common/promise-util'; +import { StatusBarImpl } from './theia/core/status-bar'; +import { StatusBarImpl as TheiaStatusBarImpl } from '@theia/core/lib/browser'; const registerArduinoThemes = () => { const themes: MonacoThemeJson[] = [ @@ -583,7 +589,7 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => { // Disabled reference counter in the editor manager to avoid opening the same editor (with different opener options) multiple times. bind(EditorManager).toSelf().inSingletonScope(); - rebind(TheiaEditorManager).to(EditorManager); + rebind(TheiaEditorManager).toService(EditorManager); // replace search icon rebind(TheiaSearchInWorkspaceFactory) @@ -822,6 +828,14 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => { bind(WidgetManager).toSelf().inSingletonScope(); rebind(TheiaWidgetManager).toService(WidgetManager); + // To avoid running a status bar update on every single `keypress` event from the editor. + bind(StatusBarImpl).toSelf().inSingletonScope(); + rebind(TheiaStatusBarImpl).toService(StatusBarImpl); + + // Debounced update for the tab-bar toolbar when typing in the editor. + bind(DockPanelRenderer).toSelf(); + rebind(TheiaDockPanelRenderer).toService(DockPanelRenderer); + // Preferences bindArduinoPreferences(bind); diff --git a/arduino-ide-extension/src/browser/contributions/ino-language.ts b/arduino-ide-extension/src/browser/contributions/ino-language.ts index 21cc2180..c9b4d2b9 100644 --- a/arduino-ide-extension/src/browser/contributions/ino-language.ts +++ b/arduino-ide-extension/src/browser/contributions/ino-language.ts @@ -145,6 +145,7 @@ export class InoLanguage extends SketchContribution { name: name ? `"${name}"` : undefined, }, realTimeDiagnostics, + silentOutput: true, } ), ]); diff --git a/arduino-ide-extension/src/browser/theia/core/application-shell.ts b/arduino-ide-extension/src/browser/theia/core/application-shell.ts index b9e0d4c6..f0610a56 100644 --- a/arduino-ide-extension/src/browser/theia/core/application-shell.ts +++ b/arduino-ide-extension/src/browser/theia/core/application-shell.ts @@ -10,28 +10,35 @@ import { import { ApplicationShell as TheiaApplicationShell, DockPanel, + DockPanelRenderer as TheiaDockPanelRenderer, Panel, + TabBar, Widget, + SHELL_TABBAR_CONTEXT_MENU, } from '@theia/core/lib/browser'; import { Sketch } from '../../../common/protocol'; import { SaveAsSketch } from '../../contributions/save-as-sketch'; -import { CurrentSketch, SketchesServiceClientImpl } from '../../../common/protocol/sketches-service-client-impl'; +import { + CurrentSketch, + SketchesServiceClientImpl, +} from '../../../common/protocol/sketches-service-client-impl'; import { nls } from '@theia/core/lib/common'; import URI from '@theia/core/lib/common/uri'; +import { ToolbarAwareTabBar } from './tab-bars'; @injectable() export class ApplicationShell extends TheiaApplicationShell { @inject(CommandService) - protected readonly commandService: CommandService; + private readonly commandService: CommandService; @inject(MessageService) - protected readonly messageService: MessageService; + private readonly messageService: MessageService; @inject(SketchesServiceClientImpl) - protected readonly sketchesServiceClient: SketchesServiceClientImpl; + private readonly sketchesServiceClient: SketchesServiceClientImpl; @inject(ConnectionStatusService) - protected readonly connectionStatusService: ConnectionStatusService; + private readonly connectionStatusService: ConnectionStatusService; protected override track(widget: Widget): void { super.track(widget); @@ -43,7 +50,7 @@ export class ApplicationShell extends TheiaApplicationShell { this.sketchesServiceClient.currentSketch().then((sketch) => { if (CurrentSketch.isValid(sketch)) { if (!this.isSketchFile(widget.editor.uri, sketch.uri)) { - return; + return; } if (Sketch.isInSketch(widget.editor.uri, sketch)) { widget.title.closable = false; @@ -54,11 +61,11 @@ export class ApplicationShell extends TheiaApplicationShell { } private isSketchFile(uri: URI, sketchUriString: string): boolean { - const sketchUri = new URI(sketchUriString); - if (uri.parent.isEqual(sketchUri)) { - return true; - } - return false; + const sketchUri = new URI(sketchUriString); + if (uri.parent.isEqual(sketchUri)) { + return true; + } + return false; } override async addWidget( @@ -120,15 +127,41 @@ export class ApplicationShell extends TheiaApplicationShell { } } +export class DockPanelRenderer extends TheiaDockPanelRenderer { + override createTabBar(): TabBar { + const renderer = this.tabBarRendererFactory(); + // `ToolbarAwareTabBar` is from IDE2 and not from Theia. Check the imports. + const tabBar = new ToolbarAwareTabBar( + this.tabBarToolbarRegistry, + this.tabBarToolbarFactory, + this.breadcrumbsRendererFactory, + { + renderer, + // Scroll bar options + handlers: ['drag-thumb', 'keyboard', 'wheel', 'touch'], + useBothWheelAxes: true, + scrollXMarginOffset: 4, + suppressScrollY: true, + } + ); + this.tabBarClasses.forEach((c) => tabBar.addClass(c)); + renderer.tabBar = tabBar; + tabBar.disposed.connect(() => renderer.dispose()); + renderer.contextMenuPath = SHELL_TABBAR_CONTEXT_MENU; + tabBar.currentChanged.connect(this.onCurrentTabChanged, this); + return tabBar; + } +} + const originalHandleEvent = DockPanel.prototype.handleEvent; DockPanel.prototype.handleEvent = function (event) { switch (event.type) { - case 'p-dragenter': - case 'p-dragleave': - case 'p-dragover': - case 'p-drop': - return; + case 'p-dragenter': + case 'p-dragleave': + case 'p-dragover': + case 'p-drop': + return; } originalHandleEvent.bind(this)(event); }; diff --git a/arduino-ide-extension/src/browser/theia/core/connection-status-service.ts b/arduino-ide-extension/src/browser/theia/core/connection-status-service.ts index 343a1398..0738772c 100644 --- a/arduino-ide-extension/src/browser/theia/core/connection-status-service.ts +++ b/arduino-ide-extension/src/browser/theia/core/connection-status-service.ts @@ -13,6 +13,7 @@ import { import { ArduinoDaemon } from '../../../common/protocol'; import { NotificationCenter } from '../../notification-center'; import { nls } from '@theia/core/lib/common'; +import debounce = require('lodash.debounce'); @injectable() export class FrontendConnectionStatusService extends TheiaFrontendConnectionStatusService { @@ -36,10 +37,11 @@ export class FrontendConnectionStatusService extends TheiaFrontendConnectionStat this.notificationCenter.onDaemonDidStop( () => (this.connectedPort = undefined) ); - this.wsConnectionProvider.onIncomingMessageActivity(() => { + const refresh = debounce(() => { this.updateStatus(!!this.connectedPort); this.schedulePing(); - }); + }, this.options.offlineTimeout - 10); + this.wsConnectionProvider.onIncomingMessageActivity(() => refresh()); } } diff --git a/arduino-ide-extension/src/browser/theia/core/status-bar.ts b/arduino-ide-extension/src/browser/theia/core/status-bar.ts new file mode 100644 index 00000000..3e7782c3 --- /dev/null +++ b/arduino-ide-extension/src/browser/theia/core/status-bar.ts @@ -0,0 +1,13 @@ +import { injectable } from '@theia/core/shared/inversify'; +import { StatusBarImpl as TheiaStatusBarImpl } from '@theia/core/lib/browser'; + +@injectable() +export class StatusBarImpl extends TheiaStatusBarImpl { + override async removeElement(id: string): Promise { + await this.ready; + if (this.entries.delete(id)) { + // Unlike Theia, IDE2 updates the status bar only if the element to remove was among the entries. Otherwise, it's a NOOP. + this.update(); + } + } +} diff --git a/arduino-ide-extension/src/browser/theia/core/tab-bars.ts b/arduino-ide-extension/src/browser/theia/core/tab-bars.ts index 406525a1..c6d5adc0 100644 --- a/arduino-ide-extension/src/browser/theia/core/tab-bars.ts +++ b/arduino-ide-extension/src/browser/theia/core/tab-bars.ts @@ -1,8 +1,13 @@ -import { TabBar } from '@theia/core/shared/@phosphor/widgets'; +import type { TabBar } from '@theia/core/shared/@phosphor/widgets'; import { Saveable } from '@theia/core/lib/browser/saveable'; -import { TabBarRenderer as TheiaTabBarRenderer } from '@theia/core/lib/browser/shell/tab-bars'; +import { + TabBarRenderer as TheiaTabBarRenderer, + ToolbarAwareTabBar as TheiaToolbarAwareTabBar, +} from '@theia/core/lib/browser/shell/tab-bars'; +import debounce = require('lodash.debounce'); export class TabBarRenderer extends TheiaTabBarRenderer { + // eslint-disable-next-line @typescript-eslint/no-explicit-any override createTabClass(data: TabBar.IRenderData): string { let className = super.createTabClass(data); if (!data.title.closable && Saveable.isDirty(data.title.owner)) { @@ -16,3 +21,16 @@ export class TabBarRenderer extends TheiaTabBarRenderer { // Context menus are empty, so they have been removed }; } + +export class ToolbarAwareTabBar extends TheiaToolbarAwareTabBar { + protected override async updateBreadcrumbs(): Promise { + // NOOP + // IDE2 does not use breadcrumbs. + } + + private readonly doUpdateToolbar = debounce(() => super.updateToolbar(), 500); + protected override updateToolbar(): void { + // Unlike Theia, IDE2 debounces the toolbar updates with 500ms + this.doUpdateToolbar(); + } +} diff --git a/arduino-ide-extension/src/browser/theia/editor/editor-widget-factory.ts b/arduino-ide-extension/src/browser/theia/editor/editor-widget-factory.ts index dd00e187..fc368c90 100644 --- a/arduino-ide-extension/src/browser/theia/editor/editor-widget-factory.ts +++ b/arduino-ide-extension/src/browser/theia/editor/editor-widget-factory.ts @@ -1,7 +1,7 @@ import { inject, injectable } from '@theia/core/shared/inversify'; import URI from '@theia/core/lib/common/uri'; import { EditorWidget } from '@theia/editor/lib/browser'; -import { LabelProvider } from '@theia/core/lib/browser'; +import type { NavigatableWidgetOptions } from '@theia/core/lib/browser'; import { EditorWidgetFactory as TheiaEditorWidgetFactory } from '@theia/editor/lib/browser/editor-widget-factory'; import { CurrentSketch, @@ -13,16 +13,16 @@ import { nls } from '@theia/core/lib/common'; @injectable() export class EditorWidgetFactory extends TheiaEditorWidgetFactory { @inject(SketchesService) - protected readonly sketchesService: SketchesService; + private readonly sketchesService: SketchesService; @inject(SketchesServiceClientImpl) - protected readonly sketchesServiceClient: SketchesServiceClientImpl; + private readonly sketchesServiceClient: SketchesServiceClientImpl; - @inject(LabelProvider) - protected override readonly labelProvider: LabelProvider; - - protected override async createEditor(uri: URI): Promise { - const widget = await super.createEditor(uri); + protected override async createEditor( + uri: URI, + options: NavigatableWidgetOptions + ): Promise { + const widget = await super.createEditor(uri, options); return this.maybeUpdateCaption(widget); } diff --git a/arduino-ide-extension/src/browser/theia/markers/problem-manager.ts b/arduino-ide-extension/src/browser/theia/markers/problem-manager.ts index 0e830fd5..b260bdab 100644 --- a/arduino-ide-extension/src/browser/theia/markers/problem-manager.ts +++ b/arduino-ide-extension/src/browser/theia/markers/problem-manager.ts @@ -1,10 +1,15 @@ -import { inject, injectable, postConstruct } from '@theia/core/shared/inversify'; +import { + inject, + injectable, + postConstruct, +} from '@theia/core/shared/inversify'; import { Diagnostic } from 'vscode-languageserver-types'; import URI from '@theia/core/lib/common/uri'; import { ILogger } from '@theia/core'; import { Marker } from '@theia/markers/lib/common/marker'; import { ProblemManager as TheiaProblemManager } from '@theia/markers/lib/browser/problem/problem-manager'; import { ConfigService } from '../../../common/protocol/config-service'; +import debounce = require('lodash.debounce'); @injectable() export class ProblemManager extends TheiaProblemManager { @@ -37,4 +42,12 @@ export class ProblemManager extends TheiaProblemManager { } return super.setMarkers(uri, owner, data); } + + private readonly debouncedFireOnDidChangeMakers = debounce( + (uri: URI) => this.onDidChangeMarkersEmitter.fire(uri), + 500 + ); + protected override fireOnDidChangeMarkers(uri: URI): void { + this.debouncedFireOnDidChangeMakers(uri); + } } diff --git a/arduino-ide-extension/src/electron-browser/theia/core/electron-menu-module.ts b/arduino-ide-extension/src/electron-browser/theia/core/electron-menu-module.ts index fb21f234..bed0179e 100644 --- a/arduino-ide-extension/src/electron-browser/theia/core/electron-menu-module.ts +++ b/arduino-ide-extension/src/electron-browser/theia/core/electron-menu-module.ts @@ -38,7 +38,7 @@ Object.assign(dialogs, { export default new ContainerModule((bind, unbind, isBound, rebind) => { bind(ElectronMenuContribution).toSelf().inSingletonScope(); bind(MainMenuManager).toService(ElectronMenuContribution); - rebind(TheiaElectronMenuContribution).to(ElectronMenuContribution); + rebind(TheiaElectronMenuContribution).toService(ElectronMenuContribution); bind(ElectronMainMenuFactory).toSelf().inSingletonScope(); rebind(TheiaElectronMainMenuFactory).toService(ElectronMainMenuFactory); bind(ElectronWindowService).toSelf().inSingletonScope(); diff --git a/electron/build/template-package.json b/electron/build/template-package.json index 2c8224ff..5c15d056 100644 --- a/electron/build/template-package.json +++ b/electron/build/template-package.json @@ -141,7 +141,7 @@ "theiaPluginsDir": "plugins", "theiaPlugins": { "vscode-builtin-cpp": "https://open-vsx.org/api/vscode/cpp/1.52.1/file/vscode.cpp-1.52.1.vsix", - "vscode-arduino-tools": "https://downloads.arduino.cc/vscode-arduino-tools/vscode-arduino-tools-0.0.2-beta.4.vsix", + "vscode-arduino-tools": "https://downloads.arduino.cc/vscode-arduino-tools/vscode-arduino-tools-0.0.2-beta.5.vsix", "vscode-builtin-json": "https://open-vsx.org/api/vscode/json/1.46.1/file/vscode.json-1.46.1.vsix", "vscode-builtin-json-language-features": "https://open-vsx.org/api/vscode/json-language-features/1.46.1/file/vscode.json-language-features-1.46.1.vsix", "cortex-debug": "https://open-vsx.org/api/marus25/cortex-debug/0.3.10/file/marus25.cortex-debug-0.3.10.vsix", diff --git a/package.json b/package.json index 9ef227ef..b0608174 100644 --- a/package.json +++ b/package.json @@ -75,7 +75,7 @@ "theiaPluginsDir": "plugins", "theiaPlugins": { "vscode-builtin-cpp": "https://open-vsx.org/api/vscode/cpp/1.52.1/file/vscode.cpp-1.52.1.vsix", - "vscode-arduino-tools": "https://downloads.arduino.cc/vscode-arduino-tools/vscode-arduino-tools-0.0.2-beta.4.vsix", + "vscode-arduino-tools": "https://downloads.arduino.cc/vscode-arduino-tools/vscode-arduino-tools-0.0.2-beta.5.vsix", "vscode-builtin-json": "https://open-vsx.org/api/vscode/json/1.46.1/file/vscode.json-1.46.1.vsix", "vscode-builtin-json-language-features": "https://open-vsx.org/api/vscode/json-language-features/1.46.1/file/vscode.json-language-features-1.46.1.vsix", "cortex-debug": "https://open-vsx.org/api/marus25/cortex-debug/0.3.10/file/marus25.cortex-debug-0.3.10.vsix",