Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add secondary window support for text editors #13493

Merged
merged 10 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
- [plugin] Extend TextEditorLineNumbersStyle with Interval [#13458](https://github.com/eclipse-theia/theia/pull/13458) - contributed on behalf of STMicroelectronics

<a name="breaking_changes_not_yet_released">[Breaking Changes:](#breaking_changes_not_yet_released)</a>
- [core] Add secondary windows support for text editors. [#13493](https://github.com/eclipse-theia/theia/pull/13493 ). The changes in require more extensive patches for our dependencies than before. For this purpose, we are using the `patch-package` library. However, this change requires adopters to add the line `"postinstall": "theia-patch"` to the `package.json` at the root of their monorepo (where the `node_modules` folder is located). - contributed on behalf of STMicroelectronics

- [component] add here

## v1.47.0 - 02/29/2024

Expand Down
1 change: 0 additions & 1 deletion dev-packages/application-manager/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
"source-map": "^0.6.1",
"source-map-loader": "^2.0.1",
"source-map-support": "^0.5.19",
"string-replace-loader": "^3.1.0",
"style-loader": "^2.0.0",
"tslib": "^2.6.2",
"umd-compat-loader": "^2.1.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,15 +188,6 @@ ${Array.from(frontendModules.values(), jsModulePath => `\
}
</style>
<link rel="stylesheet" href="./secondary-window.css">
<script>
window.addEventListener('message', e => {
// Only process messages from Theia main window
if (e.source === window.opener) {
// Delegate message to iframe
document.getElementsByTagName('iframe').item(0).contentWindow.postMessage({ ...e.data }, '*');
}
});
</script>
</head>

<body>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,24 +128,6 @@ module.exports = [{
cache: staticCompression,
module: {
rules: [
{
// Removes the host check in PhosphorJS to enable moving widgets to secondary windows.
test: /widget\\.js$/,
loader: 'string-replace-loader',
include: /node_modules[\\\\/]@phosphor[\\\\/]widgets[\\\\/]lib/,
options: {
multiple: [
{
search: /document\\.body\\.contains\\(widget.node\\)/gm,
replace: 'widget.node.ownerDocument.body.contains(widget.node)'
},
{
search: /\\!document\\.body\\.contains\\(host\\)/gm,
replace: ' !host.ownerDocument.body.contains(host)'
}
]
}
},
{
test: /\\.css$/,
exclude: /materialcolors\\.css$|\\.useable\\.css$/,
Expand Down Expand Up @@ -280,6 +262,10 @@ module.exports = [{
{
test: /\.css$/i,
use: [MiniCssExtractPlugin.loader, "css-loader"]
},
{
test: /\.wasm$/,
type: 'asset/resource'
Comment on lines +265 to +268
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this change required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I recall correctly, the monaco editor uses wasm for tokenizing and the wasm files were not included in the bundle.

}
]
},
Expand Down
38 changes: 38 additions & 0 deletions dev-packages/cli/bin/theia-patch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#!/usr/bin/env node

// *****************************************************************************
// Copyright (C) 2024 STMicroelectronics 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-only WITH Classpath-exception-2.0
// *****************************************************************************
// @ts-check
const path = require('path');
const cp = require('child_process');

const patchPackage= require.resolve('patch-package');
console.log(`patch-package = ${patchPackage}`);

const patchesDir = path.join('.', 'node_modules', '@theia', 'cli', 'patches');

console.log(`patchesdir = ${patchesDir}`);

const env = Object.assign({}, process.env);

const scriptProcess = cp.exec(`node ${patchPackage} --patch-dir "${patchesDir}"`, {
cwd: process.cwd(),
env,

});

scriptProcess.stdout.pipe(process.stdout);
scriptProcess.stderr.pipe(process.stderr);
4 changes: 3 additions & 1 deletion dev-packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
"src"
],
"bin": {
"theia": "./bin/theia"
"theia": "./bin/theia",
"theia-patch": "./bin/theia-patch.js"
},
"scripts": {
"prepare": "tsc -b",
Expand All @@ -30,6 +31,7 @@
"clean": "theiaext clean"
},
"dependencies": {
"patch-package": "^8.0.0",
"@theia/application-manager": "1.47.0",
"@theia/application-package": "1.47.0",
"@theia/ffmpeg": "1.47.0",
Expand Down
157 changes: 157 additions & 0 deletions dev-packages/cli/patches/@phosphor+widgets+1.9.3.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
diff --git a/node_modules/@phosphor/widgets/lib/menu.d.ts b/node_modules/@phosphor/widgets/lib/menu.d.ts
index 5d5053c..7802167 100644
--- a/node_modules/@phosphor/widgets/lib/menu.d.ts
+++ b/node_modules/@phosphor/widgets/lib/menu.d.ts
@@ -195,7 +195,7 @@ export declare class Menu extends Widget {
*
* This is a no-op if the menu is already attached to the DOM.
*/
- open(x: number, y: number, options?: Menu.IOpenOptions): void;
+ open(x: number, y: number, options?: Menu.IOpenOptions, anchor?: HTMLElement): void;
/**
* Handle the DOM events for the menu.
*
diff --git a/node_modules/@phosphor/widgets/lib/menu.js b/node_modules/@phosphor/widgets/lib/menu.js
index de23022..a8b15b1 100644
--- a/node_modules/@phosphor/widgets/lib/menu.js
+++ b/node_modules/@phosphor/widgets/lib/menu.js
@@ -13,7 +13,7 @@ var __extends = (this && this.__extends) || (function () {
};
})();
var __assign = (this && this.__assign) || function () {
- __assign = Object.assign || function(t) {
+ __assign = Object.assign || function (t) {
for (var s, i = 1, n = arguments.length; i < n; i++) {
s = arguments[i];
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p))
@@ -424,7 +424,7 @@ var Menu = /** @class */ (function (_super) {
*
* This is a no-op if the menu is already attached to the DOM.
*/
- Menu.prototype.open = function (x, y, options) {
+ Menu.prototype.open = function (x, y, options, node) {
if (options === void 0) { options = {}; }
// Bail early if the menu is already attached.
if (this.isAttached) {
@@ -434,7 +434,7 @@ var Menu = /** @class */ (function (_super) {
var forceX = options.forceX || false;
var forceY = options.forceY || false;
// Open the menu as a root menu.
- Private.openRootMenu(this, x, y, forceX, forceY);
+ Private.openRootMenu(this, x, y, forceX, forceY, node);
// Activate the menu to accept keyboard input.
this.activate();
};
@@ -484,8 +484,16 @@ var Menu = /** @class */ (function (_super) {
this.node.addEventListener('mouseenter', this);
this.node.addEventListener('mouseleave', this);
this.node.addEventListener('contextmenu', this);
- document.addEventListener('mousedown', this, true);
};
+
+ Menu.prototype.onAfterAttach = function (msg) {
+ this.node.ownerDocument.addEventListener('mousedown', this, true);
+ }
+
+ Menu.prototype.onBeforeDetach = function (msg) {
+ this.node.ownerDocument.removeEventListener('mousedown', this, true);
+ }
+
/**
* A message handler invoked on an `'after-detach'` message.
*/
@@ -496,7 +504,6 @@ var Menu = /** @class */ (function (_super) {
this.node.removeEventListener('mouseenter', this);
this.node.removeEventListener('mouseleave', this);
this.node.removeEventListener('contextmenu', this);
- document.removeEventListener('mousedown', this, true);
};
/**
* A message handler invoked on an `'activate-request'` message.
@@ -1124,14 +1131,15 @@ var Private;
/**
* Open a menu as a root menu at the target location.
*/
- function openRootMenu(menu, x, y, forceX, forceY) {
+ function openRootMenu(menu, x, y, forceX, forceY, element) {
// Ensure the menu is updated before attaching and measuring.
messaging_1.MessageLoop.sendMessage(menu, widget_1.Widget.Msg.UpdateRequest);
// Get the current position and size of the main viewport.
+ var doc = element ? element.ownerDocument : document;
var px = window.pageXOffset;
var py = window.pageYOffset;
- var cw = document.documentElement.clientWidth;
- var ch = document.documentElement.clientHeight;
+ var cw = doc.documentElement.clientWidth;
+ var ch = doc.documentElement.clientHeight;
// Compute the maximum allowed height for the menu.
var maxHeight = ch - (forceY ? y : 0);
// Fetch common variables.
@@ -1145,7 +1153,7 @@ var Private;
style.visibility = 'hidden';
style.maxHeight = maxHeight + "px";
// Attach the menu to the document.
- widget_1.Widget.attach(menu, document.body);
+ widget_1.Widget.attach(menu, doc.body);
// Measure the size of the menu.
var _a = node.getBoundingClientRect(), width = _a.width, height = _a.height;
// Adjust the X position of the menu to fit on-screen.
@@ -1177,8 +1185,8 @@ var Private;
// Get the current position and size of the main viewport.
var px = window.pageXOffset;
var py = window.pageYOffset;
- var cw = document.documentElement.clientWidth;
- var ch = document.documentElement.clientHeight;
+ var cw = itemNode.ownerDocument.documentElement.clientWidth;
+ var ch = itemNode.ownerDocument.documentElement.clientHeight;
// Compute the maximum allowed height for the menu.
var maxHeight = ch;
// Fetch common variables.
@@ -1192,7 +1200,7 @@ var Private;
style.visibility = 'hidden';
style.maxHeight = maxHeight + "px";
// Attach the menu to the document.
- widget_1.Widget.attach(submenu, document.body);
+ widget_1.Widget.attach(submenu, itemNode.ownerDocument.body);
// Measure the size of the menu.
var _a = node.getBoundingClientRect(), width = _a.width, height = _a.height;
// Compute the box sizing for the menu.
diff --git a/node_modules/@phosphor/widgets/lib/menubar.js b/node_modules/@phosphor/widgets/lib/menubar.js
index a8e10f4..da2ee82 100644
--- a/node_modules/@phosphor/widgets/lib/menubar.js
+++ b/node_modules/@phosphor/widgets/lib/menubar.js
@@ -521,7 +521,7 @@ var MenuBar = /** @class */ (function (_super) {
// Get the positioning data for the new menu.
var _a = itemNode.getBoundingClientRect(), left = _a.left, bottom = _a.bottom;
// Open the new menu at the computed location.
- newMenu.open(left, bottom, { forceX: true, forceY: true });
+ newMenu.open(left, bottom, { forceX: true, forceY: true }, this.node);
};
/**
* Close the child menu immediately.
diff --git a/node_modules/@phosphor/widgets/lib/widget.js b/node_modules/@phosphor/widgets/lib/widget.js
index 01241fa..62da27c 100644
--- a/node_modules/@phosphor/widgets/lib/widget.js
+++ b/node_modules/@phosphor/widgets/lib/widget.js
@@ -906,10 +906,10 @@ exports.Widget = Widget;
if (widget.parent) {
throw new Error('Cannot attach a child widget.');
}
- if (widget.isAttached || document.body.contains(widget.node)) {
+ if (widget.isAttached || widget.node.ownerDocument.body.contains(widget.node)) {
throw new Error('Widget is already attached.');
}
- if (!document.body.contains(host)) {
+ if (!host.ownerDocument.body.contains(host)) {
throw new Error('Host is not attached.');
}
messaging_1.MessageLoop.sendMessage(widget, Widget.Msg.BeforeAttach);
@@ -930,7 +930,7 @@ exports.Widget = Widget;
if (widget.parent) {
throw new Error('Cannot detach a child widget.');
}
- if (!widget.isAttached || !document.body.contains(widget.node)) {
+ if (!widget.isAttached || !widget.node.ownerDocument.body.contains(widget.node)) {
throw new Error('Widget is not attached.');
}
messaging_1.MessageLoop.sendMessage(widget, Widget.Msg.BeforeDetach);
32 changes: 32 additions & 0 deletions dev-packages/cli/patches/@theia+monaco-editor-core+1.83.101.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
diff --git a/node_modules/@theia/monaco-editor-core/esm/vs/base/browser/ui/sash/sash.js b/node_modules/@theia/monaco-editor-core/esm/vs/base/browser/ui/sash/sash.js
index 111dec4..b196066 100644
--- a/node_modules/@theia/monaco-editor-core/esm/vs/base/browser/ui/sash/sash.js
+++ b/node_modules/@theia/monaco-editor-core/esm/vs/base/browser/ui/sash/sash.js
@@ -47,14 +47,15 @@ function setGlobalHoverDelay(size) {
}
exports.setGlobalHoverDelay = setGlobalHoverDelay;
class MouseEventFactory {
- constructor() {
+ constructor(el) {
+ this.el = el;
this.disposables = new lifecycle_1.DisposableStore();
}
get onPointerMove() {
- return this.disposables.add(new event_1.DomEmitter(window, 'mousemove')).event;
+ return this.disposables.add(new event_1.DomEmitter(this.el.ownerDocument.defaultView, 'mousemove')).event;
}
get onPointerUp() {
- return this.disposables.add(new event_1.DomEmitter(window, 'mouseup')).event;
+ return this.disposables.add(new event_1.DomEmitter(this.el.ownerDocument.defaultView, 'mouseup')).event;
}
dispose() {
this.disposables.dispose();
@@ -243,7 +244,7 @@ class Sash extends lifecycle_1.Disposable {
this.el.classList.add('mac');
}
const onMouseDown = this._register(new event_1.DomEmitter(this.el, 'mousedown')).event;
- this._register(onMouseDown(e => this.onPointerStart(e, new MouseEventFactory()), this));
+ this._register(onMouseDown(e => this.onPointerStart(e, new MouseEventFactory(this.el)), this));
const onMouseDoubleClick = this._register(new event_1.DomEmitter(this.el, 'dblclick')).event;
this._register(onMouseDoubleClick(this.onPointerDoublePress, this));
const onMouseEnter = this._register(new event_1.DomEmitter(this.el, 'mouseenter')).event;
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
"lint:clean": "rimraf .eslintcache",
"lint:oneshot": "node --max-old-space-size=4096 node_modules/eslint/bin/eslint.js --cache=true \"{dev-packages,packages,examples}/**/*.{ts,tsx}\"",
"preinstall": "node-gyp install",
"postinstall": "theia-patch",
"prepare": "yarn -s compile:references && lerna run prepare && yarn -s compile",
"publish:latest": "lerna publish --exact --yes --no-push",
"publish:next": "lerna publish preminor --exact --canary --preid next --dist-tag next --no-git-reset --no-git-tag-version --no-push --yes && yarn -s publish:check",
Expand Down
2 changes: 1 addition & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -213,4 +213,4 @@
"nyc": {
"extends": "../../configs/nyc.json"
}
}
}
13 changes: 11 additions & 2 deletions packages/core/src/browser/color-application-contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { FrontendApplicationContribution } from './frontend-application-contribu
import { ContributionProvider } from '../common/contribution-provider';
import { Disposable, DisposableCollection } from '../common/disposable';
import { DEFAULT_BACKGROUND_COLOR_STORAGE_KEY } from './frontend-application-config-provider';
import { SecondaryWindowHandler } from './secondary-window-handler';

export const ColorContribution = Symbol('ColorContribution');
export interface ColorContribution {
Expand All @@ -43,6 +44,9 @@ export class ColorApplicationContribution implements FrontendApplicationContribu

@inject(ThemeService) protected readonly themeService: ThemeService;

@inject(SecondaryWindowHandler)
protected readonly secondaryWindowHandler: SecondaryWindowHandler;

onStart(): void {
for (const contribution of this.colorContributions.getContributions()) {
contribution.registerColors(this.colors);
Expand All @@ -55,13 +59,18 @@ export class ColorApplicationContribution implements FrontendApplicationContribu
this.colors.onDidChange(() => this.update());

this.registerWindow(window);
this.secondaryWindowHandler.onWillAddWidget(([widget, window]) => {
this.registerWindow(window);
});
this.secondaryWindowHandler.onWillRemoveWidget(([widget, window]) => {
this.windows.delete(window);
});
}

registerWindow(win: Window): Disposable {
registerWindow(win: Window): void {
this.windows.add(win);
this.updateWindow(win);
this.onDidChangeEmitter.fire();
return Disposable.create(() => this.windows.delete(win));
}

protected readonly toUpdate = new DisposableCollection();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ export class BrowserContextMenuRenderer extends ContextMenuRenderer {
if (onHide) {
contextMenu.aboutToClose.connect(() => onHide!());
}
contextMenu.open(x, y);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
contextMenu.open(x, y, undefined, context);
return new BrowserContextMenuAccess(contextMenu);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/browser/menu/browser-menu-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,14 +267,14 @@ export class DynamicMenuWidget extends MenuWidget {
});
}

public override open(x: number, y: number, options?: MenuWidget.IOpenOptions): void {
public override open(x: number, y: number, options?: MenuWidget.IOpenOptions, anchor?: HTMLElement): void {
const cb = () => {
this.restoreFocusedElement();
this.aboutToClose.disconnect(cb);
};
this.aboutToClose.connect(cb);
this.preserveFocusedElement();
super.open(x, y, options);
super.open(x, y, options, anchor);
}

protected updateSubMenus(parent: MenuWidget, menu: CompoundMenuNode, commands: MenuCommandRegistry): void {
Expand Down
Loading
Loading