fixed memory leak; JLIcon nows cleans up after itself when passed to …

…phosphor
jupyterlab · Oct 14, 2019 · 45848ba · 45848ba
1 parent b2fffcd
commit 45848ba
Show file tree

Hide file tree

Showing 10 changed files with 46 additions and 40 deletions.
diff --git a/packages/apputils/src/mainareawidget.ts b/packages/apputils/src/mainareawidget.ts
@@ -174,7 +174,7 @@ export class MainAreaWidget<T extends Widget = Widget> extends Widget
     this.title.mnemonic = content.title.mnemonic;
     this.title.iconClass = content.title.iconClass;
     this.title.iconLabel = content.title.iconLabel;
-    this.title.iconRender = content.title.iconRender;
+    this.title.iconRenderer = content.title.iconRenderer;
     this.title.caption = content.title.caption;
     this.title.className = content.title.className;
     this.title.dataset = content.title.dataset;
@@ -194,7 +194,7 @@ export class MainAreaWidget<T extends Widget = Widget> extends Widget
     content.title.mnemonic = this.title.mnemonic;
     content.title.iconClass = this.title.iconClass;
     content.title.iconLabel = this.title.iconLabel;
-    content.title.iconRender = this.title.iconRender;
+    content.title.iconRenderer = this.title.iconRenderer;
     content.title.caption = this.title.caption;
     content.title.className = this.title.className;
     content.title.dataset = this.title.dataset;

