km comments

Signed-off-by: Colin Grant <colin.grant@ericsson.com>

packages/core/src/browser/breadcrumbs/breadcrumb-popup-container.ts

@@ -42,8 +42,8 @@ export class BreadcrumbPopupContainer implements Disposable {
     @inject(BreadcrumbID) public readonly breadcrumbId: BreadcrumbID;
     @inject(Coordinate) protected readonly position: Coordinate;
 
-    protected toDispose: DisposableCollection = new DisposableCollection();
     protected onDidDisposeEmitter = new Emitter<void>();
+    protected toDispose: DisposableCollection = new DisposableCollection(this.onDidDisposeEmitter);
     get onDidDispose(): Event<void> {
         return this.onDidDisposeEmitter.event;
     }
@@ -92,7 +92,7 @@ export class BreadcrumbPopupContainer implements Disposable {
             if (this._container.contains(event.relatedTarget)) {
                 // A child element gets focus. Set the focus to the container again.
                 // Otherwise the popup would not be closed when elements outside the popup get the focus.
-                // A popup content should not relay on getting a focus.
+                // A popup content should not rely on getting a focus.
                 this._container.focus();
                 return;
             }
@@ -107,12 +107,12 @@ export class BreadcrumbPopupContainer implements Disposable {
     };
 
     dispose(): void {
-        this.onDidDisposeEmitter.fire();
-        this.toDispose.dispose();
-        if (this.parent.contains(this._container)) {
-            this.parent.removeChild(this._container);
+        if (!this.toDispose.disposed) {
+            this.onDidDisposeEmitter.fire();
+            this.toDispose.dispose();
+            this._container.remove();
+            this._isOpen = false;
+            document.removeEventListener('keyup', this.escFunction);
         }
-        this._isOpen = false;
-        document.removeEventListener('keyup', this.escFunction);
     }
 }

packages/core/src/browser/shell/tab-bars.ts

@@ -20,7 +20,7 @@ import { VirtualElement, h, VirtualDOM, ElementInlineStyle } from '@phosphor/vir
 import { Disposable, DisposableCollection, MenuPath, notEmpty } from '../../common';
 import { ContextMenuRenderer } from '../context-menu-renderer';
 import { Signal, Slot } from '@phosphor/signaling';
-import { Message } from '@phosphor/messaging';
+import { Message, MessageLoop } from '@phosphor/messaging';
 import { ArrayExt } from '@phosphor/algorithm';
 import { ElementExt } from '@phosphor/domutils';
 import { TabBarToolbarRegistry, TabBarToolbar } from './tab-bar-toolbar';
@@ -602,7 +602,9 @@ export class ToolbarAwareTabBar extends ScrollableTabBar {
         this.toDispose.push(this.breadcrumbsRenderer);
         this.toDispose.push(this.breadcrumbsRenderer.onDidChangeActiveState(active => {
             this.node.classList.toggle('theia-tabBar-multirow', active);
-            this.update();
+            if (this.parent) {
+                MessageLoop.sendMessage(this.parent, new Message('fit-request'));
+            }
         }));
         this.node.classList.toggle('theia-tabBar-multirow', this.breadcrumbsRenderer.active);
         const handler = () => this.updateBreadcrumbs();

packages/core/src/browser/style/breadcrumbs.css

@@ -14,6 +14,10 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 
+:root {
+    --theia-breadcrumbs-height: 22px;
+}
+
 .theia-breadcrumbs {
     position: relative;
     user-select: none;
@@ -44,7 +48,7 @@
     height: 100%;
     color: var(--theia-breadcrumb-foreground);
     outline: none;
-    padding: .25rem .3rem .25rem .25rem;
+    padding: calc(var(--theia-ui-padding) / 2) .3rem calc(var(--theia-ui-padding) / 2) .25rem;
 }
 
 .theia-breadcrumbs .theia-breadcrumb-item:hover {

packages/core/src/browser/style/tabs.css

@@ -9,6 +9,8 @@
   --theia-private-horizontal-tab-scrollbar-height: 5px;
   --theia-tabbar-toolbar-z-index: 1001;
   --theia-toolbar-active-transform-scale: 1.272019649;
+  --theia-horizontal-toolbar-height: calc(var(--theia-private-horizontal-tab-height) + var(--theia-private-horizontal-tab-scrollbar-rail-height) / 2);
+  --theia-breadcrumbs-height: 22px;
 }
 
 /*-----------------------------------------------------------------------------
@@ -22,7 +24,7 @@
 .p-TabBar[data-orientation='horizontal'] {
   overflow-x: hidden;
   overflow-y: hidden;
-  min-height: calc(var(--theia-private-horizontal-tab-height) + var(--theia-private-horizontal-tab-scrollbar-rail-height) / 2);
+  min-height: var(--theia-horizontal-toolbar-height);
 }
 
 .p-TabBar .p-TabBar-content {
@@ -31,7 +33,7 @@
 
 .p-TabBar[data-orientation='horizontal'] .p-TabBar-tab {
   flex: none;
-  height: calc(var(--theia-private-horizontal-tab-height) + var(--theia-private-horizontal-tab-scrollbar-rail-height) / 2);
+  height: var(--theia-horizontal-toolbar-height);
   min-width: 35px;
   line-height: var(--theia-private-horizontal-tab-height);
   padding: 0px 8px;
@@ -389,16 +391,16 @@ body.theia-editor-highlightModifiedTabs
 }
 
 .theia-tabBar-breadcrumb-row {
-  width: 100%;
+  min-width: 100%;
 }
 
 .p-TabBar.theia-tabBar-multirow[data-orientation='horizontal'] {
-  min-height: calc(var(--theia-private-horizontal-tab-height) + var(--theia-private-horizontal-tab-scrollbar-rail-height) / 2 + var(--theia-icon-size) * 1.5) ;
+  min-height: calc(var(--theia-breadcrumbs-height) + var(--theia-horizontal-toolbar-height));
   flex-direction: column;
 }
 
 .theia-tabBar-tab-row {
   display: flex;
   flex-flow: row nowrap;
-  width: 100%;
+  min-width: 100%;
 }

packages/filesystem/src/browser/breadcrumbs/filepath-breadcrumbs-container.ts

@@ -15,7 +15,7 @@
  ********************************************************************************/
 
 import { Container, interfaces, injectable, inject } from '@theia/core/shared/inversify';
-import { TreeProps, ContextMenuRenderer, TreeNode, OpenerService, NodeProps } from '@theia/core/lib/browser';
+import { TreeProps, ContextMenuRenderer, TreeNode, OpenerService, open, NodeProps, defaultTreeProps } from '@theia/core/lib/browser';
 import { createFileTreeContainer, FileTreeWidget } from '../';
 import { FileTreeModel, FileStatNode } from '../file-tree';
 
@@ -24,6 +24,7 @@ const BREADCRUMBS_FILETREE_CLASS = 'theia-FilepathBreadcrumbFileTree';
 export function createFileTreeBreadcrumbsContainer(parent: interfaces.Container): Container {
     const child = createFileTreeContainer(parent);
     child.unbind(FileTreeWidget);
+    child.rebind(TreeProps).toConstantValue({ ...defaultTreeProps, virtualized: false });
     child.bind(BreadcrumbsFileTreeWidget).toSelf();
     return child;
 }
@@ -57,8 +58,7 @@ export class BreadcrumbsFileTreeWidget extends FileTreeWidget {
 
     protected handleClickEvent(node: TreeNode | undefined, event: React.MouseEvent<HTMLElement>): void {
         if (FileStatNode.is(node) && !node.fileStat.isDirectory) {
-            this.openerService.getOpener(node.uri)
-                .then(opener => opener.open(node.uri));
+            open(this.openerService, node.uri, { preview: true });
         } else {
             super.handleClickEvent(node, event);
         }

packages/filesystem/src/browser/breadcrumbs/filepath-breadcrumbs-contribution.ts

@@ -18,7 +18,7 @@ import { BreadcrumbsContribution } from '@theia/core/lib/browser/breadcrumbs/bre
 import { Breadcrumb } from '@theia/core/lib/browser/breadcrumbs/breadcrumb';
 import { FilepathBreadcrumb } from './filepath-breadcrumb';
 import { injectable, inject } from '@theia/core/shared/inversify';
-import { LabelProvider, Widget } from '@theia/core/lib/browser';
+import { CompositeTreeNode, LabelProvider, SelectableTreeNode, Widget } from '@theia/core/lib/browser';
 import URI from '@theia/core/lib/common/uri';
 import { BreadcrumbsFileTreeWidget } from './filepath-breadcrumbs-container';
 import { DirNode } from '../file-tree';
@@ -69,16 +69,29 @@ export class FilepathBreadcrumbsContribution implements BreadcrumbsContribution
         const folderFileStat = await this.fileSystem.resolve(breadcrumb.uri.parent);
         if (folderFileStat) {
             const rootNode = await this.createRootNode(folderFileStat);
-            await this.breadcrumbsFileTreeWidget.model.navigateTo(rootNode);
-            Widget.attach(this.breadcrumbsFileTreeWidget, parent);
-            return {
-                dispose: () => {
-                    // Clear model otherwise the next time a popup is opened the old model is rendered first
-                    // and is shown for a short time period.
-                    this.breadcrumbsFileTreeWidget.model.root = undefined;
-                    Widget.detach(this.breadcrumbsFileTreeWidget);
-                }
-            };
+            if (rootNode) {
+                const { model } = this.breadcrumbsFileTreeWidget;
+                await model.navigateTo({ ...rootNode, visible: false });
+                Widget.attach(this.breadcrumbsFileTreeWidget, parent);
+                const toDisposeOnTreePopulated = model.onChanged(() => {
+                    if (CompositeTreeNode.is(model.root) && model.root.children.length > 0) {
+                        toDisposeOnTreePopulated.dispose();
+                        const targetNode = model.getNode(breadcrumb.uri.path.toString());
+                        if (targetNode && SelectableTreeNode.is(targetNode)) {
+                            model.selectNode(targetNode);
+                        }
+                    }
+                });
+                return {
+                    dispose: () => {
+                        // Clear model otherwise the next time a popup is opened the old model is rendered first
+                        // and is shown for a short time period.
+                        toDisposeOnTreePopulated.dispose();
+                        this.breadcrumbsFileTreeWidget.model.root = undefined;
+                        Widget.detach(this.breadcrumbsFileTreeWidget);
+                    }
+                };
+            }
         }
     }
 

packages/filesystem/src/browser/style/filepath-breadcrumbs.css

@@ -15,5 +15,6 @@
  ********************************************************************************/
 
 .theia-FilepathBreadcrumbFileTree {
-    height: 200px;
+    height: auto;
+    max-height: 200px;
 }

packages/outline-view/src/browser/outline-breadcrumbs-contribution.tsx

@@ -15,24 +15,42 @@
  ********************************************************************************/
 
 import * as React from '@theia/core/shared/react';
-import * as ReactDOM from '@theia/core/shared/react-dom';
 import { BreadcrumbsContribution } from '@theia/core/lib/browser/breadcrumbs/breadcrumbs-contribution';
 import { Breadcrumb } from '@theia/core/lib/browser/breadcrumbs/breadcrumb';
 import { injectable, inject, postConstruct } from '@theia/core/shared/inversify';
-import { LabelProvider, BreadcrumbsService } from '@theia/core/lib/browser';
+import { LabelProvider, BreadcrumbsService, Widget, TreeNode, OpenerService, open, SelectableTreeNode } from '@theia/core/lib/browser';
 import URI from '@theia/core/lib/common/uri';
 import { OutlineViewService } from './outline-view-service';
-import { OutlineSymbolInformationNode } from './outline-view-widget';
-import { EditorManager } from '@theia/editor/lib/browser';
-import { Disposable } from '@theia/core/lib/common';
-import PerfectScrollbar from 'perfect-scrollbar';
+import { OutlineSymbolInformationNode, OutlineViewWidget } from './outline-view-widget';
+import { Disposable, DisposableCollection } from '@theia/core/lib/common';
 import { UriSelection } from '@theia/core/lib/common';
 
 export const OutlineBreadcrumbType = Symbol('OutlineBreadcrumb');
+export const BreadcrumbPopupOutlineViewFactory = Symbol('BreadcrumbPopupOutlineViewFactory');
+export interface BreadcrumbPopupOutlineViewFactory {
+    (): BreadcrumbPopupOutlineView;
+}
+export class BreadcrumbPopupOutlineView extends OutlineViewWidget {
+    @inject(OpenerService) protected readonly openerService: OpenerService;
+
+    protected handleClickEvent(node: TreeNode | undefined, event: React.MouseEvent<HTMLElement>): void {
+        if (UriSelection.is(node) && OutlineSymbolInformationNode.hasRange(node)) {
+            open(this.openerService, node.uri, { selection: node.range });
+        } else {
+            super.handleClickEvent(node, event);
+        }
+    }
+
+    cloneState(roots: OutlineSymbolInformationNode[]): void {
+        const nodes = this.reconcileTreeState(roots);
+        const root = this.getRoot(nodes);
+        this.model.root = this.inflateFromStorage(this.deflateForStorage(root));
+        this.model.collapseAll();
+    }
+}
 
 @injectable()
 export class OutlineBreadcrumbsContribution implements BreadcrumbsContribution {
-
     @inject(LabelProvider)
     protected readonly labelProvider: LabelProvider;
 
@@ -42,8 +60,10 @@ export class OutlineBreadcrumbsContribution implements BreadcrumbsContribution {
     @inject(BreadcrumbsService)
     protected readonly breadcrumbsService: BreadcrumbsService;
 
-    @inject(EditorManager)
-    protected readonly editorManager: EditorManager;
+    @inject(BreadcrumbPopupOutlineViewFactory)
+    protected readonly outlineFactory: BreadcrumbPopupOutlineViewFactory;
+
+    protected outlineView: BreadcrumbPopupOutlineView;
 
     readonly type = OutlineBreadcrumbType;
     readonly priority: number = 200;
@@ -54,6 +74,9 @@ export class OutlineBreadcrumbsContribution implements BreadcrumbsContribution {
 
     @postConstruct()
     init(): void {
+        this.outlineView = this.outlineFactory();
+        this.outlineView.node.style.height = 'auto';
+        this.outlineView.node.style.maxHeight = '200px';
         this.outlineViewService.onDidChangeOutline(roots => {
             if (roots.length > 0) {
                 this.roots = roots;
@@ -78,7 +101,7 @@ export class OutlineBreadcrumbsContribution implements BreadcrumbsContribution {
         const outlinePath = this.toOutlinePath(selectedNode);
         if (outlinePath && selectedNode) {
             this.currentBreadcrumbs = outlinePath.map((node, index) =>
-                new OutlineBreadcrumb(node, uri, index.toString(), node.name!, 'symbol-icon symbol-icon-center ' + node.iconClass)
+                new OutlineBreadcrumb(node, uri, index.toString(), this.labelProvider.getName(node), 'symbol-icon symbol-icon-center ' + node.iconClass)
             );
             if (selectedNode.children && selectedNode.children.length > 0) {
                 this.currentBreadcrumbs.push(new OutlineBreadcrumb(selectedNode.children as OutlineSymbolInformationNode[],
@@ -104,53 +127,24 @@ export class OutlineBreadcrumbsContribution implements BreadcrumbsContribution {
         if (!OutlineBreadcrumb.is(breadcrumb)) {
             return undefined;
         }
-        const nodes = Array.isArray(breadcrumb.node) ? breadcrumb.node : this.siblings(breadcrumb.node);
-        const items = nodes.map(node => ({
-            label: node.name!,
-            title: node.name!,
-            iconClass: 'symbol-icon symbol-icon-center ' + node.iconClass,
-            action: () => this.revealInEditor(node)
-        }));
-        if (items.length > 0) {
-            ReactDOM.render(<React.Fragment>{this.renderItems(items)}</React.Fragment>, parent);
-            const scrollbar = new PerfectScrollbar(parent, {
-                handlers: ['drag-thumb', 'keyboard', 'wheel', 'touch'],
-                useBothWheelAxes: true,
-                scrollYMarginOffset: 8,
-                suppressScrollX: true
-            });
-            return {
-                dispose: () => {
-                    scrollbar.destroy();
-                    ReactDOM.unmountComponentAtNode(parent);
-
-                }
-            };
-        }
-        const noContent = document.createElement('div');
-        noContent.style.margin = '.5rem';
-        noContent.style.fontStyle = 'italic';
-        noContent.innerText = '(no content)';
-        parent.appendChild(noContent);
-    }
-
-    protected revealInEditor(node: OutlineSymbolInformationNode): void {
-        if (OutlineSymbolInformationNode.hasRange(node) && this.currentUri) {
-            this.editorManager.open(this.currentUri, { selection: node.range });
+        const node = Array.isArray(breadcrumb.node) ? breadcrumb.node[0] : breadcrumb.node;
+        if (!node.parent) {
+            return undefined;
         }
-    }
-
-    protected renderItems(items: { label: string, title: string, iconClass: string, action: () => void }[]): React.ReactNode {
-        return <ul>
-            {items.map((item, index) => <li key={index} title={item.title} onClick={_ => item.action()}>
-                <span className={item.iconClass}></span> <span>{item.label}</span>
-            </li>)}
-        </ul>;
-    }
-
-    protected siblings(node: OutlineSymbolInformationNode): OutlineSymbolInformationNode[] {
-        if (!node.parent) { return []; }
-        return node.parent.children.filter(n => n !== node).map(n => n as OutlineSymbolInformationNode);
+        const siblings = node.parent.children.filter((child): child is OutlineSymbolInformationNode => OutlineSymbolInformationNode.is(child));
+
+        const toDisposeOnHide = new DisposableCollection();
+        this.outlineView.cloneState(siblings);
+        this.outlineView.model.selectNode(node);
+        Widget.attach(this.outlineView, parent);
+        toDisposeOnHide.pushAll([
+            this.outlineView.model.onExpansionChanged(expandedNode => SelectableTreeNode.is(expandedNode) && this.outlineView.model.selectNode(expandedNode)),
+            Disposable.create(() => {
+                this.outlineView.model.root = undefined;
+                Widget.detach(this.outlineView);
+            }),
+        ]);
+        return toDisposeOnHide;
     }
 
     /**