From d1c3cf8bb0964f7244f7c5dcf3247f5584827e0f Mon Sep 17 00:00:00 2001
From: Christopher Willis-Ford <cwillisf@media.mit.edu>
Date: Mon, 10 Dec 2018 14:49:05 -0800
Subject: [PATCH] Move image-from-storage code into new ScratchImage component

---
 src/components/library-item/library-item.jsx  | 11 +--
 src/components/library/library.jsx            | 39 ++++++++--
 .../scratch-image/scratch-image.jsx           | 71 ++++++++++++++++++
 src/containers/library-item.jsx               | 74 +------------------
 4 files changed, 111 insertions(+), 84 deletions(-)
 create mode 100644 src/components/scratch-image/scratch-image.jsx

diff --git a/src/components/library-item/library-item.jsx b/src/components/library-item/library-item.jsx
index eb418d8d9..4289a5269 100644
--- a/src/components/library-item/library-item.jsx
+++ b/src/components/library-item/library-item.jsx
@@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
 import React from 'react';
 
 import Box from '../box/box.jsx';
+import ScratchImage from '../scratch-image/scratch-image.jsx';
 import styles from './library-item.css';
 import classNames from 'classnames';
 
@@ -34,9 +35,9 @@ class LibraryItemComponent extends React.PureComponent {
                             />
                         </div>
                     ) : null}
-                    <img
+                    <ScratchImage
                         className={styles.featuredImage}
