From 45848bad3d33a1a473c029999892de02be0f1f67 Mon Sep 17 00:00:00 2001 From: telamonian Date: Mon, 14 Oct 2019 16:14:24 -0400 Subject: [PATCH] fixed memory leak; JLIcon nows cleans up after itself when passed to phosphor --- packages/apputils/src/mainareawidget.ts | 4 +-- packages/csvviewer-extension/src/index.ts | 4 +-- packages/docregistry/src/mimedocument.ts | 2 +- packages/docregistry/src/registry.ts | 37 +++++++++++---------- packages/filebrowser/src/listing.ts | 4 +-- packages/fileeditor/src/widget.ts | 2 +- packages/imageviewer-extension/src/index.ts | 2 +- packages/markdownviewer/src/widget.ts | 2 +- packages/notebook-extension/src/index.ts | 2 +- packages/ui-components/src/icon/jlicon.tsx | 27 ++++++++------- 10 files changed, 46 insertions(+), 40 deletions(-) diff --git a/packages/apputils/src/mainareawidget.ts b/packages/apputils/src/mainareawidget.ts index 7041ae306a25..132999efcad6 100644 --- a/packages/apputils/src/mainareawidget.ts +++ b/packages/apputils/src/mainareawidget.ts @@ -174,7 +174,7 @@ export class MainAreaWidget 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 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 index d6ce60e22ad0..14a007bfb3c0 100644 --- 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 index 2a95214225e4..7fc66f89646a 100644 --- a/packages/docregistry/src/mimedocument.ts +++ b/packages/docregistry/src/mimedocument.ts @@ -287,7 +287,7 @@ export class MimeDocumentFactory extends ABCWidgetFactory { 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 index e3401cebabbc..c595bb75ac30 100644 --- 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,7 +1346,7 @@ 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' }, { @@ -1351,7 +1354,7 @@ export namespace DocumentRegistry { displayName: 'Image', mimeTypes: ['image/jpeg'], extensions: ['.jpg', '.jpeg'], - iconRender: imageIcon.phosphor({ kind: 'mainAreaTab', center: true }), + iconRenderer: imageIcon.bindStyle({ kind: 'mainAreaTab', center: true }), fileFormat: 'base64' }, { @@ -1359,7 +1362,7 @@ export namespace DocumentRegistry { displayName: 'Image', mimeTypes: ['image/gif'], extensions: ['.gif'], - iconRender: imageIcon.phosphor({ kind: 'mainAreaTab', center: true }), + iconRenderer: imageIcon.bindStyle({ kind: 'mainAreaTab', center: true }), fileFormat: 'base64' }, { @@ -1367,7 +1370,7 @@ export namespace DocumentRegistry { displayName: 'Image', mimeTypes: ['image/png'], extensions: ['.png'], - iconRender: imageIcon.phosphor({ kind: 'mainAreaTab', center: true }), + iconRenderer: imageIcon.bindStyle({ kind: 'mainAreaTab', center: true }), fileFormat: 'base64' }, { @@ -1375,7 +1378,7 @@ export namespace DocumentRegistry { 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 index aae63da62bcb..1154b6ed6955 100644 --- 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 index 78661f33d2aa..4570dcc42876 100644 --- 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 index 38c198d3eac6..66ccd0771e99 100644 --- 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 index 22ac3db899a3..3beb35fe7887 100644 --- a/packages/markdownviewer/src/widget.ts +++ b/packages/markdownviewer/src/widget.ts @@ -309,7 +309,7 @@ export class MarkdownViewerFactory extends ABCWidgetFactory { 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 index 6b6fdf573f50..d99c99d78fb2 100644 --- 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 index cd5b20f668bf..d37d5dc574eb 100644 --- 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(, host); - }; + render(host: HTMLElement, props: JLIcon.IProps = {}): void { + return ReactDOM.render(, 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 ) => { 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 {