removed override hack, improved error handling

packages/application/src/shell.ts

@@ -5,7 +5,7 @@ import { Debouncer } from '@jupyterlab/coreutils';
 
 import { DocumentRegistry } from '@jupyterlab/docregistry';
 
-import { defaultIconRegistry } from '@jupyterlab/ui-components';
+import { TabBarSvg } from '@jupyterlab/ui-components';
 
 import { ArrayExt, find, IIterator, iter, toArray } from '@phosphor/algorithm';
 
@@ -15,8 +15,6 @@ import { Message, MessageLoop, IMessageHandler } from '@phosphor/messaging';
 
 import { ISignal, Signal } from '@phosphor/signaling';
 
-import { h, VirtualElement } from '@phosphor/virtualdom';
-
 import {
   BoxLayout,
   BoxPanel,
@@ -1024,44 +1022,44 @@ namespace Private {
     });
   }
 
-  class SideBar extends TabBar<Widget> {
-    /**
-     * A message handler invoked on an `'update-request'` message.
-     */
-    protected onUpdateRequest(msg: Message): void {
-      super.onUpdateRequest(msg);
-
-      for (let itab in this.contentNode.children) {
-        let tab = this.contentNode.children[itab];
-        let title = this.titles[itab];
-        let iconNode = tab.children ? (tab.children[0] as HTMLElement) : null;
-
-        if (iconNode && iconNode.children.length < 1) {
-          // add the svg node, if not already present
-          defaultIconRegistry.icon({
-            name: title.iconClass,
-            container: iconNode,
-            center: true,
-            kind: 'sideBar'
-          });
-        }
-      }
-    }
-  }
-
-  class SideBarRenderer extends TabBar.Renderer {
-    /**
-     * Render the icon element for a tab.
-     *
-     * @param data - The data to use for rendering the tab.
-     *
-     * @returns A virtual element representing the tab icon.
-     */
-    renderIcon(data: TabBar.IRenderData<any>): VirtualElement {
-      let className = this.createIconClass(data);
-      return h.div({ className });
-    }
-  }
+  // class SideBar extends TabBar<Widget> {
+  //   /**
+  //    * A message handler invoked on an `'update-request'` message.
+  //    */
+  //   protected onUpdateRequest(msg: Message): void {
+  //     super.onUpdateRequest(msg);
+  //
+  //     for (let itab in this.contentNode.children) {
+  //       let tab = this.contentNode.children[itab];
+  //       let title = this.titles[itab];
+  //       let iconNode = tab.children ? (tab.children[0] as HTMLElement) : null;
+  //
+  //       if (iconNode && iconNode.children.length < 1) {
+  //         // add the svg node, if not already present
+  //         defaultIconRegistry.icon({
+  //           name: title.iconClass,
+  //           container: iconNode,
+  //           center: true,
+  //           kind: 'sideBar'
+  //         });
+  //       }
+  //     }
+  //   }
+  // }
+  //
+  // class SideBarRenderer extends TabBar.Renderer {
+  //   /**
+  //    * Render the icon element for a tab.
+  //    *
+  //    * @param data - The data to use for rendering the tab.
+  //    *
+  //    * @returns A virtual element representing the tab icon.
+  //    */
+  //   renderIcon(data: TabBar.IRenderData<any>): VirtualElement {
+  //     let className = this.createIconClass(data);
+  //     return h.div({ className });
+  //   }
+  // }
 
   /**
    * A class which manages a side bar and related stacked panel.
@@ -1071,11 +1069,11 @@ namespace Private {
      * Construct a new side bar handler.
      */
     constructor(side: string) {
-      this._sideBar = new SideBar({
+      this._sideBar = new TabBarSvg<Widget>({
+        kind: 'sideBar',
         insertBehavior: 'none',
         removeBehavior: 'none',
-        allowDeselect: true,
-        renderer: new SideBarRenderer()
+        allowDeselect: true
       });
       this._stackedPanel = new StackedPanel();
       this._sideBar.hide();

packages/filebrowser/src/listing.ts

@@ -1771,13 +1771,13 @@ export namespace DirListing {
             kind: 'listing'
           });
 
-          // filthy hack to get the topbar tab icons
-          defaultIconRegistry.override({
-            name: fileType.iconName,
-            className: ['p-TabBar-tabIcon', fileType.iconClass].join(' '),
-            center: true,
-            kind: 'tab'
-          });
+          // // filthy hack to get the topbar tab icons
+          // defaultIconRegistry.override({
+          //   name: fileType.iconName,
+          //   className: ['p-TabBar-tabIcon', fileType.iconClass].join(' '),
+          //   center: true,
+          //   kind: 'tab'
+          // });
         } else {
           // add icon as CSS background image. Can't be styled using CSS
           icon.textContent = fileType.iconLabel || '';

packages/ui-components/package.json

@@ -36,6 +36,9 @@
     "@blueprintjs/core": "^3.9.0",
     "@blueprintjs/icons": "^3.3.0",
     "@blueprintjs/select": "^3.3.0",
+    "@phosphor/messaging": "^1.2.2",
+    "@phosphor/virtualdom": "^1.1.2",
+    "@phosphor/widgets": "^1.7.0",
     "react": "~16.8.4",
     "typestyle": "^2.0.1"
   },

packages/ui-components/src/icon/icon.ts

@@ -0,0 +1,6 @@
+export type iconKindType =
+  | 'listing'
+  | 'sideBar'
+  | 'statusBar'
+  | 'tab'
+  | 'unset';

packages/ui-components/src/icon/iconregistry.tsx

@@ -83,11 +83,19 @@ export class IconRegistry {
     });
   }
 
