From db9236bef2d6082b7c743c61ad68e6af7fb4b824 Mon Sep 17 00:00:00 2001
From: Christopher Willis-Ford <cwillisf@media.mit.edu>
Date: Thu, 8 Nov 2018 12:56:54 -0800
Subject: [PATCH] Fix sprite lib animation: move load into LibraryItem

The `LibraryItem` component has been split into a container, responsible
for handling interaction with the storage module, and a component,
responsible for presentation. The component now always takes an
`iconURI` and the logic to load an icon from storage has been moved from
`Library` into `LibraryItem`.

TODO: move animation logic from `SpriteLibrary` into `LibraryItem`. This
should simplify the code quite a bit and might also improve performance.
---
 src/components/library-item/library-item.jsx | 24 ++----
 src/components/library/library.jsx           | 78 ++++++--------------
 src/containers/library-item.jsx              | 74 +++++++++++++++++++
 3 files changed, 105 insertions(+), 71 deletions(-)
 create mode 100644 src/containers/library-item.jsx

diff --git a/src/components/library-item/library-item.jsx b/src/components/library-item/library-item.jsx
index 48d117d96..8cd8a830d 100644
--- a/src/components/library-item/library-item.jsx
+++ b/src/components/library-item/library-item.jsx
@@ -10,16 +10,9 @@ import classNames from 'classnames';
 import bluetoothIconURL from './bluetooth.svg';
 import internetConnectionIconURL from './internet-connection.svg';
 
