Skip to content

Commit

Permalink
prevent redundant rerenders of icons created by JLIcon.element()
Browse files Browse the repository at this point in the history
  • Loading branch information
telamonian committed Dec 26, 2019
1 parent 6dcb684 commit 636ddf4
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 17 deletions.
16 changes: 2 additions & 14 deletions packages/filebrowser/src/listing.ts
Expand Up @@ -1812,20 +1812,8 @@ export namespace DirListing {
const text = DOMUtils.findElement(node, ITEM_TEXT_CLASS);
const modified = DOMUtils.findElement(node, ITEM_MODIFIED_CLASS);

let icon: JLIcon;
let iconClass: string | undefined;

if (fileType?.iconRenderer) {
// use the icon and optional iconClass supplied by the ft
icon = fileType.iconRenderer;
iconClass = classes(ITEM_ICON_CLASS, fileType.iconClass);
} else if (fileType?.iconClass) {
// try to look up the icon based on the ft iconClass
icon = JLIcon.get(fileType.iconClass, fileIcon);
} else {
// fallback to fileIcon
icon = fileIcon;
}
let icon = fileType?.iconRenderer ? fileType.iconRenderer : (fileType?.iconClass ? JLIcon.get(fileType.iconClass) : fileIcon);
let iconClass = classes(ITEM_ICON_CLASS, fileType?.iconClass);

// render the icon svg node
icon.element({className: iconClass, container: iconContainer, center: true, kind: 'listing'});
Expand Down
19 changes: 17 additions & 2 deletions packages/ui-components/src/icon/jlicon.tsx
@@ -1,6 +1,8 @@
// Copyright (c) Jupyter Development Team.
// Distributed under the terms of the Modified BSD License.

import { UUID } from '@lumino/coreutils';

import React from 'react';
import ReactDOM from 'react-dom';

Expand Down Expand Up @@ -55,7 +57,7 @@ export class JLIcon {
constructor({ name, svgstr }: JLIcon.IOptions) {
this.name = name;
this._className = JLIcon.nameToClassName(name);
this._svgstr = svgstr;
this.svgstr = svgstr;

this.react = this._initReact();

Expand All @@ -74,6 +76,13 @@ export class JLIcon {
tag = 'div',
...propsStyle
}: JLIcon.IProps = {}): HTMLElement | null {
// check if icon element is already set
const maybeSvgElement = container?.firstChild as HTMLElement;
if (maybeSvgElement?.dataset?.iconid === this._uuid) {
// return the existing icon element
return maybeSvgElement;
}

// ensure that svg html is valid
const svgElement = this.resolveSvg();
if (!svgElement) {
Expand All @@ -99,8 +108,11 @@ export class JLIcon {
return svgElement;
}

replaceSvg(svgstr: string) {
set svgstr(svgstr: string) {
this._svgstr = svgstr;

// associate a unique id with this particular svgstr
this._uuid = UUID.uuid4();
}

render(host: HTMLElement, props: JLIcon.IProps = {}): void {
Expand Down Expand Up @@ -128,6 +140,8 @@ export class JLIcon {
}
} else {
// parse succeeded
svgElement.dataset.iconid = this._uuid;

if (title) {
Private.setTitleSvg(svgElement, title);
}
Expand Down Expand Up @@ -224,6 +238,7 @@ export class JLIcon {

protected _className: string;
protected _svgstr: string;
protected _uuid: string;
}

/**
Expand Down
27 changes: 26 additions & 1 deletion packages/ui-components/src/utils.ts
Expand Up @@ -3,6 +3,9 @@

import { Text } from '@jupyterlab/coreutils';

/**
* Inner works of class combining functions
*/
function _classes(
classes: (string | false | undefined | null | { [className: string]: any })[]
): string[] {
Expand All @@ -20,6 +23,10 @@ function _classes(

/**
* Combines classNames.
*
* @param classes - A list of classNames
*
* @returns A single string with the combined className
*/
export function classes(
...classes: (
Expand All @@ -35,6 +42,10 @@ export function classes(

/**
* Combines classNames. Removes all duplicates
*
* @param classes - A list of classNames
*
* @returns A single string with the combined className
*/
export function classesDedupe(
...classes: (
Expand All @@ -48,9 +59,23 @@ export function classesDedupe(
return [...new Set(_classes(classes))].join(' ');
}

/**
* Translates the attributes of a DOM element into attributes that can
* be understood by React. Currently not comprehensive, we will add special
* cases as they become relevant.
*
* @param elem - A DOM element
*
* @returns An object with key:value pairs that are the React-friendly
* translation of elem's attributes
*/
export function getReactAttrs(elem: Element) {
return elem.getAttributeNames().reduce((d, name) => {
if (name !== 'style') {
if (name === 'style') {
void 0;
} else if (name.startsWith('data')) {
d[name] = elem.getAttribute(name);
} else {
d[Text.camelCase(name)] = elem.getAttribute(name);
}
return d;
Expand Down

0 comments on commit 636ddf4

Please sign in to comment.