Skip to content

Commit

Permalink
Rewrite Editor Preview as extensions of Editor classes
Browse files Browse the repository at this point in the history
Signed-off-by: Colin Grant <[email protected]>
  • Loading branch information
colin-grant-work committed Jun 9, 2021
1 parent d0aadd6 commit e8e88b7
Show file tree
Hide file tree
Showing 21 changed files with 364 additions and 666 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# Change Log

## v1.15.0 - 6/24/2021

[1.15.0 Milestone](https://github.com/eclipse-theia/theia/milestone/21)

- [editor-preview] rewrote `editor-preview`-package classes as extensions of `editor`-package classes [#9518](https://github.com/eclipse-theia/theia/pull/9517)

<a name="breaking_changes_1.15.0">[Breaking Changes:](#breaking_changes_1.15.0)</a>

- [editor-preview] `EditorPreviewWidget` now extends `EditorWidget` and `EditorPreviewManager` extends and overrides `EditorManager`. `instanceof` checks can no longer distinguish between preview and non-preview editors; use `.isPreview` field instead. [#9518](https://github.com/eclipse-theia/theia/pull/9517)

## v1.14.0 - 5/27/2021

[1.14.0 Milestone](https://github.com/eclipse-theia/theia/milestone/20)
Expand Down
2 changes: 1 addition & 1 deletion examples/api-tests/src/keybindings.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ describe('Keybindings', function () {
assert.notEqual(executedCommand, id);
});

it('later registered keybinding should has higher priority', async () => {
it('later registered keybinding should have higher priority', async () => {
const id = '__test:keybindings.copy';
toTearDown.push(commands.registerCommand({ id }, {
execute: () => { }
Expand Down
8 changes: 5 additions & 3 deletions examples/api-tests/src/saveable.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -468,17 +468,19 @@ describe('Saveable', function () {
}
});

it(`'${closeOnFileDelete}' should close the editor when set to 'true'`, async () => {
it.only(`'${closeOnFileDelete}' should close the editor when set to 'true'`, async () => {

await preferences.set(closeOnFileDelete, true);
assert.isTrue(preferences.get(closeOnFileDelete));
assert.isFalse(Saveable.isDirty(widget));

const waitForDisposed = new Deferred();
// Must pass in 5 seconds, so check state after 4.5.
const listener = editor.onDispose(() => waitForDisposed.resolve());
const fourSeconds = new Promise(resolve => setTimeout(resolve, 4500));
try {
await fileService.delete(fileUri);
await waitForDisposed.promise;
const deleteThenDispose = fileService.delete(fileUri).then(() => waitForDisposed.promise);
await Promise.race([deleteThenDispose, fourSeconds]);
assert.isTrue(widget.isDisposed);
} finally {
listener.dispose();
Expand Down
15 changes: 7 additions & 8 deletions examples/api-tests/src/typescript.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ describe('TypeScript', function () {
const { CommandRegistry } = require('@theia/core/lib/common/command');
const { KeybindingRegistry } = require('@theia/core/lib/browser/keybinding');
const { OpenerService, open } = require('@theia/core/lib/browser/opener-service');
const { EditorPreviewWidget } = require('@theia/editor-preview/lib/browser/editor-preview-widget');
const { animationFrame } = require('@theia/core/lib/browser/browser');
const { PreferenceService, PreferenceScope } = require('@theia/core/lib/browser/preferences/preference-service');
const { ProgressStatusBarItem } = require('@theia/core/lib/browser/progress-status-bar-item');
Expand Down Expand Up @@ -157,7 +156,7 @@ module.exports = (port, host, argv) => Promise.resolve()
*/
async function openEditor(uri, preview = false) {
const widget = await open(openerService, uri, { mode: 'activate', preview });
const editorWidget = widget instanceof EditorPreviewWidget ? widget.editorWidget : widget instanceof EditorWidget ? widget : undefined;
const editorWidget = widget instanceof EditorWidget ? widget : undefined;
const editor = MonacoEditor.get(editorWidget);
assert.isDefined(editor);

Expand Down Expand Up @@ -284,7 +283,7 @@ module.exports = (port, host, argv) => Promise.resolve()

const activeEditor = /** @type {MonacoEditor} */ (MonacoEditor.get(editorManager.activeEditor));
// @ts-ignore
assert.equal(editorManager.activeEditor.parent instanceof EditorPreviewWidget, preview);
assert.equal(editorManager.activeEditor.isPreview, preview);
assert.equal(activeEditor.uri.toString(), serverUri.toString());
// const |container = new Container();
// @ts-ignore
Expand All @@ -307,7 +306,7 @@ module.exports = (port, host, argv) => Promise.resolve()

const activeEditor = /** @type {MonacoEditor} */ (MonacoEditor.get(editorManager.activeEditor));
// @ts-ignore
assert.isFalse(editorManager.activeEditor.parent instanceof EditorPreviewWidget);
assert.isFalse(editorManager.activeEditor.isPreview);
assert.equal(activeEditor.uri.toString(), inversifyUri.toString());
// export { |Container } from "./container/container";
// @ts-ignore
Expand All @@ -328,7 +327,7 @@ module.exports = (port, host, argv) => Promise.resolve()

const activeEditor = /** @type {MonacoEditor} */ (MonacoEditor.get(editorManager.activeEditor));
// @ts-ignore
assert.isTrue(editorManager.activeEditor.parent instanceof EditorPreviewWidget);
assert.isTrue(editorManager.activeEditor.isPreview);
assert.equal(activeEditor.uri.toString(), inversifyUri.toString());
// export { |Container } from "./container/container";
// @ts-ignore
Expand Down Expand Up @@ -356,7 +355,7 @@ module.exports = (port, host, argv) => Promise.resolve()

const activeEditor = /** @type {MonacoEditor} */ (MonacoEditor.get(editorManager.activeEditor));
// @ts-ignore
assert.equal(editorManager.activeEditor.parent instanceof EditorPreviewWidget, preview);
assert.equal(editorManager.activeEditor.isPreview, preview);
assert.equal(activeEditor.uri.toString(), serverUri.toString());
// const |container = new Container();
// @ts-ignore
Expand All @@ -382,7 +381,7 @@ module.exports = (port, host, argv) => Promise.resolve()

const activeEditor = /** @type {MonacoEditor} */ (MonacoEditor.get(editorManager.activeEditor));
// @ts-ignore
assert.isFalse(editorManager.activeEditor.parent instanceof EditorPreviewWidget);
assert.isFalse(editorManager.activeEditor.isPreview);
assert.equal(activeEditor.uri.toString(), inversifyUri.toString());
// export { |Container } from "./container/container";
// @ts-ignore
Expand All @@ -406,7 +405,7 @@ module.exports = (port, host, argv) => Promise.resolve()

const activeEditor = /** @type {MonacoEditor} */ (MonacoEditor.get(editorManager.activeEditor));
// @ts-ignore
assert.isTrue(editorManager.activeEditor.parent instanceof EditorPreviewWidget);
assert.isTrue(editorManager.activeEditor.isPreview);
assert.equal(activeEditor.uri.toString(), inversifyUri.toString());
// export { |Container } from "./container/container";
// @ts-ignore
Expand Down
18 changes: 17 additions & 1 deletion packages/core/src/browser/keybinding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ export class KeybindingRegistry {
try {
this.resolveKeybinding(binding);
const scoped = Object.assign(binding, { scope });
this.keymaps[scope].unshift(scoped);
this.insertBindingIntoScope(scoped, scope);
return Disposable.create(() => {
const index = this.keymaps[scope].indexOf(scoped);
if (index !== -1) {
Expand All @@ -245,6 +245,22 @@ export class KeybindingRegistry {
}
}

/**
* Ensures that keybindings are inserted in order of increasing length of binding to ensure that if a
* user triggers a short keybinding (e.g. ctrl+k), the UI won't wait for a longer one (e.g. ctrl+k enter)
*/
protected insertBindingIntoScope(item: common.Keybinding & { scope: KeybindingScope; }, scope: KeybindingScope): void {
const scopedKeymap = this.keymaps[scope];
const getNumberOfKeystrokes = (binding: common.Keybinding): number => (binding.keybinding.trim().match(/\s/g)?.length ?? 0) + 1;
const numberOfKeystrokesInBinding = getNumberOfKeystrokes(item);
const indexOfFirstItemWithEqualStrokes = scopedKeymap.findIndex(existingBinding => getNumberOfKeystrokes(existingBinding) === numberOfKeystrokesInBinding);
if (indexOfFirstItemWithEqualStrokes > -1) {
scopedKeymap.splice(indexOfFirstItemWithEqualStrokes, 0, item);
} else {
scopedKeymap.push(item);
}
}

/**
* Ensure that the `resolved` property of the given binding is set by calling the KeyboardLayoutService.
*/
Expand Down
11 changes: 11 additions & 0 deletions packages/core/src/browser/shell/application-shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -850,6 +850,17 @@ export class ApplicationShell extends Widget {
return this.currentTabBar;
}

/**
* @returns the widget whose title has been targeted by a DOM event on a tabbar, or undefined if none can be found.
*/
findTargetedWidget(event?: Event): Widget | undefined {
if (event) {
const tab = this.findTabBar(event);
const title = tab && this.findTitle(tab, event);
return title && title.owner;
}
}

/**
* The current widget in the application shell. The current widget is the last widget that
* was active and not yet closed. See the remarks to `activeWidget` on what _active_ means.
Expand Down
14 changes: 13 additions & 1 deletion packages/core/src/browser/widget-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,18 @@ export class WidgetManager {
return undefined;
}

/**
* Try to get the existing widget for the given description.
* @param factoryId The widget factory id.
* @param options The widget factory specific information.
*
* @returns A promise that resolves to the widget, if any exists. The promise may be pending, so be cautious when assuming that it will not reject.
*/
tryGetPendingWidget<T extends Widget>(factoryId: string, options?: any): MaybePromise<T> | undefined {
const key = this.toKey({ factoryId, options });
return this.doGetWidget(key);
}

/**
* Get the widget for the given description.
* @param factoryId The widget factory id.
Expand All @@ -180,7 +192,7 @@ export class WidgetManager {
}

protected doGetWidget<T extends Widget>(key: string): MaybePromise<T> | undefined {
const pendingWidget = this.widgetPromises.get(key) || this.pendingWidgetPromises.get(key);
const pendingWidget = this.widgetPromises.get(key) ?? this.pendingWidgetPromises.get(key);
if (pendingWidget) {
return pendingWidget as MaybePromise<T>;
}
Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/browser/widget-open-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ export abstract class WidgetOpenHandler<W extends BaseWidget> implements OpenHan
return this.widgetManager.getWidgets(this.id) as W[];
}

protected tryGetPendingWidget(uri: URI, options?: WidgetOpenerOptions): MaybePromise<W> | undefined {
const factoryOptions = this.createWidgetOptions(uri, options);
return this.widgetManager.tryGetPendingWidget(this.id, factoryOptions);
}

protected getWidget(uri: URI, options?: WidgetOpenerOptions): Promise<W | undefined> {
const widgetOptions = this.createWidgetOptions(uri, options);
return this.widgetManager.getWidget<W>(this.id, widgetOptions);
Expand Down
72 changes: 72 additions & 0 deletions packages/editor-preview/src/browser/editor-preview-contribution.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/********************************************************************************
* Copyright (C) 2021 Ericsson and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the Eclipse
* Public License v. 2.0 are satisfied: GNU General Public License, version 2
* with the GNU Classpath Exception which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { ApplicationShell, KeybindingContribution, KeybindingRegistry, SHELL_TABBAR_CONTEXT_MENU, Widget } from '@theia/core/lib/browser';
import { Command, CommandContribution, CommandRegistry, MenuContribution, MenuModelRegistry } from '@theia/core/lib/common';
import { inject, injectable } from '@theia/core/shared/inversify';
import { EditorPreviewWidget } from './editor-preview-widget';

export namespace EditorPreviewCommands {
export const PIN_PREVIEW_COMMAND: Command = {
id: 'workbench.action.keepEditor',
category: 'View',
label: 'Keep Editor',
};
}

@injectable()
export class EditorPreviewContribution implements CommandContribution, MenuContribution, KeybindingContribution {
@inject(ApplicationShell) protected readonly shell: ApplicationShell;

registerCommands(registry: CommandRegistry): void {
registry.registerCommand(EditorPreviewCommands.PIN_PREVIEW_COMMAND, {
execute: async (event?: Event) => {
const widget = this.getTargetWidget(event);
if (widget instanceof EditorPreviewWidget) {
widget.convertToNonPreview();
await this.shell.activateWidget(widget.id);
}
},
isEnabled: (event?: Event) => {
const widget = this.getTargetWidget(event);
return widget instanceof EditorPreviewWidget && widget.isPreview;
},
isVisible: (event?: Event) => {
const widget = this.getTargetWidget(event);
return widget instanceof EditorPreviewWidget;
}
});
}

registerKeybindings(registry: KeybindingRegistry): void {
registry.registerKeybinding({
command: EditorPreviewCommands.PIN_PREVIEW_COMMAND.id,
keybinding: 'ctrlcmd+k enter'
});
}

registerMenus(registry: MenuModelRegistry): void {
registry.registerMenuAction(SHELL_TABBAR_CONTEXT_MENU, {
commandId: EditorPreviewCommands.PIN_PREVIEW_COMMAND.id,
label: 'Keep Open',
order: '6',
});
}

protected getTargetWidget(event?: Event): Widget | undefined {
return event ? this.shell.findTargetedWidget(event) : this.shell.activeWidget;
}
}
91 changes: 0 additions & 91 deletions packages/editor-preview/src/browser/editor-preview-factory.spec.ts

This file was deleted.

Loading

0 comments on commit e8e88b7

Please sign in to comment.