Merge pull request scratchfoundation#4542 from towerofnix/better-shareBlocksToTarget

Position blocks added from "share the love" within the viewport

Author: benjiwheeler

src/containers/blocks.jsx

@@ -14,7 +14,7 @@ import ExtensionLibrary from './extension-library.jsx';
 import extensionData from '../lib/libraries/extensions/index.jsx';
 import CustomProcedures from './custom-procedures.jsx';
 import errorBoundaryHOC from '../lib/error-boundary-hoc.jsx';
-import {STAGE_DISPLAY_SIZES} from '../lib/layout-constants';
+import {BLOCKS_DEFAULT_SCALE, STAGE_DISPLAY_SIZES} from '../lib/layout-constants';
 import DropAreaHOC from '../lib/drop-area-hoc.jsx';
 import DragConstants from '../lib/drag-constants';
 import defineDynamicBlock from '../lib/define-dynamic-block';
@@ -25,6 +25,7 @@ import {activateColorPicker} from '../reducers/color-picker';
 import {closeExtensionLibrary, openSoundRecorder, openConnectionModal} from '../reducers/modals';
 import {activateCustomProcedures, deactivateCustomProcedures} from '../reducers/custom-procedures';
 import {setConnectionModalExtensionId} from '../reducers/connection-modal';