diff --git a/packages/csvviewer-extension/src/index.ts b/packages/csvviewer-extension/src/index.ts
@@ -130,7 +130,7 @@ function activateCsv(
     if (ft) {
       widget.title.iconClass = ft.iconClass!;
       widget.title.iconLabel = ft.iconLabel!;
-      widget.title.iconRender = ft.iconRender!;
+      widget.title.iconRenderer = ft.iconRenderer!;
     }
     // Set the theme for the new widget.
     widget.content.style = style;
@@ -210,7 +210,7 @@ function activateTsv(
     if (ft) {
       widget.title.iconClass = ft.iconClass!;
       widget.title.iconLabel = ft.iconLabel!;
-      widget.title.iconRender = ft.iconRender!;
+      widget.title.iconRenderer = ft.iconRenderer!;
     }
     // Set the theme for the new widget.
     widget.content.style = style;

diff --git a/packages/docregistry/src/mimedocument.ts b/packages/docregistry/src/mimedocument.ts
@@ -287,7 +287,7 @@ export class MimeDocumentFactory extends ABCWidgetFactory<MimeDocument> {
 
     content.title.iconClass = ft.iconClass;
     content.title.iconLabel = ft.iconLabel;
-    content.title.iconRender = ft.iconRender;
+    content.title.iconRenderer = ft.iconRenderer;
 
     const widget = new MimeDocument({ content, context });
 

diff --git a/packages/docregistry/src/registry.ts b/packages/docregistry/src/registry.ts
@@ -1180,7 +1180,7 @@ export namespace DocumentRegistry {
      */
     readonly iconLabel?: string;
 
-    readonly iconRender?: JLIcon.IPhosphor;
+    readonly iconRenderer?: JLIcon;
 
     /**
      * The content type of the new file.
@@ -1238,7 +1238,7 @@ export namespace DocumentRegistry {
     name: 'text',
     mimeTypes: ['text/plain'],
     extensions: ['.txt'],
-    iconRender: fileIcon.phosphor({ kind: 'mainAreaTab', center: true })
+    iconRenderer: fileIcon.bindStyle({ kind: 'mainAreaTab', center: true })
   };
 
   /**
@@ -1252,7 +1252,7 @@ export namespace DocumentRegistry {
     extensions: ['.ipynb'],
     contentType: 'notebook',
     fileFormat: 'json',
-    iconRender: notebookIcon.phosphor({ kind: 'mainAreaTab', center: true })
+    iconRenderer: notebookIcon.bindStyle({ kind: 'mainAreaTab', center: true })
   };
 
   /**
@@ -1264,7 +1264,7 @@ export namespace DocumentRegistry {
     extensions: [],
     mimeTypes: ['text/directory'],
     contentType: 'directory',
-    iconRender: folderIcon.phosphor({ kind: 'mainAreaTab', center: true })
+    iconRenderer: folderIcon.bindStyle({ kind: 'mainAreaTab', center: true })
   };
 
   /**
@@ -1279,28 +1279,31 @@ export namespace DocumentRegistry {
       displayName: 'Markdown File',
       extensions: ['.md'],
       mimeTypes: ['text/markdown'],
-      iconRender: markdownIcon.phosphor({ kind: 'mainAreaTab', center: true })
+      iconRenderer: markdownIcon.bindStyle({
+        kind: 'mainAreaTab',
+        center: true
+      })
     },
     {
       name: 'python',
       displayName: 'Python File',
       extensions: ['.py'],
       mimeTypes: ['text/x-python'],
-      iconRender: pythonIcon.phosphor({ kind: 'mainAreaTab', center: true })
+      iconRenderer: pythonIcon.bindStyle({ kind: 'mainAreaTab', center: true })
     },
     {
       name: 'json',
       displayName: 'JSON File',
       extensions: ['.json'],
       mimeTypes: ['application/json'],
-      iconRender: jsonIcon.phosphor({ kind: 'mainAreaTab', center: true })
+      iconRenderer: jsonIcon.bindStyle({ kind: 'mainAreaTab', center: true })
     },
     {
       name: 'csv',
       displayName: 'CSV File',
       extensions: ['.csv'],
       mimeTypes: ['text/csv'],
-      iconRender: spreadsheetIcon.phosphor({
+      iconRenderer: spreadsheetIcon.bindStyle({
         kind: 'mainAreaTab',
         center: true
       })
@@ -1310,7 +1313,7 @@ export namespace DocumentRegistry {
       displayName: 'TSV File',
       extensions: ['.tsv'],
       mimeTypes: ['text/csv'],
-      iconRender: spreadsheetIcon.phosphor({
+      iconRenderer: spreadsheetIcon.bindStyle({
         kind: 'mainAreaTab',
         center: true
       })
@@ -1320,21 +1323,21 @@ export namespace DocumentRegistry {
       displayName: 'R File',
       mimeTypes: ['text/x-rsrc'],
       extensions: ['.r'],
-      iconRender: rKernelIcon.phosphor({ kind: 'mainAreaTab', center: true })
+      iconRenderer: rKernelIcon.bindStyle({ kind: 'mainAreaTab', center: true })
     },
     {
       name: 'yaml',
       displayName: 'YAML File',
       mimeTypes: ['text/x-yaml', 'text/yaml'],
       extensions: ['.yaml', '.yml'],
-      iconRender: yamlIcon.phosphor({ kind: 'mainAreaTab', center: true })
+      iconRenderer: yamlIcon.bindStyle({ kind: 'mainAreaTab', center: true })
     },
     {
       name: 'svg',
       displayName: 'Image',
       mimeTypes: ['image/svg+xml'],
       extensions: ['.svg'],
-      iconRender: imageIcon.phosphor({ kind: 'mainAreaTab', center: true }),
+      iconRenderer: imageIcon.bindStyle({ kind: 'mainAreaTab', center: true }),
       fileFormat: 'base64'
     },
     {
@@ -1343,39 +1346,39 @@ export namespace DocumentRegistry {
       mimeTypes: ['image/tiff'],
       extensions: ['.tif', '.tiff'],
       iconClass: 'jp-MaterialIcon jp-ImageIcon',
-      iconRender: imageIcon.phosphor({ kind: 'mainAreaTab', center: true }),
+      iconRenderer: imageIcon.bindStyle({ kind: 'mainAreaTab', center: true }),
       fileFormat: 'base64'
     },
     {
       name: 'jpeg',
       displayName: 'Image',
       mimeTypes: ['image/jpeg'],
       extensions: ['.jpg', '.jpeg'],
-      iconRender: imageIcon.phosphor({ kind: 'mainAreaTab', center: true }),
+      iconRenderer: imageIcon.bindStyle({ kind: 'mainAreaTab', center: true }),
       fileFormat: 'base64'
     },
     {
       name: 'gif',
       displayName: 'Image',
       mimeTypes: ['image/gif'],
       extensions: ['.gif'],
-      iconRender: imageIcon.phosphor({ kind: 'mainAreaTab', center: true }),
+      iconRenderer: imageIcon.bindStyle({ kind: 'mainAreaTab', center: true }),
       fileFormat: 'base64'
     },
     {
       name: 'png',
       displayName: 'Image',
       mimeTypes: ['image/png'],
       extensions: ['.png'],
-      iconRender: imageIcon.phosphor({ kind: 'mainAreaTab', center: true }),
+      iconRenderer: imageIcon.bindStyle({ kind: 'mainAreaTab', center: true }),
       fileFormat: 'base64'
     },
     {
       name: 'bmp',
       displayName: 'Image',
       mimeTypes: ['image/bmp'],
       extensions: ['.bmp'],
-      iconRender: imageIcon.phosphor({ kind: 'mainAreaTab', center: true }),
+      iconRenderer: imageIcon.bindStyle({ kind: 'mainAreaTab', center: true }),
       fileFormat: 'base64'
     }
   ];

diff --git a/packages/filebrowser/src/listing.ts b/packages/filebrowser/src/listing.ts
@@ -1815,9 +1815,9 @@ export namespace DirListing {
       if (fileType) {
         // TODO: remove workaround if...else/code in else clause in v2.0.0
         // workaround for 1.0.x versions of Jlab pulling in 1.1.x versions of filebrowser
-        if (fileType.iconRender) {
+        if (fileType.iconRenderer) {
           // add icon as svg node. Can be styled using CSS
-          fileType.iconRender(icon, {
+          fileType.iconRenderer.render(icon, {
             className: ITEM_ICON_CLASS,
             container: icon,
             title: fileType.iconLabel,

diff --git a/packages/fileeditor/src/widget.ts b/packages/fileeditor/src/widget.ts
@@ -326,7 +326,7 @@ export class FileEditorFactory extends ABCWidgetFactory<
       mimeTypeService: this._services.mimeTypeService
     });
 
-    content.title.iconRender = textEditorIcon.phosphor({
+    content.title.iconRenderer = textEditorIcon.bindStyle({
       kind: 'mainAreaTab',
       center: true
     });

diff --git a/packages/imageviewer-extension/src/index.ts b/packages/imageviewer-extension/src/index.ts
@@ -107,7 +107,7 @@ function activate(
     if (types.length > 0) {
       widget.title.iconClass = types[0].iconClass;
       widget.title.iconLabel = types[0].iconLabel;
-      widget.title.iconRender = types[0].iconRender;
+      widget.title.iconRenderer = types[0].iconRenderer;
     }
   });
 

diff --git a/packages/markdownviewer/src/widget.ts b/packages/markdownviewer/src/widget.ts
@@ -309,7 +309,7 @@ export class MarkdownViewerFactory extends ABCWidgetFactory<MarkdownDocument> {
     const content = new MarkdownViewer({ context, renderer });
     content.title.iconClass = this._fileType.iconClass;
     content.title.iconLabel = this._fileType.iconLabel;
-    content.title.iconRender = this._fileType.iconRender;
+    content.title.iconRenderer = this._fileType.iconRenderer;
     const widget = new MarkdownDocument({ content, context });
 
     return widget;

diff --git a/packages/notebook-extension/src/index.ts b/packages/notebook-extension/src/index.ts
@@ -589,7 +589,7 @@ function activateNotebookHandler(
     widget.id = widget.id || `notebook-${++id}`;
     widget.title.iconClass = ft.iconClass;
     widget.title.iconLabel = ft.iconLabel;
-    widget.title.iconRender = ft.iconRender;
+    widget.title.iconRenderer = ft.iconRenderer;
     // Notify the widget tracker if restore data needs to update.
     widget.context.pathChanged.connect(() => {
       void tracker.save(widget);

diff --git a/packages/ui-components/src/icon/jlicon.tsx b/packages/ui-components/src/icon/jlicon.tsx
@@ -12,6 +12,7 @@ export class JLIcon {
   constructor(
     readonly name: string,
     readonly svgstr: string,
+    readonly style: IIconStyle = {},
     protected _debug: boolean = false
   ) {}
 
@@ -51,9 +52,10 @@ export class JLIcon {
     tag = 'div',
     ...propsStyle
   }: JLIcon.IProps = {}): HTMLElement | null {
+    const propsStyleComb = { ...this.style, ...propsStyle };
     const classNames = classes(
       className,
-      propsStyle ? iconStyle(propsStyle) : ''
+      propsStyleComb ? iconStyle(propsStyleComb) : ''
     );
 
     // ensure that svg html is valid
@@ -75,11 +77,16 @@ export class JLIcon {
     return svgElement;
   }
 
-  phosphor(props: JLIcon.IProps = {}): JLIcon.IPhosphor {
-    return (host: HTMLElement, innerProps: JLIcon.IProps = {}) => {
-      const comb = { ...props, ...innerProps };
-      return ReactDOM.render(<this.react {...comb} container={host} />, host);
-    };
+  render(host: HTMLElement, props: JLIcon.IProps = {}): void {
+    return ReactDOM.render(<this.react {...props} container={host} />, host);
+  }
+
+  unrender(host: HTMLElement): void {
+    ReactDOM.unmountComponentAtNode(host);
+  }
+
+  bindStyle(propsStyle: IIconStyle = {}): JLIcon {
+    return new JLIcon(this.name, this.svgstr, propsStyle, this._debug);
   }
 
   protected _initReact() {
@@ -95,9 +102,10 @@ export class JLIcon {
         ref: React.RefObject<SVGElement>
       ) => {
         const Tag = tag;
+        const propsStyleComb = { ...this.style, ...propsStyle };
         const classNames = classes(
           className,
-          propsStyle ? iconStyle(propsStyle) : ''
+          propsStyleComb ? iconStyle(propsStyleComb) : ''
         );
 
         // ensure that svg html is valid
@@ -160,11 +168,6 @@ export namespace JLIcon {
      */
     title?: string;
   }
-
-  export type IPhosphor = (
-    host: HTMLElement,
-    innerProps?: JLIcon.IProps
-  ) => void;
 }
 
 namespace Private {