-                        src={this.props.iconURI}
+                        imageSource={this.props.iconSource}
                     />
                 </div>
                 {this.props.insetIconURL ? (
@@ -120,9 +121,9 @@ class LibraryItemComponent extends React.PureComponent {
                 {/* Layers of wrapping is to prevent layout thrashing on animation */}
                 <Box className={styles.libraryItemImageContainerWrapper}>
                     <Box className={styles.libraryItemImageContainer}>
-                        <img
+                        <ScratchImage
                             className={styles.libraryItemImage}
-                            src={this.props.iconURI}
+                            imageSource={this.props.iconSource}
                         />
                     </Box>
                 </Box>
@@ -143,7 +144,7 @@ LibraryItemComponent.propTypes = {
     extensionId: PropTypes.string,
     featured: PropTypes.bool,
     hidden: PropTypes.bool,
-    iconURI: PropTypes.string,
+    iconSource: ScratchImage.propTypes.imageSource,
     insetIconURL: PropTypes.string,
     internetConnectionRequired: PropTypes.bool,
     name: PropTypes.oneOfType([
diff --git a/src/components/library/library.jsx b/src/components/library/library.jsx
index b0a7a481c..42b342481 100644
--- a/src/components/library/library.jsx
+++ b/src/components/library/library.jsx
@@ -10,6 +10,7 @@ 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';
 
@@ -29,6 +30,24 @@ const messages = defineMessages({
 const ALL_TAG = {tag: 'all', intlLabel: messages.allTag};
 const tagListPrefix = [ALL_TAG];
 
+/**
+ * Find the AssetType which corresponds to a particular file extension. For example, 'png' => AssetType.ImageBitmap.
+ * @param {string} fileExtension - the file extension to look up.
+ * @returns {AssetType} - the AssetType corresponding to the extension, if any.
+ */
+const getAssetTypeForFileExtension = function (fileExtension) {
+    const compareOptions = {
+        sensitivity: 'accent',
+        usage: 'search'
+    };
+    for (const assetTypeId of Object.keys(storage.AssetType)) {
+        const assetType = storage.AssetType[assetTypeId];
+        if (fileExtension.localeCompare(assetType.runtimeFormat, compareOptions) === 0) {
+            return assetType;
+        }
+    }
+};
+
 class LibraryComponent extends React.Component {
     constructor (props) {
         super(props);
@@ -172,8 +191,17 @@ class LibraryComponent extends React.Component {
                     })}
                     ref={this.setFilteredDataRef}
                 >
-                    {this.getFilteredData().map((dataItem, index) => (
-                        <LibraryItem
+                    {this.getFilteredData().map((dataItem, index) => {
+                        const iconSource = {};
+                        if (dataItem.rawURL) {
+                            iconSource.uri = dataItem.rawURL;
+                        } else {
+                            // TODO: adjust libraries to be more storage-friendly; don't use split() here.
+                            const [assetId, fileExtension] = dataItem.md5.split('.');
+                            iconSource.assetId = assetId;
+                            iconSource.assetType = getAssetTypeForFileExtension(fileExtension);
+                        }
+                        return (<LibraryItem
                             bluetoothRequired={dataItem.bluetoothRequired}
                             collaborator={dataItem.collaborator}
                             description={dataItem.description}
@@ -181,8 +209,7 @@ class LibraryComponent extends React.Component {
                             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
+                            iconSource={iconSource}
                             id={index}
                             insetIconURL={dataItem.insetIconURL}
                             internetConnectionRequired={dataItem.internetConnectionRequired}
@@ -193,8 +220,8 @@ class LibraryComponent extends React.Component {
                             onMouseEnter={this.handleMouseEnter}
                             onMouseLeave={this.handleMouseLeave}
                             onSelect={this.handleSelect}
-                        />
-                    ))}
+                        />);
+                    })}
                 </div>
             </Modal>
         );
diff --git a/src/components/scratch-image/scratch-image.jsx b/src/components/scratch-image/scratch-image.jsx
new file mode 100644
index 000000000..4cf0c5d95
--- /dev/null
+++ b/src/components/scratch-image/scratch-image.jsx
@@ -0,0 +1,71 @@
+import PropTypes from 'prop-types';
+import React from 'react';
+
+import storage from '../../lib/storage';
+
+class ScratchImage extends React.PureComponent {
+    constructor (props) {
+        super(props);
+
+        this.state = {};
+        Object.assign(this.state, this._loadImageSource(props.imageSource));
+    }
+    componentWillReceiveProps (nextProps) {
+        const newState = this._loadImageSource(nextProps.imageSource);
+        this.setState(newState);
+    }
+    /**
+     * Calculate the state changes necessary to load the image specified in the provided source info. If the component
+     * is mounted, call setState() with the return value of this function. If the component has not yet mounted, use
+     * the return value of this function as initial state for the component.
+     *
+     * @param {object} imageSource - the new source for the image, including either assetId or URI
+     * @returns {object} - the new state values, if any.
+     */
+    _loadImageSource (imageSource) {
+        if (imageSource.uri) {
+            return {
+                imageURI: imageSource.uri,
+                lastRequestedAsset: null
+            };
+        }
+        if (this.state.lastRequestedAsset !== imageSource.assetId) {
+            storage.load(imageSource.assetType, imageSource.assetId)
+                .then(asset => {
+                    this.setState({
+                        imageURI: asset.encodeDataURI()
+                    });
+                });
+            return {
+                lastRequestedAsset: imageSource.assetId
+            };
+        }
+        // Nothing to do - don't change any state.
+        return {};
+    }
+    render () {
+        const {
+            src: _src,
+            imageSource: _imageSource,
+            ...imgProps
+        } = this.props;
+        return (<img
+            src={this.state.imageURI}
+            {...imgProps}
+        />);
+    }
+}
+
+ScratchImage.propTypes = {
+    imageSource: PropTypes.oneOfType([
+        PropTypes.shape({
+            assetId: PropTypes.string.isRequired,
+            assetType: PropTypes.oneOf(Object.values(storage.AssetType)).isRequired
+        }),
+        PropTypes.shape({
+            uri: PropTypes.string.isRequired
+        })
+    ]).isRequired
+};
+
+export default ScratchImage;
diff --git a/src/containers/library-item.jsx b/src/containers/library-item.jsx
index 372c21574..9837d5b2a 100644
--- a/src/containers/library-item.jsx
+++ b/src/containers/library-item.jsx
@@ -3,25 +3,6 @@ import PropTypes from 'prop-types';
 import React from 'react';
 
 import LibraryItemComponent from '../components/library-item/library-item.jsx';
-import storage from '../lib/storage';
-
-/**
- * Find the AssetType which corresponds to a particular file extension. For example, 'png' => AssetType.ImageBitmap.
- * @param {string} fileExtension - the file extension to look up.
- * @returns {AssetType} - the AssetType corresponding to the extension, if any.
- */
-const getAssetTypeForFileExtension = function (fileExtension) {
-    const compareOptions = {
-        sensitivity: 'accent',
-        usage: 'search'
-    };
-    for (const assetTypeId of Object.keys(storage.AssetType)) {
-        const assetType = storage.AssetType[assetTypeId];
-        if (fileExtension.localeCompare(assetType.runtimeFormat, compareOptions) === 0) {
-            return assetType;
-        }
-    }
-};
 
 class LibraryItem extends React.PureComponent {
     constructor (props) {
@@ -34,44 +15,6 @@ class LibraryItem extends React.PureComponent {
             'handleMouseEnter',
             'handleMouseLeave'
         ]);
-        this.state = {};
-        Object.assign(this.state, this._loadIconFromProps(props));
-    }
-    componentWillReceiveProps (nextProps) {
-        const newState = this._loadIconFromProps(nextProps);
-        this.setState(newState);
-    }
-    /**
-     * Calculate the state changes necessary to load the icon specified in the provided properties. If the component is
-     * mounted, call setState() with the return value of this function. If the component has not yet mounted, use the
-     * return value of this function as initial state for the component.
-     *
-     * @param {object} props - the new properties for this component, including either iconMD5 or iconURL
-     * @returns {object} - the new state values, if any.
-     */
-    _loadIconFromProps (props) {
-        if (props.iconURL) {
-            return {
-                iconURI: props.iconURL,
-                lastRequestedMD5: null
-            };
-        }
-        if (this.state.lastRequestedMD5 !== props.iconMD5) {
-            // TODO: adjust libraries to be more storage-friendly; don't use split() here.
-            const [md5, ext] = props.iconMD5.split('.');
-            const assetType = getAssetTypeForFileExtension(ext);
-            storage.load(assetType, md5)
-                .then(asset => {
-                    this.setState({
-                        iconURI: asset.encodeDataURI()
-                    });
-                });
-            return {
-                lastRequestedMD5: props.iconMD5
-            };
-        }
-        // Nothing to do - don't change any state.
-        return {};
     }
     handleBlur () {
         this.props.onBlur(this.props.id);
@@ -99,8 +42,6 @@ class LibraryItem extends React.PureComponent {
     }
     render () {
         const {
-            iconMD5: _iconMD5,
-            iconURL: _iconURL,
             id: _id,
             onBlur: _onBlur,
             onFocus: _onFocus,
@@ -110,7 +51,7 @@ class LibraryItem extends React.PureComponent {
             ...childProps
         } = this.props;
         return (<LibraryItemComponent
-            iconURI={this.state.iconURI}
+            imageSource={this.props.imageSource}
             onBlur={this.props.onBlur && this.handleBlur}
             onClick={this.handleClick}
             onFocus={this.props.onFocus && this.handleFocus}
@@ -122,21 +63,10 @@ class LibraryItem extends React.PureComponent {
     }
 }
 
-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 prop types EXCEPT these from LibraryItemComponent
         const {
-            iconURI: _iconURI,
             onClick: _onClick,
             onKeyPress: _onKeyPress,
             ...otherPropTypes
@@ -145,8 +75,6 @@ LibraryItem.propTypes = Object.assign({},
     })(),
     {
         id: PropTypes.number.isRequired,
-        iconMD5: LibraryItem.requireUrlOrMD5,
-        iconURL: LibraryItem.requireUrlOrMD5,
         onSelect: PropTypes.func.isRequired
     }
 );