review

projects/scion/workbench/src/lib/activity-part/activity-part.component.ts

@@ -52,9 +52,6 @@ export class ActivityPartComponent {
   private _panelWidth = PANEL_INITIAL_WIDTH;
   private _panelWidth$ = new Subject<number>();
 
-  @ViewChild('viewport')
-  public viewport!: ElementRef;
-
   @ViewChild('panel', {read: ElementRef})
   private _panelElementRef!: ElementRef;
 

projects/scion/workbench/src/lib/activity-part/activity.ts

@@ -14,43 +14,43 @@ export abstract class Activity {
   /**
    * Specifies the title of the activity.
    */
-  public title?: string;
+  public abstract title: string | null;
 
   /**
    * Specifies CSS class(es) added to the activity item and activity panel, e.g. used for e2e testing.
    */
-  public cssClass?: string | string[];
+  public abstract cssClass: string | string[] | undefined;
 
   /**
    * Specifies the text for the activity item.
    *
    * You can use it in combination with `itemCssClass`, e.g. to render an icon glyph by using its textual name.
    */
-  public itemText?: string;
+  public abstract itemText: string | null;
 
   /**
    * Specifies CSS class(es) added to the activity item, e.g. used for e2e testing or to set an icon font class.
    */
-  public itemCssClass?: string | string[];
+  public abstract itemCssClass: string | string[] | undefined;
   /**
    * Controls whether to open this activity in the activity panel or to open it in a separate view.
    */
-  public target?: 'activity-panel' | 'view';
+  public abstract target: 'activity-panel' | 'view';
 
   /**
    * Controls whether to show or hide this activity. By default, this activity is showing.
    */
-  public visible?: boolean;
+  public abstract visible: boolean;
 
   /**
    * Specifies where to insert this activity in the list of activities.
    */
-  public position?: number;
+  public abstract position: number | undefined;
 
   /**
    * Specifies the number of pixels added to the activity panel width if this is the active activity.
    */
-  public panelWidthDelta?: number;
+  public abstract panelWidthDelta: number;
 
   /**
    * Specifies the routing commands used by Angular router to navigate when this activity is activated.
@@ -99,15 +99,15 @@ export class InternalActivity implements Activity {
   private _active$ = new BehaviorSubject<boolean>(false);
 
   public panelWidthDelta = 0;
-  public title?: string;
-  public cssClass?: string | string[];
+  public title: string | null = null;
+  public cssClass: string | string[] | undefined;
 
-  public itemText?: string;
-  public itemCssClass?: string | string[];
+  public itemText: string | null = null;
+  public itemCssClass: string | string[] | undefined;
 
   public target: 'activity-panel' | 'view' = 'activity-panel';
   public visible = true;
-  public position?: number;
+  public position: number | undefined;
 
   constructor(private _wbRouter: WorkbenchRouter, private _injector: Injector) {
   }

projects/scion/workbench/src/lib/activity-part/wb-activity-action.directive.ts

@@ -10,7 +10,6 @@
 
 import { Directive, OnDestroy, TemplateRef } from '@angular/core';
 import { WorkbenchActivityPartService } from './workbench-activity-part.service';
-import { Activity } from './activity';
 import { ActivatedRoute } from '@angular/router';
 import { Disposable } from '../disposable';
 
@@ -34,18 +33,17 @@ import { Disposable } from '../disposable';
 })
 export class WbActivityActionDirective implements OnDestroy {
 
-  private readonly _activity: Activity | null;
   private readonly _action: Disposable;
 
   constructor(private _template: TemplateRef<void>,
               activityService: WorkbenchActivityPartService,
               route: ActivatedRoute) {
-    this._activity = activityService.getActivityFromRoutingContext(route.snapshot);
-    if (!this._activity) {
+    const activity = activityService.getActivityFromRoutingContext(route.snapshot);
+    if (!activity) {
       throw Error('[RoutingContextError] Route not in the context of an activity');
     }
 
-    this._action = this._activity.registerAction(this._template);
+    this._action = activity.registerAction(this._template);
   }
 
   public ngOnDestroy(): void {

projects/scion/workbench/src/lib/activity-part/wb-activity.directive.ts

@@ -37,7 +37,7 @@ export class WbActivityDirective implements OnInit, OnDestroy {
    * Specifies the title of the activity.
    */
   @Input()
-  public set title(title: string) {
+  public set title(title: string | null) {
     this.activity.title = title;
   }
 
@@ -47,23 +47,23 @@ export class WbActivityDirective implements OnInit, OnDestroy {
    * You can use it in combination with `itemCssClass`, e.g. to render an icon glyph by using its textual name.
    */
   @Input()
-  public set itemText(itemText: string) {
+  public set itemText(itemText: string | null) {
     this.activity.itemText = itemText;
   }
 
   /**
    * Specifies CSS class(es) added to the activity item, e.g. used for e2e testing or to set an icon font class.
    */
   @Input()
-  public set itemCssClass(itemCssClass: string | string[]) {
+  public set itemCssClass(itemCssClass: string | string[] | undefined) {
     this.activity.itemCssClass = itemCssClass;
   }
 
   /**
    * Specifies CSS class(es) added to the activity item and activity panel, e.g. used for e2e testing.
    */
   @Input()
-  public set cssClass(cssClass: string | string[]) {
+  public set cssClass(cssClass: string | string[] | undefined) {
     this.activity.cssClass = cssClass;
   }
 

projects/scion/workbench/src/lib/dom.util.ts

@@ -17,7 +17,8 @@ export function createElement(tag: string, options: ElementCreateOptions): HTMLE
 /**
  * Applies the given style(s) to the given element.
  *
- * To unset a style property provide `null` as its value.
+ * Specify styles to be modified by passing a dictionary containing CSS property names (hyphen case).
+ * To remove a style, set its value to `null`.
  */
 export function setStyle(element: HTMLElement | ElementRef<HTMLElement>, style: { [style: string]: any | null }): void {
   const target = coerceElement(element);

projects/scion/workbench/src/lib/layout/parts-layout.component.ts

@@ -55,12 +55,12 @@ export class PartsLayoutComponent implements OnInit, OnDestroy {
   /**
    * Reference to the root part of the layout. Is only set if the layout root is of the type {@link MPart}.
    */
-  public rootPart!: WbComponentPortal<ViewPartComponent> | null;
+  public rootPart: WbComponentPortal<ViewPartComponent> | null = null;
 
   /**
    * Reference to the root node of the layout. Is only set if the layout root is of the type {@link MTreeNode}.
    */
-  public rootNode!: MTreeNode | null;
+  public rootNode: MTreeNode | null = null;
 
   constructor(private _viewPartRegistry: WorkbenchViewPartRegistry,
               private _layoutService: WorkbenchLayoutService,
@@ -74,7 +74,7 @@ export class PartsLayoutComponent implements OnInit, OnDestroy {
         pairwise(),
         takeUntil(this._destroy$),
       )
-      .subscribe(([prevLayout, curLayout]: [PartsLayout, PartsLayout]) => {
+      .subscribe(([prevLayout, curLayout]: [PartsLayout | null, PartsLayout]) => {
         // Determine the parts which are about to be re-parented in the DOM, and detach them temporarily from the Angular component tree,
         // so that they are not destroyed when being re-parented.
         const reattachFn = this.detachPortalsToBeMovedFromComponentTree(prevLayout, curLayout);
@@ -98,22 +98,29 @@ export class PartsLayoutComponent implements OnInit, OnDestroy {
    *
    * Returns a function to attach the detached portals to the Angular component tree anew.
    */
-  private detachPortalsToBeMovedFromComponentTree(prevLayout: PartsLayout, newLayout: PartsLayout): () => void {
+  private detachPortalsToBeMovedFromComponentTree(prevLayout: PartsLayout | null, currLayout: PartsLayout): () => void {
     if (!prevLayout) {
       return noop; // no current layout in place, so no portal is moved
     }
 
     // For each part, compute its path in the layout tree. The path is the list of all parent node ids.
-    const newPartPaths: Map<string, string[]> = newLayout.parts.reduce((acc, part) => {
+    const currPartPathsByPartId: Map<string, string[]> = currLayout.parts.reduce((acc, part) => {
       return acc.set(part.partId, part.getPath());
     }, new Map<string, string[]>());
 
     // Determine parts to be moved in the layout tree.
     const partPortalsToBeMoved: WbComponentPortal<ViewPartComponent>[] = prevLayout.parts.reduce((acc, prevPart) => {
-      const prevPartPath = prevPart.getPath();
-      const newPartPath = newPartPaths.get(prevPart.partId)!;
+      const prevPartPath: string[] = prevPart.getPath();
+      const currPartPath: string[] | undefined = currPartPathsByPartId.get(prevPart.partId);
 
-      if (!Arrays.isEqual(newPartPath, prevPartPath)) {
+      // If the part is no longer contained in the layout but is still present in the parts registry, detach it so that its views
+      // will not get destroyed when applying the new layout, i.e., when moving all its views to another part.
+      if (!currPartPath) {
+        return acc.concat(this._viewPartRegistry.getElseNull(prevPart.partId)?.portal || []);
+      }
+
+      // If the part is moved in the layout, detach it to not destroy its views.
+      if (!Arrays.isEqual(currPartPath, prevPartPath)) {
         return acc.concat(this._viewPartRegistry.getElseThrow(prevPart.partId).portal);
       }
       return acc;

projects/scion/workbench/src/lib/layout/parts-layout.ts

@@ -208,7 +208,7 @@ export class PartsLayout {
    */
   public serialize(options: { nullIfEmpty: false }): string;
   public serialize(options?: { nullIfEmpty?: true } | {}): string | null;
-  public serialize(options?: { nullIfEmpty?: boolean }): string | null{
+  public serialize(options?: { nullIfEmpty?: boolean }): string | null {
     if ((options?.nullIfEmpty ?? true) && this._root instanceof MPart && this._root.isEmpty()) {
       return null;
     }
@@ -264,13 +264,12 @@ export class PartsLayout {
   private _removePart(partId: string): this {
     assertNotNullish(partId, {orElseThrow: () => Error(`[PartsLayoutError] PartId must not be 'null' or 'undefined'.`)});
 
-    const part = this._findPart(partId, {orElseThrow: true});
-
     // The last part is never removed.
     if (this.parts.length === 1) {
       return this;
     }
 
+    const part = this._findPart(partId, {orElseThrow: true});
     const partIndex = this.parts.indexOf(part);
     const siblingElement = (part.parent!.child1 === part ? part.parent!.child2 : part.parent!.child1);
     if (!part.parent!.parent) {

projects/scion/workbench/src/lib/layout/workbench-layout.service.ts

@@ -47,9 +47,9 @@ export class WorkbenchLayoutService {
    */
   public readonly layout$: Observable<PartsLayout> = this._layoutChange$
     .pipe(
-      startWith(null! as PartsLayout),
-      map(() => this.layout),
-      filter((layout): layout is PartsLayout => Boolean(layout)),
+      startWith(undefined as void),
+      map<void, PartsLayout | null>(() => this.layout),
+      filter((layout: PartsLayout | null): layout is PartsLayout => layout !== null),
     );
 
   constructor(viewDragService: ViewDragService, private _zone: NgZone) {
@@ -95,7 +95,7 @@ export class WorkbenchLayoutService {
    * Returns a reference to current {@link PartsLayout}, if any. Is `null` until the initial navigation is performed.
    */
   public get layout(): PartsLayout | null {
-    return this._layout || null;
+    return this._layout;
   }
 
   /**

projects/scion/workbench/src/lib/logging/logger.ts

@@ -12,7 +12,9 @@ import { Inject, Injectable, OnDestroy, Optional } from '@angular/core';
 import { NavigationStart, ParamMap, Router, RouterEvent } from '@angular/router';
 import { filter, map, startWith, takeUntil } from 'rxjs/operators';
 import { Observable, Subject } from 'rxjs';
-import { LogAppender, LogEvent, LoggerName, LogLevel, LogLevelStrings } from './logging.model';
+import { LogAppender, LogEvent, LoggerName, LogLevel } from './logging.model';
+
+type LogLevelStrings = keyof typeof LogLevel;
 
 /**
  * Logger used by the workbench to log messages.
@@ -130,8 +132,8 @@ export class ɵLogger implements Logger, OnDestroy {  // tslint:disable-line:cla
         startWith(router.url),
         filter((event): event is NavigationStart => event instanceof NavigationStart),
         map<RouterEvent, ParamMap>(routerEvent => router.parseUrl(routerEvent.url).queryParamMap),
-        map(queryParamMap => queryParamMap.get('loglevel')?.toUpperCase() as LogLevelStrings),
-        map(logLevelRaw => logLevelRaw ? LogLevel[logLevelRaw] : undefined),
+        map(queryParamMap => queryParamMap.get('loglevel')?.toUpperCase() as LogLevelStrings | undefined),
+        map(logLevelString => logLevelString ? LogLevel[logLevelString] : undefined),
       );
   }
 

projects/scion/workbench/src/lib/logging/logging.model.ts

@@ -16,7 +16,6 @@ import { Injectable } from '@angular/core';
 export enum LogLevel {
   DEBUG, INFO, WARN, ERROR
 }
-export type LogLevelStrings = keyof typeof LogLevel;
 
 /**
  * Provides information about a message to be logged.

projects/scion/workbench/src/lib/message-box/message-box.component.ts

@@ -36,16 +36,16 @@ export class MessageBoxComponent implements OnInit, OnDestroy {
   private _cancelBlinkTimer$ = new Subject<void>();
   private _activeActionButton: HTMLElement | undefined;
 
-  public portal!: ComponentPortal<any>;
+  public portal: ComponentPortal<any> | undefined;
 
   @HostBinding('style.transform')
-  public transform!: string;
+  public transform: string | undefined;
 
   @HostBinding('class.blinking')
-  public blinking!: boolean;
+  public blinking = false;
 
   @HostBinding('class.text-selectable')
-  public textSelectable!: boolean;
+  public textSelectable = false;
 
   @HostBinding('attr.tabindex')
   public tabindex = -1;

projects/scion/workbench/src/lib/message-box/message-box.service.ts

@@ -46,7 +46,7 @@ export class MessageBoxService implements OnDestroy {
 
   private _destroy$ = new Subject<void>();
   private _messageBoxes$ = new BehaviorSubject<ɵMessageBox[]>([]);
-  private _messageBoxServiceHierarchy: MessageBoxService[];
+  private _messageBoxServiceHierarchy: [MessageBoxService, ...MessageBoxService[]]; // minItems: 1
 
   constructor(@Optional() @SkipSelf() private _parentMessageBoxService: MessageBoxService,
               @Optional() private _view: WorkbenchView,
@@ -133,13 +133,13 @@ export class MessageBoxService implements OnDestroy {
   /**
    * Returns the message box service hierarchy.
    */
-  private computeMessageBoxServiceHierarchy(): MessageBoxService[] {
+  private computeMessageBoxServiceHierarchy(): [MessageBoxService, ...MessageBoxService[]] {
     const hierarchy: MessageBoxService[] = [];
     let current: MessageBoxService = this;
     do {
       hierarchy.push(current);
     } while ((current = current._parentMessageBoxService)); // tslint:disable-line:no-conditional-assignment
-    return hierarchy;
+    return hierarchy as [MessageBoxService, ...MessageBoxService[]];
   }
 
   /**

projects/scion/workbench/src/lib/message-box/move.directive.ts

@@ -20,8 +20,8 @@ import { first, takeUntil } from 'rxjs/operators';
 export class MoveDirective implements OnDestroy {
 
   private _destroy$ = new Subject<void>();
-  private _x!: number;
-  private _y!: number;
+  private _x = 0;
+  private _y = 0;
 
   @Output()
   public wbMoveStart = new EventEmitter<void>();