+import {updateMetrics} from '../reducers/workspace-metrics';
 
 import {
     activateTab,
@@ -79,7 +80,6 @@ class Blocks extends React.Component {
         this.ScratchBlocks.recordSoundCallback = this.handleOpenSoundRecorder;
 
         this.state = {
-            workspaceMetrics: {},
             prompt: null
         };
         this.onTargetsUpdate = debounce(this.onTargetsUpdate, 100);
@@ -301,14 +301,17 @@ class Blocks extends React.Component {
     onWorkspaceMetricsChange () {
         const target = this.props.vm.editingTarget;
         if (target && target.id) {
-            const workspaceMetrics = Object.assign({}, this.state.workspaceMetrics, {
-                [target.id]: {
+            // Dispatch updateMetrics later, since onWorkspaceMetricsChange may be (very indirectly)
+            // called from a reducer, i.e. when you create a custom procedure.
+            // TODO: Is this a vehement hack?
+            setTimeout(() => {
+                this.props.updateMetrics({
+                    targetID: target.id,
                     scrollX: this.workspace.scrollX,
                     scrollY: this.workspace.scrollY,
                     scale: this.workspace.scale
-                }
-            });
-            this.setState({workspaceMetrics});
+                });
+            }, 0);
         }
     }
     onScriptGlowOn (data) {
@@ -355,7 +358,7 @@ class Blocks extends React.Component {
             this.props.updateToolboxState(toolboxXML);
         }
 
-        if (this.props.vm.editingTarget && !this.state.workspaceMetrics[this.props.vm.editingTarget.id]) {
+        if (this.props.vm.editingTarget && !this.props.workspaceMetrics.targets[this.props.vm.editingTarget.id]) {
             this.onWorkspaceMetricsChange();
         }
 
@@ -381,8 +384,8 @@ class Blocks extends React.Component {
         }
         this.workspace.addChangeListener(this.props.vm.blockListener);
 
-        if (this.props.vm.editingTarget && this.state.workspaceMetrics[this.props.vm.editingTarget.id]) {
-            const {scrollX, scrollY, scale} = this.state.workspaceMetrics[this.props.vm.editingTarget.id];
+        if (this.props.vm.editingTarget && this.props.workspaceMetrics.targets[this.props.vm.editingTarget.id]) {
+            const {scrollX, scrollY, scale} = this.props.workspaceMetrics.targets[this.props.vm.editingTarget.id];
             this.workspace.scrollX = scrollX;
             this.workspace.scrollY = scrollY;
             this.workspace.scale = scale;
@@ -525,6 +528,8 @@ class Blocks extends React.Component {
             onRequestCloseExtensionLibrary,
             onRequestCloseCustomProcedures,
             toolboxXML,
+            updateMetrics: updateMetricsProp,
+            workspaceMetrics,
             ...props
         } = this.props;
         /* eslint-enable no-unused-vars */
@@ -607,15 +612,19 @@ Blocks.propTypes = {
     }),
     stageSize: PropTypes.oneOf(Object.keys(STAGE_DISPLAY_SIZES)).isRequired,
     toolboxXML: PropTypes.string,
+    updateMetrics: PropTypes.func,
     updateToolboxState: PropTypes.func,
-    vm: PropTypes.instanceOf(VM).isRequired
+    vm: PropTypes.instanceOf(VM).isRequired,
+    workspaceMetrics: PropTypes.shape({
+        targets: PropTypes.objectOf(PropTypes.object)
+    })
 };
 
 Blocks.defaultOptions = {
     zoom: {
         controls: true,
         wheel: true,
-        startScale: 0.675
+        startScale: BLOCKS_DEFAULT_SCALE
     },
     grid: {
         spacing: 40,
@@ -654,7 +663,8 @@ const mapStateToProps = state => ({
     locale: state.locales.locale,
     messages: state.locales.messages,
     toolboxXML: state.scratchGui.toolbox.toolboxXML,
-    customProceduresVisible: state.scratchGui.customProcedures.active
+    customProceduresVisible: state.scratchGui.customProcedures.active,
+    workspaceMetrics: state.scratchGui.workspaceMetrics
 });
 
 const mapDispatchToProps = dispatch => ({
@@ -676,6 +686,9 @@ const mapDispatchToProps = dispatch => ({
     },
     updateToolboxState: toolboxXML => {
         dispatch(updateToolbox(toolboxXML));
+    },
+    updateMetrics: metrics => {
+        dispatch(updateMetrics(metrics));
     }
 });
 

src/containers/target-pane.jsx

@@ -14,6 +14,7 @@ import {showStandardAlert, closeAlertWithId} from '../reducers/alerts';
 import {setRestore} from '../reducers/restore-deletion';
 import DragConstants from '../lib/drag-constants';
 import TargetPaneComponent from '../components/target-pane/target-pane.jsx';
+import {BLOCKS_DEFAULT_SCALE} from '../lib/layout-constants';
 import spriteLibraryContent from '../lib/libraries/sprites.json';
 import {handleFileUpload, spriteUpload} from '../lib/file-uploader.js';
 import sharedMessages from '../lib/shared-messages';
@@ -156,10 +157,42 @@ class TargetPane extends React.Component {
     }
     handleBlockDragEnd (blocks) {
         if (this.props.hoveredTarget.sprite && this.props.hoveredTarget.sprite !== this.props.editingTarget) {
-            this.props.vm.shareBlocksToTarget(blocks, this.props.hoveredTarget.sprite, this.props.editingTarget);
+            this.shareBlocks(blocks, this.props.hoveredTarget.sprite, this.props.editingTarget);
             this.props.onReceivedBlocks(true);
         }
     }
+    shareBlocks (blocks, targetId, optFromTargetId) {
+        // Position the top-level block based on the scroll position.
+        const topBlock = blocks.find(block => block.topLevel);
+        if (topBlock) {
+            let metrics;
+            if (this.props.workspaceMetrics.targets[targetId]) {
+                metrics = this.props.workspaceMetrics.targets[targetId];
+            } else {
+                metrics = {
+                    scrollX: 0,
+                    scrollY: 0,
+                    scale: BLOCKS_DEFAULT_SCALE
+                };
+            }
+
+            // Determine position of the top-level block based on the target's workspace metrics.
+            const {scrollX, scrollY, scale} = metrics;
+            const posY = -scrollY + 30;
+            let posX;
+            if (this.props.isRtl) {
+                posX = scrollX + 30;
+            } else {
+                posX = -scrollX + 30;
+            }
+
+            // Actually apply the position!
+            topBlock.x = posX / scale;
+            topBlock.y = posY / scale;
+        }
+
+        this.props.vm.shareBlocksToTarget(blocks, targetId, optFromTargetId);
+    }
     handleDrop (dragInfo) {
         const {sprite: targetId} = this.props.hoveredTarget;
         if (dragInfo.dragType === DragConstants.SPRITE) {
@@ -196,22 +229,25 @@ class TargetPane extends React.Component {
             } else if (dragInfo.dragType === DragConstants.BACKPACK_CODE) {
                 fetchCode(dragInfo.payload.bodyUrl)
                     .then(blocks => {
-                        this.props.vm.shareBlocksToTarget(blocks, targetId);
+                        this.shareBlocks(blocks, targetId);
                         this.props.vm.refreshWorkspace();
                     });
             }
         }
     }
     render () {
+        /* eslint-disable no-unused-vars */
         const {
-            onActivateTab, // eslint-disable-line no-unused-vars
-            onReceivedBlocks, // eslint-disable-line no-unused-vars
-            onHighlightTarget, // eslint-disable-line no-unused-vars
-            dispatchUpdateRestore, // eslint-disable-line no-unused-vars
-            onShowImporting, // eslint-disable-line no-unused-vars
-            onCloseImporting, // eslint-disable-line no-unused-vars
+            dispatchUpdateRestore,
+            onActivateTab,
+            onCloseImporting,
+            onHighlightTarget,
+            onReceivedBlocks,
+            onShowImporting,
+            workspaceMetrics,
             ...componentProps
         } = this.props;
+        /* eslint-enable no-unused-vars */
         return (
             <TargetPaneComponent
                 {...componentProps}
@@ -254,10 +290,12 @@ TargetPane.propTypes = {
 const mapStateToProps = state => ({
     editingTarget: state.scratchGui.targets.editingTarget,
     hoveredTarget: state.scratchGui.hoveredTarget,
+    isRtl: state.locales.isRtl,
+    spriteLibraryVisible: state.scratchGui.modals.spriteLibrary,
     sprites: state.scratchGui.targets.sprites,
     stage: state.scratchGui.targets.stage,
     raiseSprites: state.scratchGui.blockDrag,
-    spriteLibraryVisible: state.scratchGui.modals.spriteLibrary
+    workspaceMetrics: state.scratchGui.workspaceMetrics
 });
 
 const mapDispatchToProps = dispatch => ({

src/lib/layout-constants.js

@@ -37,6 +37,9 @@ const STAGE_DISPLAY_SIZES = keyMirror({
     small: null
 });
 
+// zoom level to start with
+const BLOCKS_DEFAULT_SCALE = 0.675;
+
 const STAGE_DISPLAY_SCALES = {};
 STAGE_DISPLAY_SCALES[STAGE_DISPLAY_SIZES.large] = 1; // large mode, wide browser (standard)
 STAGE_DISPLAY_SCALES[STAGE_DISPLAY_SIZES.largeConstrained] = 0.85; // large mode but narrow browser
@@ -50,6 +53,7 @@ export default {
 };
 
 export {
+    BLOCKS_DEFAULT_SCALE,
     STAGE_DISPLAY_SCALES,
     STAGE_DISPLAY_SIZES,
     STAGE_SIZE_MODES

src/reducers/gui.js

@@ -25,6 +25,7 @@ import timeoutReducer, {timeoutInitialState} from './timeout';
 import toolboxReducer, {toolboxInitialState} from './toolbox';
 import vmReducer, {vmInitialState} from './vm';
 import vmStatusReducer, {vmStatusInitialState} from './vm-status';
+import workspaceMetricsReducer, {workspaceMetricsInitialState} from './workspace-metrics';
 import throttle from 'redux-throttle';
 
 import decks from '../lib/libraries/decks/index.jsx';
@@ -57,7 +58,8 @@ const guiInitialState = {
     timeout: timeoutInitialState,
     toolbox: toolboxInitialState,
     vm: vmInitialState,
-    vmStatus: vmStatusInitialState
+    vmStatus: vmStatusInitialState,
+    workspaceMetrics: workspaceMetricsInitialState
 };
 
 const initPlayer = function (currentState) {
@@ -155,7 +157,8 @@ const guiReducer = combineReducers({
     timeout: timeoutReducer,
     toolbox: toolboxReducer,
     vm: vmReducer,
-    vmStatus: vmStatusReducer
+    vmStatus: vmStatusReducer,
+    workspaceMetrics: workspaceMetricsReducer
 });
 
 export {

src/reducers/workspace-metrics.js

@@ -0,0 +1,37 @@
+const UPDATE_METRICS = 'scratch-gui/workspace-metrics/UPDATE_METRICS';
+
+const initialState = {
+    targets: {}
+};
+
+const reducer = function (state, action) {
+    if (typeof state === 'undefined') state = initialState;
+
+    switch (action.type) {
+    case UPDATE_METRICS:
+        return Object.assign({}, state, {
+            targets: Object.assign({}, state.targets, {
+                [action.targetID]: {
+                    scrollX: action.scrollX,
+                    scrollY: action.scrollY,
+                    scale: action.scale
+                }
+            })
+        });
+    default:
+        return state;
+    }
+};
+
+const updateMetrics = function (metrics) {
+    return {
+        type: UPDATE_METRICS,
+        ...metrics
+    };
+};
+
+export {
+    reducer as default,
+    initialState as workspaceMetricsInitialState,
+    updateMetrics
+};

test/unit/reducers/workspace-metrics-reducer.test.js

@@ -0,0 +1,25 @@
+/* eslint-env jest */
+import workspaceMetricsReducer, {updateMetrics} from '../../../src/reducers/workspace-metrics';
+
+test('initialState', () => {
+    let defaultState;
+    /* workspaceMetricsReducer(state, action) */
+    expect(workspaceMetricsReducer(defaultState, {type: 'anything'})).toBeDefined();
+    expect(workspaceMetricsReducer(defaultState, {type: 'anything'})).toEqual({targets: {}});
+});
+
+test('updateMetrics action creator', () => {
+    let defaultState;
+    const action = updateMetrics({
+        targetID: 'abcde',
+        scrollX: 225,
+        scrollY: 315,
+        scale: 1.25
+    });
+    const resultState = workspaceMetricsReducer(defaultState, action);
+    expect(Object.keys(resultState.targets).length).toBe(1);
+    expect(resultState.targets.abcde).toBeDefined();
+    expect(resultState.targets.abcde.scrollX).toBe(225);
+    expect(resultState.targets.abcde.scrollY).toBe(315);
+    expect(resultState.targets.abcde.scale).toBe(1.25);
+});