Merge pull request #976 from fsih/monitorLayout

Monitor layout

src/components/monitor-list/monitor-list.jsx

@@ -11,10 +11,9 @@ const MonitorList = props => (
     <Box
         className={styles.monitorList}
     >
-        {props.monitors.valueSeq().map((monitorData, index) => (
+        {props.monitors.valueSeq().map(monitorData => (
             <Monitor
                 id={monitorData.id}
-                index={index}
                 key={monitorData.id}
                 opcode={monitorData.opcode}
                 params={monitorData.params}

src/components/monitor/monitor.jsx

@@ -16,13 +16,12 @@ const MonitorComponent = props => (
     <Draggable
         bounds="parent"
         defaultClassNameDragging={styles.dragging}
-        defaultPosition={{
-            x: props.x,
-            y: props.y
-        }}
         onStop={props.onDragEnd}
     >
-        <Box className={styles.monitor}>
+        <Box
+            className={styles.monitor}
+            componentRef={props.componentRef}
+        >
             <Box className={styles.label}>
                 {props.label}
             </Box>
@@ -40,17 +39,14 @@ MonitorComponent.categories = categories;
 
 MonitorComponent.propTypes = {
     category: PropTypes.oneOf(Object.keys(categories)),
+    componentRef: PropTypes.func.isRequired,
     label: PropTypes.string.isRequired,
     onDragEnd: PropTypes.func.isRequired,
-    value: PropTypes.string.isRequired,
-    x: PropTypes.number,
-    y: PropTypes.number
+    value: PropTypes.string.isRequired
 };
 
 MonitorComponent.defaultProps = {
-    category: 'data',
-    x: 0,
-    y: 0
+    category: 'data'
 };
 
 export default MonitorComponent;

src/containers/monitor-list.jsx

@@ -1,7 +1,9 @@
 import bindAll from 'lodash.bindall';
 import React from 'react';
+import PropTypes from 'prop-types';
 
 import {connect} from 'react-redux';
+import {moveMonitorRect} from '../reducers/monitor-layout';
 
 import MonitorListComponent from '../components/monitor-list/monitor-list.jsx';
 
@@ -13,7 +15,7 @@ class MonitorList extends React.Component {
         ]);
     }
     handleMonitorChange (id, x, y) { // eslint-disable-line no-unused-vars
-        // @todo send this event to the VM
+        this.props.moveMonitorRect(id, x, y);
     }
     render () {
         return (
@@ -25,10 +27,15 @@ class MonitorList extends React.Component {
     }
 }
 
+MonitorList.propTypes = {
+    moveMonitorRect: PropTypes.func.isRequired
+};
 const mapStateToProps = state => ({
     monitors: state.monitors
 });
-const mapDispatchToProps = () => ({});
+const mapDispatchToProps = dispatch => ({
+    moveMonitorRect: (id, x, y) => dispatch(moveMonitorRect(id, x, y))
+});
 
 export default connect(
     mapStateToProps,

src/containers/monitor.jsx

@@ -4,25 +4,69 @@ import PropTypes from 'prop-types';
 
 import monitorAdapter from '../lib/monitor-adapter.js';
 import MonitorComponent from '../components/monitor/monitor.jsx';
+import {addMonitorRect, getInitialPosition, resizeMonitorRect, removeMonitorRect} from '../reducers/monitor-layout';
+
+import {connect} from 'react-redux';
 
 class Monitor extends React.Component {
     constructor (props) {
         super(props);
         bindAll(this, [
-            'handleDragEnd'
+            'handleDragEnd',
+            'setElement'
         ]);
     }
+    componentDidMount () {
+        let rect;
+        // Load the VM provided position if not loaded already
+        if (this.props.x && this.props.y && !this.props.monitorLayout.savedMonitorPositions[this.props.id]) {
+            rect = {
+                upperStart: {x: this.props.x, y: this.props.y},
+                lowerEnd: {x: this.props.x + this.element.offsetWidth, y: this.props.y + this.element.offsetHeight}
+            };
+            this.props.addMonitorRect(this.props.id, rect, true /* savePosition */);
+        } else { // Newly created user monitor
+            rect = getInitialPosition(
+                this.props.monitorLayout, this.props.id, this.element.offsetWidth, this.element.offsetHeight);
+            this.props.addMonitorRect(this.props.id, rect);
+        }
+        this.element.style.top = `${rect.upperStart.y}px`;
+        this.element.style.left = `${rect.upperStart.x}px`;
+    }
+    shouldComponentUpdate (nextProps, nextState) {
+        if (nextState !== this.state) {
+            return true;
+        }
+        for (const key of Object.getOwnPropertyNames(nextProps)) {
+            // Don't need to rerender when other monitors are moved.
+            // monitorLayout is only used during initial layout.
+            if (key !== 'monitorLayout' && nextProps[key] !== this.props[key]) {
+                return true;
+            }
+        }
+        return false;
+    }
+    componentDidUpdate () {
+        this.props.resizeMonitorRect(this.props.id, this.element.offsetWidth, this.element.offsetHeight);
+    }
+    componentWillUnmount () {
+        this.props.removeMonitorRect(this.props.id);
+    }
     handleDragEnd (e, {x, y}) {
         this.props.onDragEnd(
             this.props.id,
-            x,
-            y
+            parseInt(this.element.style.left, 10) + x,
+            parseInt(this.element.style.top, 10) + y
         );
     }
+    setElement (monitorElt) {
+        this.element = monitorElt;
+    }
     render () {
         const monitorProps = monitorAdapter(this.props);
         return (
             <MonitorComponent
+                componentRef={this.setElement}
                 {...monitorProps}
                 onDragEnd={this.handleDragEnd}
             />
@@ -31,13 +75,32 @@ class Monitor extends React.Component {
 }
 
 Monitor.propTypes = {
+    addMonitorRect: PropTypes.func.isRequired,
     id: PropTypes.string.isRequired,
-    index: PropTypes.number.isRequired, // eslint-disable-line react/no-unused-prop-types
+    monitorLayout: PropTypes.shape({
+        monitors: PropTypes.object,
+        savedMonitorPositions: PropTypes.object
+    }).isRequired,
     onDragEnd: PropTypes.func.isRequired,
     opcode: PropTypes.string.isRequired, // eslint-disable-line react/no-unused-prop-types
     params: PropTypes.object, // eslint-disable-line react/no-unused-prop-types, react/forbid-prop-types
+    removeMonitorRect: PropTypes.func.isRequired,
+    resizeMonitorRect: PropTypes.func.isRequired,
     spriteName: PropTypes.string, // eslint-disable-line react/no-unused-prop-types
-    value: PropTypes.string.isRequired // eslint-disable-line react/no-unused-prop-types
+    value: PropTypes.string.isRequired, // eslint-disable-line react/no-unused-prop-types
+    x: PropTypes.number,
+    y: PropTypes.number
 };
-
-export default Monitor;
+const mapStateToProps = state => ({
+    monitorLayout: state.monitorLayout
+});
+const mapDispatchToProps = dispatch => ({
+    addMonitorRect: (id, rect, savePosition) =>
+        dispatch(addMonitorRect(id, rect.upperStart, rect.lowerEnd, savePosition)),
+    resizeMonitorRect: (id, newWidth, newHeight) => dispatch(resizeMonitorRect(id, newWidth, newHeight)),
+    removeMonitorRect: id => dispatch(removeMonitorRect(id))
+});
+export default connect(
+    mapStateToProps,
+    mapDispatchToProps
+)(Monitor);

src/lib/monitor-adapter.js

@@ -1,45 +1,33 @@
 import OpcodeLabels from './opcode-labels.js';
 
-const PADDING = 5;
-const MONITOR_HEIGHT = 23;
-
 const isUndefined = a => typeof a === 'undefined';
 
 /**
  * Convert monitors from VM format to what the GUI needs to render.
  * - Convert opcode to a label and a category
- * - Add missing XY position data if needed
- * @param {object} block - The monitor block
  * @param {string} block.id - The id of the monitor block
  * @param {string} block.spriteName - Present only if the monitor applies only to the sprite
  *     with given target ID. The name of the target sprite when the monitor was created
- * @param {number} block.index - The index of the monitor
  * @param {string} block.opcode - The opcode of the monitor
  * @param {object} block.params - Extra params to the monitor block
  * @param {string} block.value - The monitor value
- * @param {number} x - The monitor x position
- * @param {number} y - The monitor y position
  * @return {object} The adapted monitor with label and category
  */
-export default function ({id, spriteName, index, opcode, params, value, x, y}) {
+export default function ({id, spriteName, opcode, params, value}) {
     let {label, category, labelFn} = OpcodeLabels(opcode);
 
     // Use labelFn if provided for dynamic labelling (e.g. variables)
     if (!isUndefined(labelFn)) label = labelFn(params);
 
+    // Append sprite name for sprite-specific monitors
     if (spriteName) {
         label = `${spriteName}: ${label}`;
     }
-    // Simple layout if x or y are undefined
-    // @todo scratch2 has a more complex layout behavior we may want to adopt
-    // @todo e.g. this does not work well when monitors have already been moved
-    if (isUndefined(x)) x = PADDING;
-    if (isUndefined(y)) y = PADDING + (index * (PADDING + MONITOR_HEIGHT));
-    
+
     // If value is a number, round it to six decimal places
     if (typeof value === 'number' || (typeof value === 'string' && String(parseFloat(value)) === value)) {
         value = Number(Number(value).toFixed(6));
     }
     
-    return {id, label, category, value, x, y};
+    return {id, label, category, value};
 }

src/reducers/gui.js

@@ -4,6 +4,7 @@ import customProceduresReducer from './custom-procedures';
 import intlReducer from './intl';
 import modalReducer from './modals';
 import monitorReducer from './monitors';
+import monitorLayoutReducer from './monitor-layout';
 import targetReducer from './targets';
 import toolboxReducer from './toolbox';
 import vmReducer from './vm';
@@ -15,6 +16,7 @@ export default combineReducers({
     intl: intlReducer,
     modals: modalReducer,
     monitors: monitorReducer,
+    monitorLayout: monitorLayoutReducer,
     targets: targetReducer,
     toolbox: toolboxReducer,
     vm: vmReducer,

src/reducers/monitor-layout.js

@@ -0,0 +1,318 @@
+import log from '../lib/log';
+
+const ADD_MONITOR_RECT = 'scratch-gui/monitors/ADD_MONITOR_RECT';
+const MOVE_MONITOR_RECT = 'scratch-gui/monitors/MOVE_MONITOR_RECT';
+const RESIZE_MONITOR_RECT = 'scratch-gui/monitors/RESIZE_MONITOR_RECT';
+const REMOVE_MONITOR_RECT = 'scratch-gui/monitors/REMOVE_MONITOR_RECT';
+
+const initialState = {
+    monitors: {},
+    savedMonitorPositions: {}
+};
+
+// Verify that the rectangle formed by the 2 points is well-formed
+const _verifyRect = function (upperStart, lowerEnd) {
+    if (isNaN(upperStart.x) || isNaN(upperStart.y) || isNaN(lowerEnd.x) || isNaN(lowerEnd.y)) {
+        return false;
+    }
+    if (!(upperStart.x < lowerEnd.x)) {
+        return false;
+    }
+    if (!(upperStart.y < lowerEnd.y)) {
+        return false;
+    }
+    return true;
+};
+
+const _addMonitorRect = function (state, action) {
+    if (state.monitors.hasOwnProperty(action.monitorId)) {
+        log.error(`Can't add monitor, monitor with id ${action.monitorId} already exists.`);
+        return state;
+    }
+    if (!_verifyRect(action.upperStart, action.lowerEnd)) {
+        log.error(`Monitor rectangle not formatted correctly`);
+        return state;
+    }
+    return {
+        monitors: Object.assign({}, state.monitors, {
+            [action.monitorId]: {
+                upperStart: action.upperStart,
+                lowerEnd: action.lowerEnd
+            }
+        }),
+        savedMonitorPositions: action.savePosition ?
+            Object.assign({}, state.savedMonitorPositions, {
+                [action.monitorId]: {x: action.upperStart.x, y: action.upperStart.y}
+            }) :
+            state.savedMonitorPositions
+    };
+};
+
+const _moveMonitorRect = function (state, action) {
+    if (!state.monitors.hasOwnProperty(action.monitorId)) {
+        log.error(`Can't move monitor, monitor with id ${action.monitorId} does not exist.`);
+        return state;
+    }
+    if (isNaN(action.newX) || isNaN(action.newY)) {
+        log.error(`Monitor rectangle not formatted correctly`);
+        return state;
+    }
+    
+    const oldMonitor = state.monitors[action.monitorId];
+    if (oldMonitor.upperStart.x === action.newX &&
+            oldMonitor.upperStart.y === action.newY) {
+        // Hasn't moved
+        return state;
+    }
+    const monitorWidth = oldMonitor.lowerEnd.x - oldMonitor.upperStart.x;
+    const monitorHeight = oldMonitor.lowerEnd.y - oldMonitor.upperStart.y;
+    return {
+        monitors: Object.assign({}, state.monitors, {
+            [action.monitorId]: {
+                upperStart: {x: action.newX, y: action.newY},
+                lowerEnd: {x: action.newX + monitorWidth, y: action.newY + monitorHeight}
+            }
+        }),
+        // User generated position is saved
+        savedMonitorPositions: Object.assign({}, state.savedMonitorPositions, {
+            [action.monitorId]: {x: action.newX, y: action.newY}
+        })
+    };
+};
+
+const _resizeMonitorRect = function (state, action) {
+    if (!state.monitors.hasOwnProperty(action.monitorId)) {
+        log.error(`Can't resize monitor, monitor with id ${action.monitorId} does not exist.`);
+        return state;
+    }
+    if (isNaN(action.newWidth) || isNaN(action.newHeight) ||
+            action.newWidth <= 0 || action.newHeight <= 0) {
+        log.error(`Monitor rectangle not formatted correctly`);
+        return state;
+    }
+
+    const oldMonitor = state.monitors[action.monitorId];
+    const newMonitor = {
+        upperStart: oldMonitor.upperStart,
+        lowerEnd: {
+            x: oldMonitor.upperStart.x + action.newWidth,
+            y: oldMonitor.upperStart.y + action.newHeight
+        }
+    };
+    if (newMonitor.lowerEnd.x === oldMonitor.lowerEnd.x &&
+            newMonitor.lowerEnd.y === oldMonitor.lowerEnd.y) {
+        // no change
+        return state;
+    }
+
+    return {
+        monitors: Object.assign({}, state.monitors, {[action.monitorId]: newMonitor}),
+        savedMonitorPositions: state.savedMonitorPositions
+    };
+
+};
+
+const _removeMonitorRect = function (state, action) {
+    if (!state.monitors.hasOwnProperty(action.monitorId)) {
+        log.error(`Can't remove monitor, monitor with id ${action.monitorId} does not exist.`);
+        return state;
+    }
+
+    const newMonitors = Object.assign({}, state.monitors);
+    delete newMonitors[action.monitorId];
+    return {
+        monitors: newMonitors,
+        savedMonitorPositions: state.savedMonitorPositions
+    };
+};
+
+const reducer = function (state, action) {
+    if (typeof state === 'undefined') state = initialState;
+    switch (action.type) {
+    case ADD_MONITOR_RECT:
+        return _addMonitorRect(state, action);
+    case MOVE_MONITOR_RECT:
+        return _moveMonitorRect(state, action);
+    case RESIZE_MONITOR_RECT:
+        return _resizeMonitorRect(state, action);
+    case REMOVE_MONITOR_RECT:
+        return _removeMonitorRect(state, action);
+    default:
+        return state;
+    }
+};
+
+// Init position --------------------------
+const PADDING = 5;
+// @todo fix these numbers when we fix https://github.com/LLK/scratch-gui/issues/980
+const SCREEN_WIDTH = 400;
+const SCREEN_HEIGHT = 300;
+const SCREEN_EDGE_BUFFER = 40;
+
+const _rectsIntersect = function (rect1, rect2) {
+    // If one rectangle is on left side of other
+    if (rect1.upperStart.x >= rect2.lowerEnd.x || rect2.upperStart.x >= rect1.lowerEnd.x) return false;
+    // If one rectangle is above other
+    if (rect1.upperStart.y >= rect2.lowerEnd.y || rect2.upperStart.y >= rect1.lowerEnd.y) return false;
+    return true;
+};
+
+// We need to place a monitor with the given width and height. Return a rect defining where it should be placed.
+const getInitialPosition = function (state, monitorId, eltWidth, eltHeight) {
+    // If this monitor was purposefully moved to a certain position before, put it back in that position
+    if (state.savedMonitorPositions.hasOwnProperty(monitorId)) {
+        const saved = state.savedMonitorPositions[monitorId];
+        return {
+            upperStart: saved,
+            lowerEnd: {x: saved.x + eltWidth, y: saved.y + eltHeight}
+        };
+    }
+
+    // Try all starting positions for the new monitor to find one that doesn't intersect others
+    const endXs = [0];
+    const endYs = [0];
+    let lastX = null;
+    let lastY = null;
+    for (const monitor in state.monitors) {
+        let x = state.monitors[monitor].lowerEnd.x;
+        x = Math.ceil(x / 50) * 50; // Try to choose a sensible "tab width" so more monitors line up
+        endXs.push(x);
+        endYs.push(Math.ceil(state.monitors[monitor].lowerEnd.y));
+    }
+    endXs.sort((a, b) => a - b);
+    endYs.sort((a, b) => a - b);
+    // We'll use plan B if the monitor doesn't fit anywhere (too long or tall)
+    let planB = null;
+    for (const x of endXs) {
+        if (x === lastX) {
+            continue;
+        }
+        lastX = x;
+        outer:
+        for (const y of endYs) {
+            if (y === lastY) {
+                continue;
+            }
+            lastY = y;
+            const monitorRect = {
+                upperStart: {x: x + PADDING, y: y + PADDING},
+                lowerEnd: {x: x + PADDING + eltWidth, y: y + PADDING + eltHeight}
+            };
+            // Intersection testing rect that includes padding
+            const rect = {
+                upperStart: {x, y},
+                lowerEnd: {x: x + eltWidth + (2 * PADDING), y: y + eltHeight + (2 * PADDING)}
+            };
+            for (const monitor in state.monitors) {
+                if (_rectsIntersect(state.monitors[monitor], rect)) {
+                    continue outer;
+                }
+            }
+            // If the rect overlaps the ends of the screen
+            if (rect.lowerEnd.x > SCREEN_WIDTH || rect.lowerEnd.y > SCREEN_HEIGHT) {
+                // If rect is not too close to completely off screen, set it as plan B
+                if (!planB &&
+                        !(rect.upperStart.x + SCREEN_EDGE_BUFFER > SCREEN_WIDTH ||
+                            rect.upperStart.y + SCREEN_EDGE_BUFFER > SCREEN_HEIGHT)) {
+                    planB = monitorRect;
+                }
+                continue;
+            }
+            return monitorRect;
+        }
+    }
+    // If the monitor is too long to fit anywhere, put it in the leftmost spot available
+    // that intersects the right or bottom edge and isn't too close to the edge.
+    if (planB) {
+        return planB;
+    }
+
+    // If plan B fails and there's nowhere reasonable to put it, plan C is to place the monitor randomly
+    const randX = Math.ceil(Math.random() * (SCREEN_WIDTH / 2));
+    const randY = Math.ceil(Math.random() * (SCREEN_HEIGHT - SCREEN_EDGE_BUFFER));
+    return {
+        upperStart: {
+            x: randX,
+            y: randY
+        },
+        lowerEnd: {
+            x: randX + eltWidth,
+            y: randY + eltHeight
+        }
+    };
+};
+
+// Action creators ------------------------
+/**
+ * @param {!string} monitorId Id to add
+ * @param {!object} upperStart upper point defining the rectangle
+ * @param {!number} upperStart.x X of top point that defines the monitor location
+ * @param {!number} upperStart.y Y of top point that defines the monitor location
+ * @param {!object} lowerEnd lower point defining the rectangle
+ * @param {!number} lowerEnd.x X of bottom point that defines the monitor location
+ * @param {!number} lowerEnd.y Y of bottom point that defines the monitor location
+ * @param {?boolean} savePosition True if the placement should be saved when adding the monitor
+ * @returns {object} action to add a new monitor at the location
+ */
+const addMonitorRect = function (monitorId, upperStart, lowerEnd, savePosition) {
+    return {
+        type: ADD_MONITOR_RECT,
+        monitorId: monitorId,
+        upperStart: upperStart,
+        lowerEnd: lowerEnd,
+        savePosition: savePosition
+    };
+};
+
+/**
+ * @param {!string} monitorId Id for monitor to move
+ * @param {!number} newX X of top point that defines the monitor location
+ * @param {!number} newY Y of top point that defines the monitor location
+ * @returns {object} action to move an existing monitor to the location
+ */
+const moveMonitorRect = function (monitorId, newX, newY) {
+    return {
+        type: MOVE_MONITOR_RECT,
+        monitorId: monitorId,
+        newX: newX,
+        newY: newY
+    };
+};
+
+/**
+ * @param {!string} monitorId Id for monitor to resize
+ * @param {!number} newWidth Width to set monitor to
+ * @param {!number} newHeight Height to set monitor to
+ * @returns {object} action to resize an existing monitor to the given dimensions
+ */
+const resizeMonitorRect = function (monitorId, newWidth, newHeight) {
+    return {
+        type: RESIZE_MONITOR_RECT,
+        monitorId: monitorId,
+        newWidth: newWidth,
+        newHeight: newHeight
+    };
+};
+
+/**
+ * @param {!string} monitorId Id for monitor to remove
+ * @returns {object} action to remove an existing monitor
+ */
+const removeMonitorRect = function (monitorId) {
+    return {
+        type: REMOVE_MONITOR_RECT,
+        monitorId: monitorId
+    };
+};
+
+export {
+    reducer as default,
+    addMonitorRect,
+    getInitialPosition,
+    moveMonitorRect,
+    resizeMonitorRect,
+    removeMonitorRect,
+    PADDING,
+    SCREEN_HEIGHT,
+    SCREEN_WIDTH
+};

test/unit/reducers/monitor-layout-reducer.test.js

@@ -0,0 +1,301 @@
+/* eslint-env jest */
+import monitorLayoutReducer from '../../../src/reducers/monitor-layout';
+import {addMonitorRect, moveMonitorRect} from '../../../src/reducers/monitor-layout';
+import {resizeMonitorRect, removeMonitorRect} from '../../../src/reducers/monitor-layout';
+import {getInitialPosition, PADDING, SCREEN_WIDTH, SCREEN_HEIGHT} from '../../../src/reducers/monitor-layout';
+
+test('initialState', () => {
+    let defaultState;
+
+    expect(monitorLayoutReducer(defaultState /* state */, {type: 'anything'} /* action */)).toBeDefined();
+    expect(monitorLayoutReducer(defaultState /* state */, {type: 'anything'} /* action */).monitors).toBeDefined();
+    expect(monitorLayoutReducer(defaultState /* state */, {type: 'anything'} /* action */).savedMonitorPositions)
+        .toBeDefined();
+});
+
+test('addMonitorRect', () => {
+    let defaultState;
+    const monitorId = 1;
+    const monitorId2 = 2;
+    const upperStart = {x: 100, y: 100};
+    const lowerEnd = {x: 200, y: 200};
+
+    // Add a monitor rect
+    const reduxState = monitorLayoutReducer(defaultState, addMonitorRect(monitorId, upperStart, lowerEnd));
+    expect(reduxState.monitors[monitorId]).toBeDefined();
+    expect(reduxState.monitors[monitorId].upperStart).toEqual(upperStart);
+    expect(reduxState.monitors[monitorId].lowerEnd).toEqual(lowerEnd);
+    // Add monitor rect doesn't save position
+    expect(reduxState.savedMonitorPositions[monitorId]).toBeUndefined();
+    const reduxState2 = monitorLayoutReducer(reduxState, moveMonitorRect(monitorId, 0, 0));
+
+    // Add a second monitor rect
+    const reduxState3 = monitorLayoutReducer(reduxState2, addMonitorRect(monitorId2, upperStart, lowerEnd));
+    expect(reduxState3.monitors[monitorId]).toBeDefined();
+    expect(reduxState3.monitors[monitorId2]).toBeDefined();
+    expect(reduxState3.monitors[monitorId2].upperStart).toEqual(upperStart);
+    expect(reduxState3.monitors[monitorId2].lowerEnd).toEqual(lowerEnd);
+    // Saved positions aren't changed by adding monitor
+    expect(reduxState3.savedMonitorPositions).toEqual(reduxState2.savedMonitorPositions);
+});
+
+test('addMonitorRectWithSavedPosition', () => {
+    let defaultState;
+    const monitorId = 1;
+    const upperStart = {x: 100, y: 100};
+    const lowerEnd = {x: 200, y: 200};
+
+    // Add a monitor rect
+    const reduxState = monitorLayoutReducer(defaultState,
+        addMonitorRect(monitorId, upperStart, lowerEnd, true /* savePosition */));
+    expect(reduxState.monitors[monitorId]).toBeDefined();
+    expect(reduxState.monitors[monitorId].upperStart).toEqual(upperStart);
+    expect(reduxState.monitors[monitorId].lowerEnd).toEqual(lowerEnd);
+    // Save position
+    expect(reduxState.savedMonitorPositions[monitorId].x).toEqual(upperStart.x);
+    expect(reduxState.savedMonitorPositions[monitorId].y).toEqual(upperStart.y);
+});
+
+test('invalidRect', () => {
+    let defaultState;
+    const reduxState = monitorLayoutReducer(defaultState /* state */, {type: 'initialize'} /* action */);
+
+    // Problem: x end is before x start
+    expect(
+        monitorLayoutReducer(reduxState,
+            addMonitorRect(1, {x: 100, y: 100}, {x: 10, y: 200})))
+        .toEqual(reduxState);
+
+    // Problem: y end is before y start
+    expect(
+        monitorLayoutReducer(reduxState,
+            addMonitorRect(1, {x: 100, y: 100}, {x: 200, y: 10})))
+        .toEqual(reduxState);
+});
+
+test('invalidAddMonitorRect', () => {
+    let defaultState;
+    const monitorId = 1;
+    const upperStart = {x: 100, y: 100};
+    const lowerEnd = {x: 200, y: 200};
+
+    // Add a monitor rect
+    const reduxState = monitorLayoutReducer(defaultState, addMonitorRect(monitorId, upperStart, lowerEnd));
+    // Try to add the same one
+    expect(monitorLayoutReducer(reduxState, addMonitorRect(monitorId, upperStart, lowerEnd)))
+        .toEqual(reduxState);
+});
+
+test('moveMonitorRect', () => {
+    let defaultState;
+    const monitorId = 1;
+    const monitorId2 = 2;
+    const width = 102;
+    const height = 101;
+    const upperStart = {x: 100, y: 100};
+    const lowerEnd = {x: upperStart.x + width, y: upperStart.y + height};
+    const movedToPosition = {x: 0, y: 0};
+    const movedToPosition2 = {x: 543, y: 2};
+
+    // Add a monitor rect and move it. Expect it to be in monitors state and saved positions.
+    const reduxState = monitorLayoutReducer(defaultState, addMonitorRect(monitorId, upperStart, lowerEnd));
+    const reduxState2 = monitorLayoutReducer(reduxState,
+        moveMonitorRect(monitorId, movedToPosition.x, movedToPosition.y));
+    expect(reduxState2.monitors[monitorId]).toBeDefined();
+    expect(reduxState2.monitors[monitorId].upperStart).toEqual(movedToPosition);
+    expect(reduxState2.monitors[monitorId].lowerEnd.x).toEqual(movedToPosition.x + width);
+    expect(reduxState2.monitors[monitorId].lowerEnd.y).toEqual(movedToPosition.y + height);
+    expect(reduxState2.savedMonitorPositions[monitorId]).toBeDefined();
+    expect(reduxState2.savedMonitorPositions[monitorId].x).toEqual(movedToPosition.x);
+    expect(reduxState2.savedMonitorPositions[monitorId].y).toEqual(movedToPosition.y);
+
+    // Add a second monitor rect and move it. Expect there to now be 2 saved positions.
+    const reduxState3 = monitorLayoutReducer(reduxState2, addMonitorRect(monitorId2, upperStart, lowerEnd));
+    const reduxState4 = monitorLayoutReducer(reduxState3,
+        moveMonitorRect(monitorId2, movedToPosition2.x, movedToPosition2.y));
+    expect(reduxState4.savedMonitorPositions[monitorId]).toEqual(reduxState2.savedMonitorPositions[monitorId]);
+    expect(reduxState4.savedMonitorPositions[monitorId2].x).toEqual(movedToPosition2.x);
+    expect(reduxState4.savedMonitorPositions[monitorId2].y).toEqual(movedToPosition2.y);
+});
+
+test('invalidMoveMonitorRect', () => {
+    let defaultState;
+    let reduxState = monitorLayoutReducer(defaultState, {type: 'initialize'} /* action */);
+    const monitorId = 1;
+
+    // Try to move a monitor rect that doesn't exist
+    expect(monitorLayoutReducer(reduxState, moveMonitorRect(monitorId, 1 /* newX */, 1 /* newY */)))
+        .toEqual(reduxState);
+
+    // Add the monitor to move
+    reduxState = monitorLayoutReducer(reduxState, addMonitorRect(monitorId, {x: 100, y: 100}, {x: 200, y: 200}));
+
+    // Invalid newX
+    expect(monitorLayoutReducer(reduxState, moveMonitorRect(monitorId, 'Oregon' /* newX */, 1 /* newY */)))
+        .toEqual(reduxState);
+
+    // Invalid newY
+    expect(monitorLayoutReducer(reduxState, moveMonitorRect(monitorId, 1 /* newX */)))
+        .toEqual(reduxState);
+});
+
+test('resizeMonitorRect', () => {
+    let defaultState;
+    const monitorId = 1;
+    const upperStart = {x: 100, y: 100};
+    const newWidth = 10;
+    const newHeight = 20;
+
+    // Add a monitor rect and resize it
+    const reduxState = monitorLayoutReducer(defaultState, addMonitorRect(monitorId, upperStart, {x: 200, y: 200}));
+    const reduxState2 = monitorLayoutReducer(reduxState,
+        resizeMonitorRect(monitorId, newWidth, newHeight));
+    expect(reduxState2.monitors[monitorId]).toBeDefined();
+    expect(reduxState2.monitors[monitorId].upperStart).toEqual(upperStart);
+    expect(reduxState2.monitors[monitorId].lowerEnd.x).toEqual(upperStart.x + newWidth);
+    expect(reduxState2.monitors[monitorId].lowerEnd.y).toEqual(upperStart.y + newHeight);
+    // Saved positions aren't changed by resizing monitor
+    expect(reduxState2.savedMonitorPositions).toEqual(reduxState.savedMonitorPositions);
+});
+
+test('invalidResizeMonitorRect', () => {
+    let defaultState;
+    let reduxState = monitorLayoutReducer(defaultState, {type: 'initialize'} /* action */);
+    const monitorId = 1;
+
+    // Try to resize a monitor rect that doesn't exist
+    expect(monitorLayoutReducer(reduxState, resizeMonitorRect(monitorId, 1 /* newWidth */, 1 /* newHeight */)))
+        .toEqual(reduxState);
+
+    // Add the monitor to resize
+    reduxState = monitorLayoutReducer(reduxState, addMonitorRect(monitorId, {x: 100, y: 100}, {x: 200, y: 200}));
+
+    // Invalid newWidth
+    expect(monitorLayoutReducer(reduxState, resizeMonitorRect(monitorId, 'Oregon' /* newWidth */, 1 /* newHeight */)))
+        .toEqual(reduxState);
+
+    // Invalid newHeight
+    expect(monitorLayoutReducer(reduxState, moveMonitorRect(monitorId, 1 /* newWidth */)))
+        .toEqual(reduxState);
+
+    // newWidth < 0
+    expect(monitorLayoutReducer(reduxState, resizeMonitorRect(monitorId, -1 /* newWidth */, 1 /* newHeight */)))
+        .toEqual(reduxState);
+
+    // newHeight < 0
+    expect(monitorLayoutReducer(reduxState, resizeMonitorRect(monitorId, 1 /* newWidth */, -1 /* newHeight */)))
+        .toEqual(reduxState);
+});
+
+test('removeMonitorRect', () => {
+    let defaultState;
+    const monitorId = 1;
+
+    // Add a monitor rect, move it, and remove it
+    const reduxState = monitorLayoutReducer(defaultState, addMonitorRect(monitorId,
+        {x: 100, y: 100},
+        {x: 200, y: 200}
+    ));
+    const reduxState2 = monitorLayoutReducer(reduxState, moveMonitorRect(monitorId, 0, 0));
+    const reduxState3 = monitorLayoutReducer(reduxState2, removeMonitorRect(monitorId));
+    expect(reduxState3.monitors[monitorId]).toBeUndefined();
+    // Check that saved positions aren't changed by removing monitor
+    expect(reduxState3.savedMonitorPositions).toEqual(reduxState2.savedMonitorPositions);
+});
+
+test('invalidRemoveMonitorRect', () => {
+    let defaultState;
+    const reduxState = monitorLayoutReducer(defaultState, {type: 'initialize'} /* action */);
+
+    // Try to remove a monitor rect that doesn't exist
+    expect(monitorLayoutReducer(reduxState, resizeMonitorRect(1)))
+        .toEqual(reduxState);
+});
+
+test('getInitialPosition_lineUpTopLeft', () => {
+    let defaultState;
+    const width = 100;
+    const height = 200;
+    // Add monitors to right and bottom, but there is a space in the top left
+    let reduxState = monitorLayoutReducer(defaultState, addMonitorRect(1,
+        {x: width + PADDING, y: 0},
+        {x: 100, y: height}
+    ));
+    reduxState = monitorLayoutReducer(defaultState, addMonitorRect(2,
+        {x: 0, y: height + PADDING},
+        {x: width, y: 100}
+    ));
+
+    // Check that the added monitor appears in the space
+    const rect = getInitialPosition(reduxState, 3, width, height);
+    expect(rect.upperStart).toBeDefined();
+    expect(rect.lowerEnd).toBeDefined();
+    expect(rect.lowerEnd.x - rect.upperStart.x).toEqual(width);
+    expect(rect.lowerEnd.y - rect.upperStart.y).toEqual(height);
+    expect(rect.upperStart.x).toEqual(PADDING);
+    expect(rect.upperStart.y).toEqual(PADDING);
+});
+
+test('getInitialPosition_savedPosition', () => {
+    const monitorId = 1;
+    const savedX = 100;
+    const savedY = 200;
+    const width = 7;
+    const height = 8;
+    const reduxState = {
+        monitors: {},
+        savedMonitorPositions: {[monitorId]: {x: savedX, y: savedY}}
+    };
+
+    // Check that initial position uses saved state
+    const rect = getInitialPosition(reduxState, monitorId, width, height);
+    expect(rect.upperStart).toBeDefined();
+    expect(rect.lowerEnd).toBeDefined();
+    expect(rect.lowerEnd.x - rect.upperStart.x).toEqual(width);
+    expect(rect.lowerEnd.y - rect.upperStart.y).toEqual(height);
+    expect(rect.upperStart.x).toEqual(savedX);
+    expect(rect.upperStart.y).toEqual(savedY);
+});
+
+test('getInitialPosition_lineUpLeft', () => {
+    let defaultState;
+    const monitor1EndY = 60;
+    // Add a monitor that takes up the upper left corner
+    const reduxState = monitorLayoutReducer(defaultState, addMonitorRect(1, {x: 0, y: 0}, {x: 100, y: monitor1EndY}));
+
+    // Check that added monitor is under it and lines up left
+    const rect = getInitialPosition(reduxState, 2, 20 /* width */, 20 /* height */);
+    expect(rect.upperStart.y >= monitor1EndY + PADDING).toBeTruthy();
+});
+
+test('getInitialPosition_lineUpTop', () => {
+    let defaultState;
+    const monitor1EndX = 100;
+    // Add a monitor that takes up the whole left side
+    const reduxState = monitorLayoutReducer(defaultState, addMonitorRect(1,
+        {x: 0, y: 0},
+        {x: monitor1EndX, y: SCREEN_HEIGHT}
+    ));
+
+    // Check that added monitor is to the right of it and lines up top
+    const rect = getInitialPosition(reduxState, 2, 20 /* width */, 20 /* height */);
+    expect(rect.upperStart.y).toEqual(PADDING);
+    expect(rect.upperStart.x >= monitor1EndX + PADDING).toBeTruthy();
+});
+
+test('getInitialPosition_noRoom', () => {
+    let defaultState;
+    const width = 7;
+    const height = 8;
+    // Add a monitor that takes up the whole screen
+    const reduxState = monitorLayoutReducer(defaultState, addMonitorRect(1,
+        {x: 0, y: 0},
+        {x: SCREEN_WIDTH, y: SCREEN_HEIGHT}
+    ));
+
+    // Check that added monitor exists somewhere (we don't care where)
+    const rect = getInitialPosition(reduxState, 2, width, height);
+    expect(rect.upperStart).toBeDefined();
+    expect(rect.lowerEnd.x - rect.upperStart.x).toEqual(width);
+    expect(rect.lowerEnd.y - rect.upperStart.y).toEqual(height);
+});