-  classNameToName(className: string): string {
-    // for now, just assume that the first className is the relevant one
-    let name = className.split(/\s+/)[0];
+  resolveName(name: string): string {
+    if (!(name in this._svg)) {
+      // assume name is really a className, split the className into parts and check each part
+      for (let className of name.split(/\s+/)) {
+        if (className in this._classNameToName) {
+          return this._nameToClassName[className];
+        }
+      }
+      // couldn't resolve name, mark as bad
+      return 'bad';
+    }
 
-    return name in this._classNameToName ? this._classNameToName[name] : 'bad';
+    return name;
   }
 
   /**
@@ -96,13 +104,15 @@ export class IconRegistry {
   icon(
     props: IconRegistry.IIconOptions & { container: HTMLElement } & IIconStyle
   ): HTMLElement {
-    const { name, className, title, container, ...propsStyle } = props;
+    const { name, className, title, skipbad, container, ...propsStyle } = props;
 
     // if name not in _svg, assume we've been handed a className in place of name
-    let svg =
-      !(name in this._svg) && name.startsWith('jp-')
-        ? this.svg(this.classNameToName(name))
-        : this.svg(name);
+    let svg = this.svg(name, skipbad);
+    if (!svg) {
+      // bail
+      return;
+    }
+
     let svgNode = Private.parseSvg(svg);
 
     if (title) {
@@ -140,41 +150,61 @@ export class IconRegistry {
   iconReact(
     props: IconRegistry.IIconOptions & { tag?: 'div' | 'span' } & IIconStyle
   ): React.ReactElement {
-    const { name, className, title, tag, ...propsStyle } = props;
+    const { name, className, title, skipbad, tag, ...propsStyle } = props;
     const Tag = tag || 'div';
 
+    let svg = this.svg(name, skipbad);
+    if (!svg) {
+      // bail
+      return;
+    }
+
     return (
       <Tag
         className={classes(className, propsStyle ? iconStyle(propsStyle) : '')}
-        dangerouslySetInnerHTML={{ __html: this.svg(name) }}
+        dangerouslySetInnerHTML={{ __html: svg }}
       />
     );
   }
 
-  override(
-    props: { className: string; name?: string; title?: string } & IIconStyle
-  ) {
-    const { name, className, title, ...propsStyle } = props;
-
-    for (let container of document.getElementsByClassName(
-      className
-    ) as HTMLCollectionOf<HTMLElement>) {
-      this.icon({
-        name: name ? name : this.classNameToName(className),
-        title: title,
-        container: container,
-        ...propsStyle
-      });
-    }
-  }
-
-  svg(name: string): string {
-    if (!(name in this._svg)) {
-      console.error(`Invalid icon name: ${name}`);
-      return this._svg['bad'];
+  // override(
+  //   props: { className: string; name?: string; title?: string } & IIconStyle
+  // ) {
+  //   let { name, className, title, ...propsStyle } = props;
+  //
+  //   // try to resolve name
+  //   name = name ? name : this.classNameToName(className);
+  //   if (!name) {
+  //     // bail
+  //     return;
+  //   }
+  //
+  //   for (let container of document.getElementsByClassName(
+  //     className
+  //   ) as HTMLCollectionOf<HTMLElement>) {
+  //     this.icon({
+  //       name: name,
+  //       title: title,
+  //       container: container,
+  //       ...propsStyle
+  //     });
+  //   }
+  // }
+
+  svg(name: string, skipbad: boolean = false): string {
+    let svgname = this.resolveName(name);
+
+    if (name === 'bad') {
+      if (!skipbad) {
+        // log a warning and mark missing icons with an X
+        console.error(`Invalid icon name: ${name}`);
+      } else {
+        // silently return empty string
+        return '';
+      }
     }
 
-    return this._svg[name];
+    return this._svg[svgname];
   }
 
   static iconClassName(name: string): string {
@@ -211,6 +241,7 @@ export namespace IconRegistry {
     name: string;
     className?: string;
     title?: string;
+    skipbad?: boolean;
   }
 
   // needs the explicit type to avoid a typedoc issue

packages/ui-components/src/icon/index.ts

@@ -2,3 +2,5 @@
 // Distributed under the terms of the Modified BSD License.
 
 export * from './icon';
+export * from './iconregistry';
+export * from './tabbarsvg';

packages/ui-components/src/icon/tabbarsvg.ts

@@ -2,22 +2,26 @@ import { Message } from '@phosphor/messaging';
 
 import { h, VirtualElement } from '@phosphor/virtualdom';
 
-import { TabBar, Widget } from '@phosphor/widgets';
+import { TabBar } from '@phosphor/widgets';
 
-import { defaultIconRegistry } from './icon';
+import { iconKindType } from './icon';
+import { defaultIconRegistry } from './iconregistry';
 
-export class TabBarSvg extends TabBar<Widget> {
+export class TabBarSvg<T> extends TabBar<T> {
   /**
    * Construct a new tab bar. Sets the (icon) kind and overrides
    * the default renderer.
    *
    * @param options - The options for initializing the tab bar.
    */
-  constructor(options: { kind: string } & TabBar.IOptions<Widget>) {
+  constructor(
+    options: { kind: iconKindType; skipbad?: boolean } & TabBar.IOptions<T>
+  ) {
     options.renderer = options.renderer || TabBarSvg.defaultRenderer;
     super(options);
 
     this._kind = options.kind;
+    this._skipbad = options.skipbad;
   }
 
   /**
@@ -37,13 +41,15 @@ export class TabBarSvg extends TabBar<Widget> {
           name: title.iconClass,
           container: iconNode,
           center: true,
-          kind: this._kind
+          kind: this._kind,
+          skipbad: this._skipbad
         });
       }
     }
   }
 
-  protected _kind: string;
+  protected _kind: iconKindType;
+  protected _skipbad: boolean;
 }
 
 export namespace TabBarSvg {

packages/ui-components/src/style/icon.ts

@@ -4,7 +4,7 @@
 import { style } from 'typestyle/lib';
 import { NestedCSSProperties } from 'typestyle/lib/types';
 
-type iconKindType = 'listing' | 'sideBar' | 'statusBar' | 'tab' | 'unset';
+import { iconKindType } from '../icon';
 
 export interface IIconStyle extends NestedCSSProperties {
   /**