diff --git a/package.json b/package.json index 6603b7ee4f0a93c64dd7c528958b82e215ad0d75..2f9bc35ee5b8a96c41686f96c348108f12c6f363 100644 --- a/package.json +++ b/package.json @@ -104,13 +104,12 @@ "redux-throttle": "0.1.1", "rimraf": "^2.6.1", "scratch-audio": "0.1.0-prerelease.20190114210212", - "scratch-blocks": "0.1.0-prerelease.1547735159", "scratch-l10n": "3.1.20190123150639", "scratch-paint": "0.2.0-prerelease.20190114205252", "scratch-render": "0.1.0-prerelease.20190122223537", "scratch-storage": "1.2.2", "scratch-svg-renderer": "0.2.0-prerelease.20190110205335", - "scratch-vm": "0.2.0-prerelease.20190118221822", + "scratch-vm": "0.2.0-prerelease.20190123164824", "selenium-webdriver": "3.6.0", "startaudiocontext": "1.2.1", "style-loader": "^0.23.0", diff --git a/src/components/meter/meter.css b/src/components/meter/meter.css index c16e920cd73f18e274a80449706509f7042e8235..953f3641b754caddcb68a17eb4d38642127a57bd 100644 --- a/src/components/meter/meter.css +++ b/src/components/meter/meter.css @@ -1,3 +1,5 @@ +@import "../../css/colors.css"; + .green { fill: rgb(171, 220, 170); stroke: rgb(174, 211, 168); @@ -12,3 +14,19 @@ fill: rgb(251, 194, 142); stroke: rgb(235, 189, 142); } + +.mask-container { + position: relative; +} + +.mask { + position: absolute; + top: 0; + left: 0; + height: 100%; + width: 100%; + transform-origin: top; + will-change: transform; + background: $ui-primary; + opacity: 0.75; +} diff --git a/src/components/meter/meter.jsx b/src/components/meter/meter.jsx index 2d16ce681da69f1d1160b41f3b08230d8729b2e9..4f07119168950ee39fb3f285717499981eb04330 100644 --- a/src/components/meter/meter.jsx +++ b/src/components/meter/meter.jsx @@ -20,36 +20,40 @@ const Meter = props => { const barHeight = (height - (barSpacing * (nBars + 1))) / nBars; const nBarsToMask = nBars - Math.floor(level * nBars); + const scale = ((nBarsToMask * (barHeight + barSpacing)) + (barSpacing / 2)) / height; return ( - <svg - className={styles.container} - height={height} - width={width} + <div + className={styles.maskContainer} + style={{height: `${height}px`}} > - {Array(nBars).fill(0) - .map((value, index) => ( - <rect - className={index < nGreen ? styles.green : - (index < nGreen + nYellow ? styles.yellow : styles.red)} - height={barHeight} - key={index} - rx={barRounding} - ry={barRounding} - width={width - 2} - x={1} - y={height - ((barSpacing + barHeight) * (index + 1))} - /> - ))} - <rect - fill="hsla(215, 100%, 95%, 1)" - height={(nBarsToMask * (barHeight + barSpacing)) + (barSpacing / 2)} - opacity="0.75" + <svg + className={styles.container} + height={height} width={width} - x={0} - y={0} + > + {Array(nBars).fill(0) + .map((value, index) => ( + <rect + className={index < nGreen ? styles.green : + (index < nGreen + nYellow ? styles.yellow : styles.red)} + height={barHeight} + key={index} + rx={barRounding} + ry={barRounding} + width={width - 2} + x={1} + y={height - ((barSpacing + barHeight) * (index + 1))} + /> + ))} + </svg> + <div + className={styles.mask} + style={{ + transform: `scaleY(${scale})` + }} /> - </svg> + </div> ); }; diff --git a/src/components/sprite-selector/sprite-list.jsx b/src/components/sprite-selector/sprite-list.jsx index de727969e175aed1146730740416d9889e0a96e4..e89898e47ce621fb3c6e7a349c7ac0fc0cdd497b 100644 --- a/src/components/sprite-selector/sprite-list.jsx +++ b/src/components/sprite-selector/sprite-list.jsx @@ -8,9 +8,12 @@ import Box from '../box/box.jsx'; import SpriteSelectorItem from '../../containers/sprite-selector-item.jsx'; import SortableHOC from '../../lib/sortable-hoc.jsx'; import SortableAsset from '../asset-panel/sortable-asset.jsx'; +import ThrottledPropertyHOC from '../../lib/throttled-property-hoc.jsx'; import styles from './sprite-selector.css'; +const ThrottledSpriteSelectorItem = ThrottledPropertyHOC('asset', 500)(SpriteSelectorItem); + const SpriteList = function (props) { const { containerRef, @@ -74,13 +77,13 @@ const SpriteList = function (props) { onAddSortable={onAddSortable} onRemoveSortable={onRemoveSortable} > - <SpriteSelectorItem + <ThrottledSpriteSelectorItem asset={sprite.costume && sprite.costume.asset} className={classNames(styles.sprite, { [styles.raised]: isRaised, [styles.receivedBlocks]: receivedBlocks })} - dragPayload={sprite} + dragPayload={sprite.id} dragType={DragConstants.SPRITE} id={sprite.id} index={index} diff --git a/src/containers/sprite-selector-item.jsx b/src/containers/sprite-selector-item.jsx index f284be34e1990ee13936d1dfdc6cbcc204433faf..f3f79bb56e55dad12852ecc54867445248fb36de 100644 --- a/src/containers/sprite-selector-item.jsx +++ b/src/containers/sprite-selector-item.jsx @@ -14,7 +14,7 @@ import SpriteSelectorItemComponent from '../components/sprite-selector-item/spri const dragThreshold = 3; // Same as the block drag threshold -class SpriteSelectorItem extends React.Component { +class SpriteSelectorItem extends React.PureComponent { constructor (props) { super(props); bindAll(this, [ @@ -29,19 +29,6 @@ class SpriteSelectorItem extends React.Component { 'handleMouseMove', 'handleMouseUp' ]); - - // Asset ID of the current decoded costume - this.decodedAssetId = null; - } - shouldComponentUpdate (nextProps) { - // Ignore dragPayload due to https://github.com/LLK/scratch-gui/issues/3172. - // This function should be removed once the issue is fixed. - for (const property in nextProps) { - if (property !== 'dragPayload' && this.props[property] !== nextProps[property]) { - return true; - } - } - return false; } getCostumeData () { if (this.props.costumeURL) return this.props.costumeURL; @@ -153,10 +140,7 @@ SpriteSelectorItem.propTypes = { asset: PropTypes.instanceOf(storage.Asset), costumeURL: PropTypes.string, dispatchSetHoveredSprite: PropTypes.func.isRequired, - dragPayload: PropTypes.shape({ - name: PropTypes.string, - body: PropTypes.string - }), + dragPayload: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), dragType: PropTypes.string, dragging: PropTypes.bool, id: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), diff --git a/src/containers/stage-selector.jsx b/src/containers/stage-selector.jsx index 08ffd3730a520da9a16a92af68fde070d8e3a1ee..988ebeddc7c5fbf9f4ae97ac5c3cc1b7828751ec 100644 --- a/src/containers/stage-selector.jsx +++ b/src/containers/stage-selector.jsx @@ -10,6 +10,7 @@ import {activateTab, COSTUMES_TAB_INDEX} from '../reducers/editor-tab'; import {setHoveredSprite} from '../reducers/hovered-target'; import DragConstants from '../lib/drag-constants'; import DropAreaHOC from '../lib/drop-area-hoc.jsx'; +import ThrottledPropertyHOC from '../lib/throttled-property-hoc.jsx'; import {emptyCostume} from '../lib/empty-assets'; import sharedMessages from '../lib/shared-messages'; import {fetchCode} from '../lib/backpack-api'; @@ -27,7 +28,9 @@ const dragTypes = [ DragConstants.BACKPACK_CODE ]; -const DroppableStage = DropAreaHOC(dragTypes)(StageSelectorComponent); +const DroppableThrottledStage = DropAreaHOC(dragTypes)( + ThrottledPropertyHOC('url', 500)(StageSelectorComponent) +); class StageSelector extends React.Component { constructor (props) { @@ -116,7 +119,7 @@ class StageSelector extends React.Component { const componentProps = omit(this.props, [ 'asset', 'dispatchSetHoveredSprite', 'id', 'intl', 'onActivateTab', 'onSelect']); return ( - <DroppableStage + <DroppableThrottledStage fileInputRef={this.setFileInput} onBackdropFileUpload={this.handleBackdropUpload} onBackdropFileUploadClick={this.handleFileUploadClick} diff --git a/src/containers/watermark.jsx b/src/containers/watermark.jsx index 8cc9324b933c6ea5ec3976f0c5084b1310980b3b..fcb08ead285571471953bbec9b8a2aa24edbc056 100644 --- a/src/containers/watermark.jsx +++ b/src/containers/watermark.jsx @@ -4,6 +4,8 @@ import PropTypes from 'prop-types'; import React from 'react'; import {connect} from 'react-redux'; +import ThrottledPropertyHOC from '../lib/throttled-property-hoc.jsx'; + import VM from 'scratch-vm'; import storage from '../lib/storage'; import getCostumeUrl from '../lib/get-costume-url'; @@ -16,8 +18,6 @@ class Watermark extends React.Component { bindAll(this, [ 'getCostumeData' ]); - // Asset ID of the current sprite's current costume - this.decodedAssetId = null; } getCostumeData () { @@ -64,6 +64,8 @@ const mapStateToProps = state => { const ConnectedComponent = connect( mapStateToProps -)(Watermark); +)( + ThrottledPropertyHOC('asset', 500)(Watermark) +); export default ConnectedComponent; diff --git a/src/lib/backpack/sprite-payload.js b/src/lib/backpack/sprite-payload.js index 12fc68c40d467eedd591ed4b9adf9f8e070adad2..6343dc407017ef287beede13b3f4923df1bbb784 100644 --- a/src/lib/backpack/sprite-payload.js +++ b/src/lib/backpack/sprite-payload.js @@ -1,23 +1,29 @@ import jpegThumbnail from './jpeg-thumbnail'; -const spritePayload = (sprite, vm) => vm.exportSprite( - sprite.id, - 'base64' -).then(zippedSprite => { - const payload = { - type: 'sprite', - name: sprite.name, - mime: 'application/zip', - body: zippedSprite, - // Filled in below - thumbnail: '' - }; +const spritePayload = (id, vm) => { + const target = vm.runtime.getTargetById(id); + if (!target) return null; - const costumeDataUrl = sprite.costume.asset.encodeDataURI(); - return jpegThumbnail(costumeDataUrl).then(thumbnail => { - payload.thumbnail = thumbnail.replace('data:image/jpeg;base64,', ''); - return payload; + return vm.exportSprite( + id, + 'base64' + ).then(zippedSprite => { + const payload = { + type: 'sprite', + name: target.sprite.name, + mime: 'application/zip', + body: zippedSprite, + // Filled in below + thumbnail: '' + }; + + const costumeDataUrl = target.sprite.costumes[target.currentCostume].asset.encodeDataURI(); + + return jpegThumbnail(costumeDataUrl).then(thumbnail => { + payload.thumbnail = thumbnail.replace('data:image/jpeg;base64,', ''); + return payload; + }); }); -}); +}; export default spritePayload; diff --git a/src/lib/throttled-property-hoc.jsx b/src/lib/throttled-property-hoc.jsx new file mode 100644 index 0000000000000000000000000000000000000000..7e508dfbea48be828ddfdde0c38cb4bf17c7542a --- /dev/null +++ b/src/lib/throttled-property-hoc.jsx @@ -0,0 +1,49 @@ +import React from 'react'; + +/* Higher Order Component to throttle updates to specific props. + * Why? Because certain prop updates are expensive, and need to be throttled. + * This allows renders when other properties change, and will use the last + * rendered value of a prop for comparison. + * @param {string} propName the name of the prop to throttle updates from. + * @param {string} throttleTime the minimum time between updates to that specific property. + * @returns {function} a function that accepts a component to wrap. + */ +const ThrottledPropertyHOC = function (propName, throttleTime) { + /** + * The function to be called with a React component to wrap it. + * @param {React.Component} WrappedComponent - Component to wrap with throttler. + * @returns {React.Component} the component wrapped with the throttler. + */ + return function (WrappedComponent) { + class ThrottledPropertyWrapper extends React.Component { + shouldComponentUpdate (nextProps) { + for (const property in nextProps) { + if (property !== propName && this.props[property] !== nextProps[property]) { + return true; // Always update if another property has changed + } + } + + // If only that prop has changed, allow update to go to render based + // on _lastRenderedTime and _lastRenderTime are updated in render + if (nextProps[propName] !== this._lastRenderedValue && + Date.now() - this._lastRenderTime > throttleTime + ) { + return true; // Allow this update to go to render + } + + return false; + } + render () { + this._lastRenderTime = Date.now(); + this._lastRenderedValue = this.props[propName]; + return ( + <WrappedComponent {...this.props} /> + ); + } + } + + return ThrottledPropertyWrapper; + }; +}; + +export default ThrottledPropertyHOC; diff --git a/src/lib/vm-listener-hoc.jsx b/src/lib/vm-listener-hoc.jsx index 5e51aad16a6f1f5539a10706b995edd8abca6c69..efeaa35008af225e9d0479339cbcc7a2f1212760 100644 --- a/src/lib/vm-listener-hoc.jsx +++ b/src/lib/vm-listener-hoc.jsx @@ -164,7 +164,9 @@ const vmListenerHOC = function (WrappedComponent) { }; const mapStateToProps = state => ({ // Do not emit target or project updates in fullscreen or player only mode - shouldEmitUpdates: !state.scratchGui.mode.isFullScreen && !state.scratchGui.mode.isPlayerOnly, + // or when recording sounds (it leads to garbled recordings on low-power machines) + shouldEmitUpdates: !state.scratchGui.mode.isFullScreen && !state.scratchGui.mode.isPlayerOnly && + !state.scratchGui.modals.soundRecorder, vm: state.scratchGui.vm, username: state.session && state.session.session && state.session.session.user ? state.session.session.user.username : '' diff --git a/src/reducers/targets.js b/src/reducers/targets.js index 17a73f7d88f350e65ee44530ad8de6ced6521df8..7da81982aa4103ffe6088b76ee8a0c77e7e10a19 100644 --- a/src/reducers/targets.js +++ b/src/reducers/targets.js @@ -39,10 +39,7 @@ const updateTargets = function (targetList, editingTarget) { return { type: UPDATE_TARGET_LIST, targets: targetList, - editingTarget: editingTarget, - meta: { - throttle: 30 - } + editingTarget: editingTarget }; }; const highlightTarget = function (targetId) { diff --git a/test/unit/util/throttled-property-hoc.test.jsx b/test/unit/util/throttled-property-hoc.test.jsx new file mode 100644 index 0000000000000000000000000000000000000000..c24f937a357f9195d593aa31820452ca608f08bb --- /dev/null +++ b/test/unit/util/throttled-property-hoc.test.jsx @@ -0,0 +1,54 @@ +import React from 'react'; +import {mount} from 'enzyme'; + +import ThrottledPropertyHOC from '../../../src/lib/throttled-property-hoc.jsx'; + +describe('VMListenerHOC', () => { + let mounted; + const throttleTime = 500; + beforeEach(() => { + const Component = ({propToThrottle, doNotThrottle}) => ( + <input + name={doNotThrottle} + value={propToThrottle} + /> + ); + const WrappedComponent = ThrottledPropertyHOC('propToThrottle', throttleTime)(Component); + + global.Date.now = () => 0; + + mounted = mount( + <WrappedComponent + doNotThrottle="oldvalue" + propToThrottle={0} + /> + ); + }); + + test('it passes the props on initial render ', () => { + expect(mounted.find('[value=0]').exists()).toEqual(true); + expect(mounted.find('[name="oldvalue"]').exists()).toEqual(true); + }); + + test('it does not rerender if throttled prop is updated too soon', () => { + global.Date.now = () => throttleTime / 2; + mounted.setProps({propToThrottle: 1}); + mounted.update(); + expect(mounted.find('[value=0]').exists()).toEqual(true); + }); + + test('it does rerender if throttled prop is updated after throttle timeout', () => { + global.Date.now = () => throttleTime * 2; + mounted.setProps({propToThrottle: 1}); + mounted.update(); + expect(mounted.find('[value=1]').exists()).toEqual(true); + }); + + test('it does rerender if a non-throttled prop is changed', () => { + global.Date.now = () => throttleTime / 2; + mounted.setProps({doNotThrottle: 'newvalue', propToThrottle: 2}); + mounted.update(); + expect(mounted.find('[name="newvalue"]').exists()).toEqual(true); + expect(mounted.find('[value=2]').exists()).toEqual(true); + }); +}); diff --git a/test/unit/util/vm-listener-hoc.test.jsx b/test/unit/util/vm-listener-hoc.test.jsx index a6c61c1a1e96ee34fb67ba01c623b8d3dbdc2473..33753cf8e3d58770626de8937870fe2d1d9b424f 100644 --- a/test/unit/util/vm-listener-hoc.test.jsx +++ b/test/unit/util/vm-listener-hoc.test.jsx @@ -15,6 +15,7 @@ describe('VMListenerHOC', () => { store = mockStore({ scratchGui: { mode: {}, + modals: {}, vm: vm } }); @@ -49,4 +50,45 @@ describe('VMListenerHOC', () => { const child = wrapper.find(Component); expect(child.props().onGreenFlag).toBeUndefined(); }); + + test('targetsUpdate event from vm triggers targets update action', () => { + const Component = () => (<div />); + const WrappedComponent = vmListenerHOC(Component); + mount( + <WrappedComponent + store={store} + vm={vm} + /> + ); + const targetList = []; + const editingTarget = 'id'; + vm.emit('targetsUpdate', {targetList, editingTarget}); + const actions = store.getActions(); + expect(actions[0].type).toEqual('scratch-gui/targets/UPDATE_TARGET_LIST'); + expect(actions[0].targets).toEqual(targetList); + expect(actions[0].editingTarget).toEqual(editingTarget); + }); + + test('targetsUpdate does not dispatch if the sound recorder is visible', () => { + const Component = () => (<div />); + const WrappedComponent = vmListenerHOC(Component); + store = mockStore({ + scratchGui: { + mode: {}, + modals: {soundRecorder: true}, + vm: vm + } + }); + mount( + <WrappedComponent + store={store} + vm={vm} + /> + ); + const targetList = []; + const editingTarget = 'id'; + vm.emit('targetsUpdate', {targetList, editingTarget}); + const actions = store.getActions(); + expect(actions.length).toEqual(0); + }); });