Skip to content

Commit

Permalink
#714: UX improvements of the Arduino LS in IDE2
Browse files Browse the repository at this point in the history
 - 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 <a.kitta@arduino.cc>
  • Loading branch information
Akos Kitta authored and kittaakos committed Aug 1, 2022
1 parent 90d2950 commit b62f3de
Show file tree
Hide file tree
Showing 12 changed files with 129 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
18 changes: 16 additions & 2 deletions arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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[] = [
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ export class InoLanguage extends SketchContribution {
name: name ? `"${name}"` : undefined,
},
realTimeDiagnostics,
silentOutput: true,
}
),
]);
Expand Down
65 changes: 49 additions & 16 deletions arduino-ide-extension/src/browser/theia/core/application-shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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(
Expand Down Expand Up @@ -120,15 +127,41 @@ export class ApplicationShell extends TheiaApplicationShell {
}
}

export class DockPanelRenderer extends TheiaDockPanelRenderer {
override createTabBar(): TabBar<Widget> {
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);
};
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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());
}
}

Expand Down
13 changes: 13 additions & 0 deletions arduino-ide-extension/src/browser/theia/core/status-bar.ts
Original file line number Diff line number Diff line change
@@ -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<void> {
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();
}
}
}
22 changes: 20 additions & 2 deletions arduino-ide-extension/src/browser/theia/core/tab-bars.ts
Original file line number Diff line number Diff line change
@@ -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<any>): string {
let className = super.createTabClass(data);
if (!data.title.closable && Saveable.isDirty(data.title.owner)) {
Expand All @@ -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<void> {
// 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();
}
}
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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<EditorWidget> {
const widget = await super.createEditor(uri);
protected override async createEditor(
uri: URI,
options: NavigatableWidgetOptions
): Promise<EditorWidget> {
const widget = await super.createEditor(uri, options);
return this.maybeUpdateCaption(widget);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion electron/build/template-package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit b62f3de

Please sign in to comment.