Merge pull request #4099 from benjiwheeler/sprite-animate-render

Sprite library animations no longer update the entire library state on every costume change

src/components/library-item/library-item.css

@@ -8,6 +8,7 @@
     justify-content: flex-start;
     flex-basis: 160px;
     height: 160px;
+    max-width: 160px;
     margin: $space;
     padding: 1rem 1rem 0 1rem;
     font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;

src/components/library-item/library-item.jsx

-import bindAll from 'lodash.bindall';
 import {FormattedMessage} from 'react-intl';
 import PropTypes from 'prop-types';
 import React from 'react';
@@ -10,42 +9,8 @@ import classNames from 'classnames';
 import bluetoothIconURL from './bluetooth.svg';
 import internetConnectionIconURL from './internet-connection.svg';
 
-class LibraryItem extends React.PureComponent {
-    constructor (props) {
-        super(props);
-        bindAll(this, [
-            'handleBlur',
-            'handleClick',
-            'handleFocus',
-            'handleKeyPress',
-            'handleMouseEnter',
-            'handleMouseLeave'
-        ]);
-    }
-    handleBlur () {
-        this.props.onBlur(this.props.id);
-    }
-    handleFocus () {
-        this.props.onFocus(this.props.id);
-    }
-    handleClick (e) {
-        if (!this.props.disabled) {
-            this.props.onSelect(this.props.id);
-        }
-        e.preventDefault();
-    }
-    handleKeyPress (e) {
-        if (e.key === ' ' || e.key === 'Enter') {
-            e.preventDefault();
-            this.props.onSelect(this.props.id);
-        }
-    }
-    handleMouseEnter () {
-        this.props.onMouseEnter(this.props.id);
-    }
-    handleMouseLeave () {
-        this.props.onMouseLeave(this.props.id);
-    }
+/* eslint-disable react/prefer-stateless-function */
+class LibraryItemComponent extends React.PureComponent {
     render () {
         return this.props.featured ? (
             <div
@@ -58,7 +23,7 @@ class LibraryItem extends React.PureComponent {
                     this.props.extensionId ? styles.libraryItemExtension : null,
                     this.props.hidden ? styles.hidden : null
                 )}
-                onClick={this.handleClick}
+                onClick={this.props.onClick}
             >
                 <div className={styles.featuredImageContainer}>
                     {this.props.disabled ? (
@@ -146,12 +111,12 @@ class LibraryItem extends React.PureComponent {
                 )}
                 role="button"
                 tabIndex="0"
-                onBlur={this.handleBlur}
-                onClick={this.handleClick}
-                onFocus={this.handleFocus}
-                onKeyPress={this.handleKeyPress}
-                onMouseEnter={this.handleMouseEnter}
-                onMouseLeave={this.handleMouseLeave}
+                onBlur={this.props.onBlur}
+                onClick={this.props.onClick}
+                onFocus={this.props.onFocus}
+                onKeyPress={this.props.onKeyPress}
+                onMouseEnter={this.props.onMouseEnter}
+                onMouseLeave={this.props.onMouseLeave}
             >
                 {/* Layers of wrapping is to prevent layout thrashing on animation */}
                 <Box className={styles.libraryItemImageContainerWrapper}>
@@ -167,8 +132,10 @@ class LibraryItem extends React.PureComponent {
         );
     }
 }
+/* eslint-enable react/prefer-stateless-function */
+
 
-LibraryItem.propTypes = {
+LibraryItemComponent.propTypes = {
     bluetoothRequired: PropTypes.bool,
     collaborator: PropTypes.string,
     description: PropTypes.oneOfType([
@@ -180,22 +147,22 @@ LibraryItem.propTypes = {
     featured: PropTypes.bool,
     hidden: PropTypes.bool,
     iconURL: PropTypes.string,
-    id: PropTypes.number.isRequired,
     insetIconURL: PropTypes.string,
     internetConnectionRequired: PropTypes.bool,
     name: PropTypes.oneOfType([
         PropTypes.string,
         PropTypes.node
     ]),
-    onBlur: PropTypes.func,
-    onFocus: PropTypes.func,
+    onBlur: PropTypes.func.isRequired,
+    onClick: PropTypes.func.isRequired,
+    onFocus: PropTypes.func.isRequired,
+    onKeyPress: PropTypes.func.isRequired,
     onMouseEnter: PropTypes.func.isRequired,
-    onMouseLeave: PropTypes.func.isRequired,
-    onSelect: PropTypes.func.isRequired
+    onMouseLeave: PropTypes.func.isRequired
 };
 
-LibraryItem.defaultProps = {
+LibraryItemComponent.defaultProps = {
     disabled: false
 };
 
-export default LibraryItem;
+export default LibraryItemComponent;

src/components/library/library.jsx

@@ -4,7 +4,7 @@ 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';
@@ -33,11 +33,9 @@ class LibraryComponent extends React.Component {
     constructor (props) {
         super(props);
         bindAll(this, [
-            'handleBlur',
             'handleClose',
             'handleFilterChange',
             'handleFilterClear',
-            'handleFocus',
             'handleMouseEnter',
             'handleMouseLeave',
             'handleSelect',
@@ -56,12 +54,6 @@ class LibraryComponent extends React.Component {
             this.scrollToTop();
         }
     }
-    handleBlur (id) {
-        this.handleMouseLeave(id);
-    }
-    handleFocus (id) {
-        this.handleMouseEnter(id);
-    }
     handleSelect (id) {
         this.handleClose();
         this.props.onItemSelected(this.getFilteredData()[id]);
@@ -172,33 +164,28 @@ class LibraryComponent extends React.Component {
                     })}
                     ref={this.setFilteredDataRef}
                 >
-                    {this.getFilteredData().map((dataItem, index) => {
-                        const scratchURL = dataItem.md5 ?
-                            `https://cdn.assets.scratch.mit.edu/internalapi/asset/${dataItem.md5}/get/` :
-                            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={scratchURL}
-                                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}
+                            iconRawURL={dataItem.rawURL}
+                            icons={dataItem.json && dataItem.json.costumes}
+                            id={index}
+                            insetIconURL={dataItem.insetIconURL}
+                            internetConnectionRequired={dataItem.internetConnectionRequired}
+                            key={`item_${index}`}
+                            name={dataItem.name}
+                            onMouseEnter={this.handleMouseEnter}
+                            onMouseLeave={this.handleMouseLeave}
+                            onSelect={this.handleSelect}
+                        />
+                    ))}
                 </div>
             </Modal>
         );

src/containers/library-item.jsx

+import bindAll from 'lodash.bindall';
+import PropTypes from 'prop-types';
+import React from 'react';
+import {injectIntl} from 'react-intl';
+
+import LibraryItemComponent from '../components/library-item/library-item.jsx';
+
+class LibraryItem extends React.PureComponent {
+    constructor (props) {
+        super(props);
+        bindAll(this, [
+            'handleBlur',
+            'handleClick',
+            'handleFocus',
+            'handleKeyPress',
+            'handleMouseEnter',
+            'handleMouseLeave',
+            'rotateIcon',
+            'startRotatingIcons',
+            'stopRotatingIcons'
+        ]);
+        this.state = {
+            iconIndex: 0,
+            isRotatingIcon: false
+        };
+    }
+    componentWillUnmount () {
+        clearInterval(this.intervalId);
+    }
+    handleBlur (id) {
+        this.handleMouseLeave(id);
+    }
+    handleClick (e) {
+        if (!this.props.disabled) {
+            this.props.onSelect(this.props.id);
+        }
+        e.preventDefault();
+    }
+    handleFocus (id) {
+        this.handleMouseEnter(id);
+    }
+    handleKeyPress (e) {
+        if (e.key === ' ' || e.key === 'Enter') {
+            e.preventDefault();
+            this.props.onSelect(this.props.id);
+        }
+    }
+    handleMouseEnter () {
+        this.props.onMouseEnter(this.props.id);
+        if (this.props.icons && this.props.icons.length) {
+            this.stopRotatingIcons();
+            this.setState({
+                isRotatingIcon: true
+            }, this.startRotatingIcons);
+        }
+    }
+    handleMouseLeave () {
+        this.props.onMouseLeave(this.props.id);
+        if (this.props.icons && this.props.icons.length) {
+            this.setState({
+                isRotatingIcon: false
+            }, this.stopRotatingIcons);
+        }
+    }
+    startRotatingIcons () {
+        this.rotateIcon();
+        this.intervalId = setInterval(this.rotateIcon, 300);
+    }
+    stopRotatingIcons () {
+        if (this.intervalId) {
+            this.intervalId = clearInterval(this.intervalId);
+        }
+    }
+    rotateIcon () {
+        const nextIconIndex = (this.state.iconIndex + 1) % this.props.icons.length;
+        this.setState({iconIndex: nextIconIndex});
+    }
+    curIconMd5 () {
+        if (this.props.icons &&
+            this.state.isRotatingIcon &&
+            this.state.iconIndex < this.props.icons.length &&
+            this.props.icons[this.state.iconIndex] &&
+            this.props.icons[this.state.iconIndex].baseLayerMD5) {
+            return this.props.icons[this.state.iconIndex].baseLayerMD5;
+        }
+        return this.props.iconMd5;
+    }
+    render () {
+        const iconMd5 = this.curIconMd5();
+        const iconURL = iconMd5 ?
+            `https://cdn.assets.scratch.mit.edu/internalapi/asset/${iconMd5}/get/` :
+            this.props.iconRawURL;
+        return (
+            <LibraryItemComponent
+                bluetoothRequired={this.props.bluetoothRequired}
+                collaborator={this.props.collaborator}
+                description={this.props.description}
+                disabled={this.props.disabled}
+                extensionId={this.props.extensionId}
+                featured={this.props.featured}
+                hidden={this.props.hidden}
+                iconURL={iconURL}
+                icons={this.props.icons}
+                id={this.props.id}
+                insetIconURL={this.props.insetIconURL}
+                internetConnectionRequired={this.props.internetConnectionRequired}
+                name={this.props.name}
+                onBlur={this.handleBlur}
+                onClick={this.handleClick}
+                onFocus={this.handleFocus}
+                onKeyPress={this.handleKeyPress}
+                onMouseEnter={this.handleMouseEnter}
+                onMouseLeave={this.handleMouseLeave}
+            />
+        );
+    }
+}
+
+LibraryItem.propTypes = {
+    bluetoothRequired: PropTypes.bool,
+    collaborator: PropTypes.string,
+    description: PropTypes.oneOfType([
+        PropTypes.string,
+        PropTypes.node
+    ]),
+    disabled: PropTypes.bool,
+    extensionId: PropTypes.string,
+    featured: PropTypes.bool,
+    hidden: PropTypes.bool,
+    iconMd5: PropTypes.string,
+    iconRawURL: PropTypes.string,
+    icons: PropTypes.arrayOf(
+        PropTypes.shape({
+            baseLayerMD5: PropTypes.string
+        })
+    ),
+    id: PropTypes.number.isRequired,
+    insetIconURL: PropTypes.string,
+    internetConnectionRequired: PropTypes.bool,
+    name: PropTypes.oneOfType([
+        PropTypes.string,
+        PropTypes.node
+    ]),
+    onMouseEnter: PropTypes.func.isRequired,
+    onMouseLeave: PropTypes.func.isRequired,
+    onSelect: PropTypes.func.isRequired
+};
+
+export default injectIntl(LibraryItem);

src/containers/sprite-library.jsx

@@ -23,21 +23,8 @@ class SpriteLibrary extends React.PureComponent {
     constructor (props) {
         super(props);
         bindAll(this, [
-            'handleItemSelect',
-            'handleMouseEnter',
-            'handleMouseLeave',
-            'rotateCostume',
-            'startRotatingCostumes',
-            'stopRotatingCostumes'
+            'handleItemSelect'
         ]);
-        this.state = {
-            activeSprite: null,
-            costumeIndex: 0,
-            sprites: spriteLibraryContent
-        };
-    }
-    componentWillUnmount () {
-        clearInterval(this.intervalId);
     }
     handleItemSelect (item) {
         // Randomize position of library sprite
@@ -51,46 +38,13 @@ class SpriteLibrary extends React.PureComponent {
             label: item.name
         });
     }
-    handleMouseEnter (item) {
-        this.stopRotatingCostumes();
-        this.setState({activeSprite: item}, this.startRotatingCostumes);
-    }
-    handleMouseLeave () {
-        this.stopRotatingCostumes();
-    }
-    startRotatingCostumes () {
-        if (!this.state.activeSprite) return;
-        this.rotateCostume();
-        this.intervalId = setInterval(this.rotateCostume, 300);
-    }
-    stopRotatingCostumes () {
-        this.intervalId = clearInterval(this.intervalId);
-    }
-    rotateCostume () {
-        const costumes = this.state.activeSprite.json.costumes;
-        const nextCostumeIndex = (this.state.costumeIndex + 1) % costumes.length;
-        this.setState({
-            costumeIndex: nextCostumeIndex,
-            sprites: this.state.sprites.map(sprite => {
-                if (sprite.name === this.state.activeSprite.name) {
-                    return {
-                        ...sprite,
-                        md5: sprite.json.costumes[nextCostumeIndex].baseLayerMD5
-                    };
-                }
-                return sprite;
-            })
-        });
-    }
     render () {
         return (
             <LibraryComponent
-                data={this.state.sprites}
+                data={spriteLibraryContent}
                 id="spriteLibrary"
                 tags={spriteTags}
                 title={this.props.intl.formatMessage(messages.libraryTitle)}
-                onItemMouseEnter={this.handleMouseEnter}
-                onItemMouseLeave={this.handleMouseLeave}
                 onItemSelected={this.handleItemSelect}
                 onRequestClose={this.props.onRequestClose}
             />

test/integration/costumes.test.js

@@ -164,4 +164,21 @@ describe('Working with costumes', () => {
         const logs = await getLogs();
         await expect(logs).toEqual([]);
     });
+
+    test('Costumes animate on mouseover', async () => {
+        await loadUri(uri);
+        await clickXpath('//button[@title="Try It"]');
+        await clickXpath('//button[@aria-label="Choose a Sprite"]');
+        const searchElement = await findByXpath("//input[@placeholder='Search']");
+        await searchElement.sendKeys('abb');
+        const abbyElement = await findByXpath('//*[span[text()="Abby"]]');
+        driver.actions()
+            .mouseMove(abbyElement)
+            .perform();
+        // wait for one of Abby's alternate costumes to appear
+        await findByXpath('//img[@src="https://cdn.assets.scratch.mit.edu/internalapi/asset/b6e23922f23b49ddc6f62f675e77417c.svg/get/"]');
+        const logs = await getLogs();
+        await expect(logs).toEqual([]);
+    });
+
 });