Skip to content

Commit c87d3bc

Browse files
author
Akos Kitta
committed
fix: flawed timing issue when opening editors
From now on, the editor widget open promise resolution does not rely on internal Theia events but solely on @phosphor's `isAttached`/`isVisible` properties. The editor widget promise resolves with the next task after a navigation frame so the browser can render the widget. Closes #1612 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
1 parent 8a85b5c commit c87d3bc

File tree

1 file changed

+42
-47
lines changed

1 file changed

+42
-47
lines changed

arduino-ide-extension/src/browser/contributions/open-sketch-files.ts

+42-47
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { nls } from '@theia/core/lib/common/nls';
2-
import { inject, injectable } from '@theia/core/shared/inversify';
2+
import { injectable } from '@theia/core/shared/inversify';
33
import type { EditorOpenerOptions } from '@theia/editor/lib/browser/editor-manager';
44
import { Later } from '../../common/nls';
55
import { Sketch, SketchesError } from '../../common/protocol';
@@ -15,14 +15,9 @@ import { ApplicationError } from '@theia/core/lib/common/application-error';
1515
import { Deferred, wait } from '@theia/core/lib/common/promise-util';
1616
import { EditorWidget } from '@theia/editor/lib/browser/editor-widget';
1717
import { DisposableCollection } from '@theia/core/lib/common/disposable';
18-
import { MonacoEditor } from '@theia/monaco/lib/browser/monaco-editor';
19-
import { ContextKeyService as VSCodeContextKeyService } from '@theia/monaco-editor-core/esm/vs/platform/contextkey/browser/contextKeyService';
2018

2119
@injectable()
2220
export class OpenSketchFiles extends SketchContribution {
23-
@inject(VSCodeContextKeyService)
24-
private readonly contextKeyService: VSCodeContextKeyService;
25-
2621
override registerCommands(registry: CommandRegistry): void {
2722
registry.registerCommand(OpenSketchFiles.Commands.OPEN_SKETCH_FILES, {
2823
execute: (uri: URI) => this.openSketchFiles(uri),
@@ -135,39 +130,29 @@ export class OpenSketchFiles extends SketchContribution {
135130
const widget = this.editorManager.all.find(
136131
(widget) => widget.editor.uri.toString() === uri
137132
);
133+
if (widget && !forceOpen) {
134+
return widget;
135+
}
136+
138137
const disposables = new DisposableCollection();
139-
if (!widget || forceOpen) {
140-
const deferred = new Deferred<EditorWidget>();
138+
const deferred = new Deferred<EditorWidget>();
139+
if (!widget) {
140+
// If the editor or is not defined, assume one on create event
141141
disposables.push(
142142
this.editorManager.onCreated((editor) => {
143143
if (editor.editor.uri.toString() === uri) {
144-
if (editor.isVisible) {
145-
disposables.dispose();
144+
if (editor.isAttached && editor.isVisible) {
146145
deferred.resolve(editor);
147146
} else {
148-
// In Theia, the promise resolves after opening the editor, but the editor is neither attached to the DOM, nor visible.
149-
// This is a hack to first get an event from monaco after the widget update request, then IDE2 waits for the next monaco context key event.
150-
// Here, the monaco context key event is not used, but this is the first event after the editor is visible in the UI.
151147
disposables.push(
152-
(editor.editor as MonacoEditor).onDidResize((dimension) => {
153-
if (dimension) {
154-
const isKeyOwner = (
155-
arg: unknown
156-
): arg is { key: string } => {
157-
if (typeof arg === 'object') {
158-
const object = arg as Record<string, unknown>;
159-
return typeof object['key'] === 'string';
160-
}
161-
return false;
162-
};
163-
disposables.push(
164-
this.contextKeyService.onDidChangeContext((e) => {
165-
// `commentIsEmpty` is the first context key change event received from monaco after the editor is for real visible in the UI.
166-
if (isKeyOwner(e) && e.key === 'commentIsEmpty') {
167-
deferred.resolve(editor);
168-
disposables.dispose();
169-
}
170-
})
148+
editor.onDidChangeVisibility((visible) => {
149+
if (visible) {
150+
// wait an animation frame. although the visible and attached props are true the editor is not there.
151+
// let the browser render the widget
152+
setTimeout(
153+
() =>
154+
requestAnimationFrame(() => deferred.resolve(editor)),
155+
0
171156
);
172157
}
173158
})
@@ -176,29 +161,39 @@ export class OpenSketchFiles extends SketchContribution {
176161
}
177162
})
178163
);
179-
this.editorManager.open(
164+
}
165+
166+
this.editorManager
167+
.open(
180168
new URI(uri),
181169
options ?? {
182170
mode: 'reveal',
183171
preview: false,
184172
counter: 0,
185173
}
174+
)
175+
.then((editorWidget) => {
176+
// if the widget was already opened we assume it was already already visible and attached to the DOM
177+
// can resolve the deferred, no need to wait for the on create event.
178+
if (!widget) {
179+
deferred.resolve(editorWidget);
180+
}
181+
});
182+
183+
const timeout = 5_000; // number of ms IDE2 waits for the editor to show up in the UI
184+
const result = await Promise.race([
185+
deferred.promise,
186+
wait(timeout).then(() => {
187+
disposables.dispose();
188+
return 'timeout';
189+
}),
190+
]);
191+
if (result === 'timeout') {
192+
console.warn(
193+
`Timeout after ${timeout} millis. The editor has not shown up in time. URI: ${uri}`
186194
);
187-
const timeout = 5_000; // number of ms IDE2 waits for the editor to show up in the UI
188-
const result = await Promise.race([
189-
deferred.promise,
190-
wait(timeout).then(() => {
191-
disposables.dispose();
192-
return 'timeout';
193-
}),
194-
]);
195-
if (result === 'timeout') {
196-
console.warn(
197-
`Timeout after ${timeout} millis. The editor has not shown up in time. URI: ${uri}`
198-
);
199-
}
200-
return result;
201195
}
196+
return result;
202197
}
203198
}
204199
export namespace OpenSketchFiles {

0 commit comments

Comments
 (0)