Merge pull request #452 from sbates-idrc/remember-program-speed

Store the program speed in a new programSpeed state variable (fixes #451)

Author: sbates-idrc

src/App.js

@@ -119,6 +119,8 @@ export type AppState = {
     sceneDimensions: SceneDimensions,
     drawingEnabled: boolean,
     runningState: RunningState,
+    // programSpeed is a number in the range 1..(speedLookUp.length)
+    programSpeed: number,
     disallowedActions: ActionToggleRegister,
     keyBindingsEnabled: boolean,
     keyboardInputSchemeName: KeyboardInputSchemeName,
@@ -154,7 +156,6 @@ export class App extends React.Component<AppProps, AppState> {
     customBackgroundSerializer: CustomBackgroundSerializer;
     speedLookUp: Array<number>;
     pushStateTimeoutID: ?TimeoutID;
-    speedControlRef: { current: null | HTMLElement };
     programBlockEditorRef: { current: any };
     sequenceInProgress: Array<KeyboardEvent>;
     announcementBuilder: AnnouncementBuilder;
@@ -216,6 +217,8 @@ export class App extends React.Component<AppProps, AppState> {
             sceneDimensions: this.sceneDimensions,
             drawingEnabled: true,
             runningState: 'stopped',
+            // Default to the middle speed
+            programSpeed: Math.ceil(this.speedLookUp.length / 2),
             disallowedActions: {},
             keyBindingsEnabled: false,
             showKeyboardModal: false,
@@ -262,12 +265,16 @@ export class App extends React.Component<AppProps, AppState> {
         this.programChangeController = new ProgramChangeController(this,
             this.props.intl, this.audioManager);
 
-        this.speedControlRef = React.createRef();
         this.programBlockEditorRef = React.createRef();
 
         const actionsHandler = new ActionsHandler(this, this.audioManager,
             this.sceneDimensions, this.props.intl);
-        this.interpreter = new Interpreter(1000, this, actionsHandler);
+
+        this.interpreter = new Interpreter(
+            this.speedLookUp[this.state.programSpeed - 1],
+            this,
+            actionsHandler
+        );
     }
 
     setStateSettings(settings: $Shape<AppSettings>) {
@@ -660,10 +667,24 @@ export class App extends React.Component<AppProps, AppState> {
                             }
                             break;
                         case("decreaseProgramSpeed"):
-                            this.changeProgramSpeedIndex(this.speedLookUp.indexOf(this.interpreter.stepTimeMs) - 1);
+                            this.setState((state) => {
+                                return {
+                                    programSpeed: Math.max(
+                                        1,
+                                        state.programSpeed - 1
+                                    )
+                                };
+                            });
                             break;
                         case("increaseProgramSpeed"):
-                            this.changeProgramSpeedIndex(this.speedLookUp.indexOf(this.interpreter.stepTimeMs) + 1);
+                            this.setState((state) => {
+                                return {
+                                    programSpeed: Math.min(
+                                        this.speedLookUp.length,
+                                        state.programSpeed + 1
+                                    )
+                                };
+                            });
                             break;
                         case("selectForward1"):
                             this.setState({ "selectedAction": "forward1" });
@@ -964,20 +985,14 @@ export class App extends React.Component<AppProps, AppState> {
         });
     }
 
-    changeProgramSpeedIndex = (newSpeedIndex: number) => {
-        if (newSpeedIndex >= 0 && newSpeedIndex <= (this.speedLookUp.length - 1)) {
-            this.interpreter.setStepTime(this.speedLookUp[newSpeedIndex]);
-            if (this.speedControlRef.current) {
-                // $FlowFixMe: Flow doesn't believe that we have sufficiently ensured that current !== null.
-                this.speedControlRef.current.value = (newSpeedIndex + 1).toString();
-            }
+    handleChangeProgramSpeed = (value: number) => {
+        if (value >= 1 && value <= this.speedLookUp.length) {
+            this.setState({
+                programSpeed: value
+            });
         }
     }
 
-    handleChangeProgramSpeed = (stepTimeMs: number) => {
-        this.interpreter.setStepTime(stepTimeMs);
-    }
-
     renderCommandBlocks = (commands: Array<DisplayedCommandName>) => {
         const commandBlocks = [];
 
@@ -1570,8 +1585,8 @@ export class App extends React.Component<AppProps, AppState> {
                             onClick={this.handleStop}
                         />
                         <ProgramSpeedController
-                            rangeControlRef={this.speedControlRef}
-                            values={this.speedLookUp}
+                            numValues={this.speedLookUp.length}
+                            value={this.state.programSpeed}
                             onChange={this.handleChangeProgramSpeed}
                         />
                     </div>
@@ -1993,6 +2008,16 @@ export class App extends React.Component<AppProps, AppState> {
             document.body.className = `${this.state.settings.theme}-theme`;
         }
 
+        if (this.state.programSpeed !== prevState.programSpeed) {
+            if (this.speedLookUp.length > 0
+                    && this.state.programSpeed >= 1
+                    && this.state.programSpeed <= this.speedLookUp.length) {
+                this.interpreter.setStepTime(
+                    this.speedLookUp[this.state.programSpeed - 1]
+                );
+            }
+        }
+
         /* Dash connection removed for version 0.5
         if (this.state.dashConnectionStatus !== prevState.dashConnectionStatus) {
             console.log(this.state.dashConnectionStatus);

src/ProgramSpeedController.js

@@ -10,15 +10,16 @@ import './ProgramSpeedController.scss';
 
 type ProgramSpeedControllerProps = {
     intl: IntlShape,
-    values: Array<number>,
-    rangeControlRef: { current: null | HTMLElement };
+    numValues: number,
+    value: number,
     onChange: (value: number) => void
 };
 
 class ProgramSpeedController extends React.Component<ProgramSpeedControllerProps, {}> {
-    onChangeInput = (e: SyntheticInputEvent<HTMLInputElement>) => {
-        this.props.onChange(this.props.values[parseInt(e.target.value) - 1]);
+    handleChange = (e: SyntheticInputEvent<HTMLInputElement>) => {
+        this.props.onChange(parseInt(e.target.value));
     }
+
     render() {
         return (
             <div className='ProgramSpeedController__container'>
@@ -31,10 +32,11 @@ class ProgramSpeedController extends React.Component<ProgramSpeedControllerProps
                         aria-label={`${this.props.intl.formatMessage({id:'ProgramSpeedController.slider'})}`}
                         className='ProgramSpeedController__slider'
                         type='range'
-                        min='1'
-                        max={this.props.values.length}
-                        ref={this.props.rangeControlRef}
-                        onChange={this.onChangeInput} />
+                        min={1}
+                        max={this.props.numValues}
+                        value={this.props.value}
+                        onChange={this.handleChange}
+                    />
                 }
                 <FastIcon aria-hidden={true} />
             </div>

src/ProgramSpeedController.test.js

@@ -7,21 +7,19 @@ import { IntlProvider } from 'react-intl';
 import ProgramSpeedController from './ProgramSpeedController';
 import messages from './messages.json';
 
-configure({ adapter: new Adapter()});
+configure({ adapter: new Adapter() });
 
-const defaultSpeedControllerProps = {
-    rangeControlRef: React.createRef()
-};
+function createMountProgramSpeedController(numValues: number, value: number) {
+    const changeHandler = jest.fn();
 
-function createMountProgramSpeedController(props) {
     const wrapper = mount(
         React.createElement(
             ProgramSpeedController,
-            Object.assign(
-                {},
-                defaultSpeedControllerProps,
-                props
-            )
+            {
+                numValues: numValues,
+                value: value,
+                onChange: changeHandler
+            }
         ),
         {
             wrappingComponent: IntlProvider,
@@ -33,39 +31,29 @@ function createMountProgramSpeedController(props) {
         }
     );
 
-    return {
-        wrapper
-    };
+    return { wrapper, changeHandler };
 }
 
 function getProgramSpeedController(programSpeedControllerWrapper) {
     return programSpeedControllerWrapper.find('.ProgramSpeedController__slider').at(0);
 }
 
-describe('When value from the controller changes', () => {
-    test('Should call onChange callback with value from speedLookUp at index value-1', () => {
-        expect.assertions(2);
-        const mockOnChange = jest.fn();
-        const values = [2000, 1500, 1000, 500, 250];
-        const { wrapper } = createMountProgramSpeedController({
-            onChange: mockOnChange,
-            values
-        });
-        const programSpeedController = getProgramSpeedController(wrapper);
-        const targetValue = 4;
-        programSpeedController.simulate('change', { target: { value: targetValue}});
-        expect(mockOnChange.mock.calls.length).toBe(1);
-        expect(mockOnChange.mock.calls[0][0]).toBe(values[targetValue-1]);
-    })
+test('Renders with the specified numValues and value', () => {
+    expect.assertions(3);
+    const { wrapper } = createMountProgramSpeedController(5, 3);
+    const programSpeedController = getProgramSpeedController(wrapper);
+    expect(programSpeedController.get(0).props.min).toBe(1);
+    expect(programSpeedController.get(0).props.max).toBe(5);
+    expect(programSpeedController.get(0).props.value).toBe(3);
 });
 
-test('Maximum value of the controller should be equal to the length of values property', () => {
-    expect.assertions(1);
-    const values = [2000, 1500, 1000, 500, 250];
-    const { wrapper } = createMountProgramSpeedController({
-        onChange: () => {},
-        values
-    });
+test('When the controller changes, should call the onChange callback with the new value', () => {
+    expect.assertions(2);
+    const { wrapper, changeHandler } = createMountProgramSpeedController(5, 3);
+
     const programSpeedController = getProgramSpeedController(wrapper);
-    expect(programSpeedController.get(0).props.max).toBe(values.length);
-})
+    programSpeedController.simulate('change', { target: { value: 4 } });
+
+    expect(changeHandler.mock.calls.length).toBe(1);
+    expect(changeHandler.mock.calls[0][0]).toBe(4);
+});