Apply vince's feedback

* Change host div id from pwidget to widget-host to make it clearer what the div is used for * Remove constructor injection in secondary window handler * Add changes to 1.26.0 and add breaking change for application shell constructor injection change * Fix typo
eclipse-theia · May 5, 2022 · 87cf80a · 87cf80a
1 parent e332c74
commit 87cf80a
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,16 @@
 
 - [Previous Changelogs](https://github.com/eclipse-theia/theia/tree/master/doc/changelogs/)
 
+## v1.26.0 - 5/26/2022
+
+[1.26.0 Milestone](https://github.com/eclipse-theia/theia/milestone/34)
+
+- [core] Added support for moving webview-based views into a secondary window/tab. Added new extension `secondary-window-ui` that contributes the UI integration to use this. [#11048](https://github.com/eclipse-theia/theia/pull/11048) - Contributed on behalf of ST Microelectronics and Ericsson and by ARM and EclipseSource
+
+<a name="breaking_changes_1.26.0">[Breaking Changes:](#breaking_changes_1.26.0)</a>
+
+- [core] Added constructor injection to `ApplicationShell`: `SecondaryWindowHandler`. [#11048](https://github.com/eclipse-theia/theia/pull/11048) - Contributed on behalf of ST Microelectronics and Ericsson and by ARM and EclipseSource
+
 ## v1.25.0 - 4/28/2022
 
 [1.25.0 Milestone](https://github.com/eclipse-theia/theia/milestone/35)

diff --git a/dev-packages/application-manager/src/generator/frontend-generator.ts b/dev-packages/application-manager/src/generator/frontend-generator.ts
@@ -219,7 +219,7 @@ module.exports = Promise.resolve()${this.compileElectronMainModuleImports(electr
     html,
     head,
     body,
-    #pwidget,
+    #widget-host,
     .p-Widget {
         width: 100% !important;
         height: 100% !important;
@@ -237,7 +237,7 @@ module.exports = Promise.resolve()${this.compileElectronMainModuleImports(electr
 </head>
 
 <body>
-    <div id="pwidget"></div>
+    <div id="widget-host"></div>
 </body>
 
 </html>`;

diff --git a/packages/core/src/browser/secondary-window-handler.ts b/packages/core/src/browser/secondary-window-handler.ts
@@ -72,10 +72,11 @@ export class SecondaryWindowHandler {
     /** Subscribe to get notified when a widget is removed from this handler, i.e. the widget's window was closed or the widget was disposed. */
     readonly onDidRemoveWidget = this.onDidRemoveWidgetEmitter.event;
 
-    constructor(
-        @inject(MessageService) protected readonly messageService: MessageService,
-        @inject(WindowService) protected readonly windowService: WindowService
-    ) { }
+    @inject(MessageService)
+    protected readonly messageService: MessageService;
+
+    @inject(WindowService)
+    protected readonly windowService: WindowService;
 
     /** @returns List of widgets in secondary windows. */
     get widgets(): ReadonlyArray<Widget> {
@@ -151,7 +152,7 @@ export class SecondaryWindowHandler {
         const newWindow = window.open('secondary-window.html', this.nextWindowId(), 'popup');
 
         if (!newWindow) {
-            this.messageService.error('The widget could not be moved to a secondary window because the window creation failed. Please make sure to allow popus.');
+            this.messageService.error('The widget could not be moved to a secondary window because the window creation failed. Please make sure to allow popups.');
             return;
         }
 
@@ -163,7 +164,7 @@ export class SecondaryWindowHandler {
             // See https://html.spec.whatwg.org/multipage/dom.html#document.title
             newWindow.document.title = widget.title.label;
 
-            const element = newWindow.document.getElementById('pwidget');
+            const element = newWindow.document.getElementById('widget-host');
             if (!element) {
                 console.error('Could not find dom element to attach to in secondary window');
                 return;