-class LibraryItem extends React.PureComponent {
+class LibraryItemComponent extends React.PureComponent {
     constructor (props) {
         super(props);
-        this.state = {
-            iconURL: null
-        };
-        Promise.resolve(props.iconURL)
-            .then(url => this.setState({
-                iconURL: url
-            }));
         bindAll(this, [
             'handleBlur',
             'handleClick',
@@ -79,7 +72,7 @@ class LibraryItem extends React.PureComponent {
                     ) : null}
                     <img
                         className={styles.featuredImage}
-                        src={this.state.iconURL}
+                        src={this.props.iconURI}
                     />
                 </div>
                 {this.props.insetIconURL ? (
@@ -165,7 +158,7 @@ class LibraryItem extends React.PureComponent {
                     <Box className={styles.libraryItemImageContainer}>
                         <img
                             className={styles.libraryItemImage}
-                            src={this.state.iconURL}
+                            src={this.props.iconURI}
                         />
                     </Box>
                 </Box>
@@ -175,7 +168,7 @@ class LibraryItem extends React.PureComponent {
     }
 }
 
-LibraryItem.propTypes = {
+LibraryItemComponent.propTypes = {
     bluetoothRequired: PropTypes.bool,
     collaborator: PropTypes.string,
     description: PropTypes.oneOfType([
@@ -186,10 +179,7 @@ LibraryItem.propTypes = {
     extensionId: PropTypes.string,
     featured: PropTypes.bool,
     hidden: PropTypes.bool,
-    iconURL: PropTypes.oneOfType([
-        PropTypes.string, // URL
-        PropTypes.instanceOf(Promise) // Promise for URL
-    ]),
+    iconURI: PropTypes.string,
     id: PropTypes.number.isRequired,
     insetIconURL: PropTypes.string,
     internetConnectionRequired: PropTypes.bool,
@@ -204,8 +194,8 @@ LibraryItem.propTypes = {
     onSelect: PropTypes.func.isRequired
 };
 
-LibraryItem.defaultProps = {
+LibraryItemComponent.defaultProps = {
     disabled: false
 };
 
-export default LibraryItem;
+export default LibraryItemComponent;
diff --git a/src/components/library/library.jsx b/src/components/library/library.jsx
index f1eceb806..b0a7a481c 100644
--- a/src/components/library/library.jsx
+++ b/src/components/library/library.jsx
@@ -4,13 +4,12 @@ import PropTypes from 'prop-types';
 import React from 'react';
 import {defineMessages, injectIntl, intlShape} from 'react-intl';
 
-import LibraryItem from '../library-item/library-item.jsx';
+import LibraryItem from '../../containers/library-item.jsx';
 import Modal from '../../containers/modal.jsx';
 import Divider from '../divider/divider.jsx';
 import Filter from '../filter/filter.jsx';
 import TagButton from '../../containers/tag-button.jsx';
 import analytics from '../../lib/analytics';
-import storage from '../../lib/storage';
 
 import styles from './library.css';
 
@@ -30,23 +29,6 @@ const messages = defineMessages({
 const ALL_TAG = {tag: 'all', intlLabel: messages.allTag};
 const tagListPrefix = [ALL_TAG];
 
-/**
- * @param {string} extension - the extension to look up.
- * @returns {AssetType} - the AssetType corresponding to the extension, if any.
- */
-const getAssetTypeForExtension = function (extension) {
-    const compareOptions = {
-        sensitivity: 'accent',
-        usage: 'search'
-    };
-    for (const assetTypeId of Object.keys(storage.AssetType)) {
-        const assetType = storage.AssetType[assetTypeId];
-        if (extension.localeCompare(assetType.runtimeFormat, compareOptions) === 0) {
-            return assetType;
-        }
-    }
-};
-
 class LibraryComponent extends React.Component {
     constructor (props) {
         super(props);
@@ -190,41 +172,29 @@ class LibraryComponent extends React.Component {
                     })}
                     ref={this.setFilteredDataRef}
                 >
-                    {this.getFilteredData().map((dataItem, index) => {
-                        let iconURL;
-                        if (dataItem.md5) {
-                            // TODO: adjust libraries to be more storage-friendly; don't use split() here.
-                            const [md5, ext] = dataItem.md5.split('.');
-                            const assetType = getAssetTypeForExtension(ext);
-                            iconURL = storage.load(assetType, md5)
-                                .then(asset => asset.encodeDataURI());
-                        } else {
-                            iconURL = dataItem.rawURL;
-                        }
-
-                        return (
-                            <LibraryItem
-                                bluetoothRequired={dataItem.bluetoothRequired}
-                                collaborator={dataItem.collaborator}
-                                description={dataItem.description}
-                                disabled={dataItem.disabled}
-                                extensionId={dataItem.extensionId}
-                                featured={dataItem.featured}
-                                hidden={dataItem.hidden}
-                                iconURL={iconURL}
-                                id={index}
-                                insetIconURL={dataItem.insetIconURL}
-                                internetConnectionRequired={dataItem.internetConnectionRequired}
-                                key={`item_${index}`}
-                                name={dataItem.name}
-                                onBlur={this.handleBlur}
-                                onFocus={this.handleFocus}
-                                onMouseEnter={this.handleMouseEnter}
-                                onMouseLeave={this.handleMouseLeave}
-                                onSelect={this.handleSelect}
-                            />
-                        );
-                    })}
+                    {this.getFilteredData().map((dataItem, index) => (
+                        <LibraryItem
+                            bluetoothRequired={dataItem.bluetoothRequired}
+                            collaborator={dataItem.collaborator}
+                            description={dataItem.description}
+                            disabled={dataItem.disabled}
+                            extensionId={dataItem.extensionId}
+                            featured={dataItem.featured}
+                            hidden={dataItem.hidden}
+                            iconMD5={dataItem.md5} // either this or iconURL must be defined
+                            iconURL={dataItem.rawURL} // either this or iconMD5 must be defined
+                            id={index}
+                            insetIconURL={dataItem.insetIconURL}
+                            internetConnectionRequired={dataItem.internetConnectionRequired}
+                            key={`item_${index}`}
+                            name={dataItem.name}
+                            onBlur={this.handleBlur}
+                            onFocus={this.handleFocus}
+                            onMouseEnter={this.handleMouseEnter}
+                            onMouseLeave={this.handleMouseLeave}
+                            onSelect={this.handleSelect}
+                        />
+                    ))}
                 </div>
             </Modal>
         );
diff --git a/src/containers/library-item.jsx b/src/containers/library-item.jsx
new file mode 100644
index 000000000..f3cd9dea1
--- /dev/null
+++ b/src/containers/library-item.jsx
@@ -0,0 +1,74 @@
+import React from 'react';
+
+import LibraryItemComponent from '../components/library-item/library-item.jsx';
+import storage from '../lib/storage';
+
+/**
+ * @param {string} extension - the extension to look up.
+ * @returns {AssetType} - the AssetType corresponding to the extension, if any.
+ */
+const getAssetTypeForExtension = function (extension) {
+    const compareOptions = {
+        sensitivity: 'accent',
+        usage: 'search'
+    };
+    for (const assetTypeId of Object.keys(storage.AssetType)) {
+        const assetType = storage.AssetType[assetTypeId];
+        if (extension.localeCompare(assetType.runtimeFormat, compareOptions) === 0) {
+            return assetType;
+        }
+    }
+};
+
+class LibraryItem extends React.PureComponent {
+    constructor (props) {
+        super(props);
+        this.state = Object.assign(this.state || {}, {
+            iconURI: props.iconURL // may be undefined if we're using iconMD5 instead
+        });
+    }
+    componentWillReceiveProps (nextProps) {
+        if (nextProps.iconURL) {
+            this.setState({iconURI: nextProps.iconURL});
+        } else if ((!this.state.iconURI) || nextProps.iconMD5 !== this.props.iconMD5) {
+            // TODO: adjust libraries to be more storage-friendly; don't use split() here.
+            const [md5, ext] = nextProps.iconMD5.split('.');
+            const assetType = getAssetTypeForExtension(ext);
+            storage.load(assetType, md5)
+                .then(asset => {
+                    this.setState({iconURI: asset.encodeDataURI()});
+                });
+        }
+    }
+    render () {
+        const {iconMD5: _iconMD5, iconURL: _iconURL, ...childProps} = this.props;
+        return (<LibraryItemComponent
+            iconURI={this.state.iconURI}
+            {...childProps}
+        />);
+    }
+}
+
+LibraryItem.requireUrlOrMD5 = function (props) {
+    if (typeof props.iconMD5 === 'string' && typeof props.iconURL === 'undefined') {
+        return;
+    }
+    if (typeof props.iconMD5 === 'undefined' && typeof props.iconURL === 'string') {
+        return;
+    }
+    return new Error('Exactly one of iconURL or iconMD5 must be set and the value must be a string.');
+};
+
+LibraryItem.propTypes = Object.assign({},
+    (() => {
+        // copy all props EXCEPT iconURI from LibraryItemComponent
+        const {iconURI: _iconURI, ...otherProps} = LibraryItemComponent.propTypes;
+        return otherProps;
+    })(),
+    {
+        iconMD5: LibraryItem.requireUrlOrMD5,
+        iconURL: LibraryItem.requireUrlOrMD5
+    }
+);
+
+export default LibraryItem;