[Timer] Updated Timer UI and fixed tests

Added peeking stop button to view, added legacy and first run support, added new and fixed old tests

platform/features/clock/res/templates/timer.html

@@ -23,7 +23,11 @@
 	<div class="l-elem-wrapper l-flex-row">
 		<a ng-click="timer.clickButton()"
 		   title="{{timer.buttonText()}}"
-			class="flex-elem s-icon-button {{timer.buttonCssClass()}}"></a>
+		   class="flex-elem control s-icon-button {{timer.buttonCssClass()}}"></a>
+		<a ng-click="timer.clickStopButton()"
+			title="{{timer.stopButtonText()}}"
+			class="{{!timer.stopButtonCssClass() || 'flex-elem control s-icon-button'}} {{timer.stopButtonCssClass()}}"></a>
+	    <span class="flex-elem l-value {{timer.stateClass()}}"></span>
 	    <span class="flex-elem l-value {{timer.signClass()}}">
 	        <span class="value"
 		        ng-class="{ active:timer.text() }">{{timer.text() || "--:--:--"}}

platform/features/clock/src/actions/AbstractTimerAction.js

@@ -30,9 +30,6 @@ define(
          * Sets the reference timestamp in a timer to the current
          * time, such that it begins counting up.
          *
-         * Both "Start" and "Restart" share this implementation, but
-         * control their visibility with different `appliesTo` behavior.
-         *
          * @implements {Action}
          * @memberof platform/features/clock
          * @constructor
@@ -60,7 +57,7 @@ define(
             }
 
             function setTimerState(model) {
-                model.timerState = 'play';
+                model.timerState = 'started';
             }
 
             function setPausedTime(model) {

platform/features/clock/src/actions/PauseTimerAction.js

@@ -54,8 +54,7 @@ define(
             // We show this variant for timers which have
             // a target time, or is in a playing state.
             return model.type === 'timer' &&
-                (model.timestamp !== undefined ||
+                    model.timerState === 'started';
-                    model.timerState === 'play');
         };
 
         PauseTimerAction.prototype.perform = function () {
@@ -63,7 +62,7 @@ define(
                 now = this.now;
 
             function setTimerState(model) {
-                model.timerState = 'pause';
+                model.timerState = 'paused';
             }
 
             function setPausedTime(model) {

platform/features/clock/src/actions/RestartTimerAction.js

@@ -50,14 +50,32 @@ define(
                 (context.domainObject && context.domainObject.getModel()) ||
                 {};
 
-            // We show this variant for timers which already have
+            // We show this variant for timers which already have a target time.
-            // a target time.
             return model.type === 'timer' &&
-                (model.timestamp !== undefined ||
+                model.timerState !== 'stopped';
-                    model.timerState !== undefined);
+        };
+
+        RestartTimerAction.prototype.perform = function () {
+            var domainObject = this.domainObject,
+                now = this.now;
+
+            function setTimestamp(model) {
+                model.timestamp = now();
+            }
+
+            function setTimerState(model) {
+                model.timerState = 'started';
+            }
+
+            function setPausedTime(model) {
+                model.pausedTime = undefined;
+            }
+
+            return domainObject.useCapability('mutation', setTimestamp) &&
+                domainObject.useCapability('mutation', setTimerState) &&
+                domainObject.useCapability('mutation', setPausedTime);
         };
 
         return RestartTimerAction;
-
     }
 );

platform/features/clock/src/actions/StartTimerAction.js

@@ -53,8 +53,7 @@ define(
             // We show this variant for timers which do not yet have
             // a target time.
             return model.type === 'timer' &&
-                (model.timestamp === undefined ||
+                    model.timerState !== 'started';
-                    model.timerState !== 'play');
         };
 
         return StartTimerAction;

platform/features/clock/src/actions/StopTimerAction.js

@@ -54,8 +54,7 @@ define(
             // We show this variant for timers which do not yet have
             // a target time.
             return model.type === 'timer' &&
-                (model.timestamp !== undefined ||
+                    model.timerState !== 'stopped';
-                    model.timerState !== undefined);
         };
 
         StopTimerAction.prototype.perform = function () {
@@ -66,7 +65,7 @@ define(
             }
 
             function setTimerState(model) {
-                model.timerState = undefined;
+                model.timerState = 'stopped';
             }
 
             function setPausedTime(model) {

platform/features/clock/src/controllers/TimerController.js

@@ -54,10 +54,13 @@ define(
                         timeDelta >= 1000 ? "+" : "";
                     self.signCssClass = timeDelta < 0 ? "icon-minus" :
                         timeDelta >= 1000 ? "icon-plus" : "";
+                    self.stateCssClass = relativeTimerState === "play" ? "icon-play" :
+                        relativeTimerState === "pause" ? "icon-pause" : "icon-box";
                 } else {
                     self.textValue = "";
                     self.signValue = "";
                     self.signCssClass = "";
+                    self.stateCssClass = "icon-box";
                 }
             }
 
@@ -73,34 +76,47 @@ define(
                 relativeTimerState = timerState;
             }
 
+            function updateActions(actionCapability, actionKey) {
+                self.relevantAction = actionCapability &&
+                    actionCapability.getActions(actionKey)[0];
+
+                self.stopAction = relativeTimerState !== 'stopped' ?
+                    actionCapability && actionCapability.getActions('timer.stop')[0] : undefined;
+
+            }
+
             function isPaused() {
-                return relativeTimerState === 'pause';
+                return relativeTimerState === 'paused';
+            }
+
+            function handleLegacyTimer(model) {
+                if (model.timerState === undefined) {
+                    model.timerState = model.timestamp === undefined ?
+                        'stopped' : 'started';
+                }
             }
 
             function updateObject(domainObject) {
-                var model = domainObject.getModel(),
+                var model = domainObject.getModel();
-                    timestamp = model.timestamp,
+                handleLegacyTimer(model);
+
+                var timestamp = model.timestamp,
                     formatKey = model.timerFormat,
                     timerState = model.timerState,
                     actionCapability = domainObject.getCapability('action'),
-                    actionKey = (timerState !== 'play') ?
+                    actionKey = (timerState !== 'started') ?
                         'timer.start' : 'timer.pause';
 
-                self.timerState = model.timerState;
-                self.pausedTime = model.pausedTime;
-
                 updateFormat(formatKey);
                 updateTimestamp(timestamp);
                 updateTimerState(timerState);
+                updateActions(actionCapability, actionKey);
 
                 //if paused on startup show last known position
                 if (isPaused() && !lastTimestamp) {
-                    lastTimestamp = self.pausedTime;
+                    lastTimestamp = model.pausedTime;
                 }
 
-                self.relevantAction = actionCapability &&
-                    actionCapability.getActions(actionKey)[0];
-
                 update();
             }
 
@@ -122,6 +138,11 @@ define(
                     lastTimestamp = now();
                     update();
                 }
+
+                if (relativeTimerState === undefined) {
+                    handleModification();
+                }
+
                 // We're running in an animation frame, not in a digest cycle.
                 // We need to trigger a digest cycle if our displayable data
                 // changes.
@@ -181,6 +202,36 @@ define(
             }
         };
 
+        /**
+         * Get the CSS class to display the stop button
+         * @returns {string} cssclass to display
+         */
+        TimerController.prototype.stopButtonCssClass = function () {
+            return this.stopAction ?
+                this.stopAction.getMetadata().cssclass : '';
+        };
+
+        /**
+         * Get the text to show the stop button
+         * (e.g. in a tooltip)
+         * @returns {string} name of the action
+         */
+        TimerController.prototype.stopButtonText = function () {
+            return this.stopAction ?
+                this.stopAction.getMetadata().name : '';
+        };
+
+
+        /**
+         * Perform the action associated with the stop button.
+         */
+        TimerController.prototype.clickStopButton = function () {
+            if (this.stopAction) {
+                this.stopAction.perform();
+                this.updateObject(this.$scope.domainObject);
+            }
+        };
+
         /**
          * Get the sign (+ or -) of the current timer value, as
          * displayable text.
@@ -199,6 +250,15 @@ define(
             return this.signCssClass;
         };
 
+        /**
+         * Get the symbol (play, pause or stop) of the current timer state, as
+         * a CSS class.
+         * @returns {string} symbol of the current timer state
+         */
+        TimerController.prototype.stateClass = function () {
+            return this.stateCssClass;
+        };
+
         /**
          * Get the text to display for the current timer value.
          * @returns {string} current timer value

platform/features/clock/test/actions/PauseTimerActionSpec.js

@@ -62,27 +62,34 @@ define(
                 action = new PauseTimerAction(mockNow, testContext);
             });
 
-            it("updates the model with a timestamp", function () {
+            it("updates the model with a timerState", function () {
+                testModel.timerState = 'started';
+                action.perform();
+                expect(testModel.timerState).toEqual('paused');
+            });
+
+            it("updates the model with a pausedTime", function () {
+                testModel.pausedTime = undefined;
                 mockNow.andReturn(12000);
                 action.perform();
-                expect(testModel.timestamp).toEqual(12000);
+                expect(testModel.pausedTime).toEqual(12000);
             });
 
             it("applies only to timers in a playing state", function () {
                 //in a stopped state
-                testStates(testModel, 'timer', undefined, undefined, false);
+                testState('timer', 'stopped', undefined, false);
 
                 //in a paused state
-                testStates(testModel, 'timer', 'pause', undefined, false);
+                testState('timer', 'paused', 12000, false);
 
                 //in a playing state
-                testStates(testModel, 'timer', 'play', undefined, true);
+                testState('timer', 'started', 12000, true);
 
                 //not a timer
-                testStates(testModel, 'clock', 'pause', undefined, false);
+                testState('clock', 'started', 12000, false);
             });
 
-            function testStates(testModel, type, timerState, timestamp, expected) {
+            function testState(type, timerState, timestamp, expected) {
                 testModel.type = type;
                 testModel.timerState = timerState;
                 testModel.timestamp = timestamp;
@@ -92,11 +99,6 @@ define(
                 } else {
                     expect(PauseTimerAction.appliesTo(testContext)).toBeFalsy()
                 }
-
-                //first test without time, this test with time
-                if (timestamp === undefined) {
-                    testStates(testModel, type, timerState, 12000, expected);
-                }
             }
         });
     }

platform/features/clock/test/actions/RestartTimerActionSpec.js

@@ -63,26 +63,39 @@ define(
             });
 
             it("updates the model with a timestamp", function () {
+                testModel.pausedTime = 12000;
                 mockNow.andReturn(12000);
                 action.perform();
                 expect(testModel.timestamp).toEqual(12000);
             });
 
-            it("applies only to timers in a non-stopped state", function () {
+            it("updates the model with a pausedTime", function () {
-                //in a stopped state
+                testModel.pausedTime = 12000;
-                testStates(testModel, 'timer', undefined, undefined, false);
+                action.perform();
-
+                expect(testModel.pausedTime).toEqual(undefined);
-                //in a paused state
-                testStates(testModel, 'timer', 'pause', undefined, true);
-
-                //in a playing state
-                testStates(testModel, 'timer', 'play', undefined, true);
-
-                //not a timer
-                testStates(testModel, 'clock', 'pause', undefined, false);
             });
 
-            function testStates(testModel, type, timerState, timestamp, expected) {
+            it("updates the model with a timerState", function () {
+                testModel.timerState = 'stopped';
+                action.perform();
+                expect(testModel.timerState).toEqual('started');
+            });
+
+            it("applies only to timers in a non-stopped state", function () {
+                //in a stopped state
+                testState('timer', 'stopped', undefined, false);
+
+                //in a paused state
+                testState('timer', 'paused', 12000, true);
+
+                //in a playing state
+                testState('timer', 'started', 12000, true);
+
+                //not a timer
+                testState('clock', 'paused', 12000, false);
+            });
+
+            function testState(type, timerState, timestamp, expected) {
                 testModel.type = type;
                 testModel.timerState = timerState;
                 testModel.timestamp = timestamp;
@@ -92,11 +105,6 @@ define(
                 } else {
                     expect(RestartTimerAction.appliesTo(testContext)).toBeFalsy()
                 }
-
-                //first test without time, this test with time
-                if (timestamp === undefined) {
-                    testStates(testModel, type, timerState, 12000, expected);
-                }
             }
         });
     }

platform/features/clock/test/actions/StartTimerActionSpec.js

@@ -68,21 +68,33 @@ define(
                 expect(testModel.timestamp).toEqual(12000);
             });
 
-            it("applies only to timers not in a playing state", function () {
+            it("updates the model with a pausedTime", function () {
-                //in a stopped state
+                testModel.pausedTime = 12000;
-                testStates(testModel, 'timer', undefined, undefined, true);
+                action.perform();
-
+                expect(testModel.pausedTime).toEqual(undefined);
-                //in a paused state
-                testStates(testModel, 'timer', 'pause', undefined, true);
-
-                //in a playing state
-                testStates(testModel, 'timer', 'play', undefined, false);
-
-                //not a timer
-                testStates(testModel, 'clock', 'pause', undefined, false);
             });
 
-            function testStates(testModel, type, timerState, timestamp, expected) {
+            it("updates the model with a timerState", function () {
+                testModel.timerState = undefined;
+                action.perform();
+                expect(testModel.timerState).toEqual('started');
+            });
+
+            it("applies only to timers not in a playing state", function () {
+                //in a stopped state
+                testState('timer', 'stopped', undefined, true);
+
+                //in a paused state
+                testState('timer', 'paused', 12000, true);
+
+                //in a playing state
+                testState('timer', 'started', 12000, false);
+
+                //not a timer
+                testState('clock', 'paused', 12000, false);
+            });
+
+            function testState(type, timerState, timestamp, expected) {
                 testModel.type = type;
                 testModel.timerState = timerState;
                 testModel.timestamp = timestamp;
@@ -92,11 +104,6 @@ define(
                 } else {
                     expect(StartTimerAction.appliesTo(testContext)).toBeFalsy()
                 }
-
-                //first test without time, this test with time
-                if (timestamp === undefined) {
-                    testStates(testModel, type, timerState, 12000, expected);
-                }
             }
         });
     }

platform/features/clock/test/actions/StopTimerActionSpec.js

@@ -65,24 +65,36 @@ define(
             it("updates the model with a timestamp", function () {
                 mockNow.andReturn(12000);
                 action.perform();
-                expect(testModel.timestamp).toEqual(12000);
+                expect(testModel.timestamp).toEqual(undefined);
+            });
+
+            it("updates the model with a pausedTime", function () {
+                testModel.pausedTime = 12000;
+                action.perform();
+                expect(testModel.pausedTime).toEqual(undefined);
+            });
+
+            it("updates the model with a timerState", function () {
+                testModel.timerState = 'started';
+                action.perform();
+                expect(testModel.timerState).toEqual('stopped');
             });
 
             it("applies only to timers in a non-stopped state", function () {
                 //in a stopped state
-                testStates(testModel, 'timer', undefined, undefined, false);
+                testState('timer', 'stopped', undefined, false);
 
                 //in a paused state
-                testStates(testModel, 'timer', 'pause', undefined, true);
+                testState('timer', 'paused', 12000, true);
 
                 //in a playing state
-                testStates(testModel, 'timer', 'play', undefined, true);
+                testState('timer', 'started', 12000, true);
 
                 //not a timer
-                testStates(testModel, 'clock', 'pause', undefined, false);
+                testState('clock', 'paused', 12000, false);
             });
 
-            function testStates(testModel, type, timerState, timestamp, expected) {
+            function testState(type, timerState, timestamp, expected) {
                 testModel.type = type;
                 testModel.timerState = timerState;
                 testModel.timestamp = timestamp;
@@ -92,11 +104,6 @@ define(
                 } else {
                     expect(StopTimerAction.appliesTo(testContext)).toBeFalsy()
                 }
-
-                //first test without time, this test with time
-                if (timestamp === undefined) {
-                    testStates(testModel, type, timerState, 12000, expected);
-                }
             }
         });
     }

platform/features/clock/test/controllers/TimerControllerSpec.js

@@ -35,6 +35,7 @@ define(
                 mockActionCapability,
                 mockStart,
                 mockPause,
+                mockStop,
                 testModel,
                 controller;
 
@@ -68,7 +69,11 @@ define(
                     ['getMetadata', 'perform']
                 );
                 mockPause = jasmine.createSpyObj(
-                    'pause',
+                    'paused',
+                    ['getMetadata', 'perform']
+                );
+                mockStop = jasmine.createSpyObj(
+                    'stopped',
                     ['getMetadata', 'perform']
                 );
                 mockNow = jasmine.createSpy('now');
@@ -82,11 +87,13 @@ define(
                 mockActionCapability.getActions.andCallFake(function (k) {
                     return [{
                         'timer.start': mockStart,
-                        'timer.pause': mockPause
+                        'timer.pause': mockPause,
+                        'timer.stop': mockStop
                     }[k]];
                 });
                 mockStart.getMetadata.andReturn({cssclass: "icon-play", name: "Start"});
                 mockPause.getMetadata.andReturn({cssclass: "icon-pause", name: "Pause"});
+                mockStop.getMetadata.andReturn({cssclass: "icon-box", name: "Stop"});
                 mockScope.domainObject = mockDomainObject;
 
                 testModel = {};
@@ -120,6 +127,7 @@ define(
                 mockWindow.requestAnimationFrame.mostRecentCall.args[0]();
                 expect(controller.sign()).toEqual("");
                 expect(controller.text()).toEqual("");
+                expect(controller.stopButtonText()).toEqual("");
             });
 
             it("formats time to display relative to target", function () {
@@ -150,22 +158,43 @@ define(
                 expect(controller.buttonText()).toEqual("Start");
 
                 testModel.timestamp = 12321;
+                testModel.timerState = 'started';
                 invokeWatch('model.modified', 1);
                 expect(controller.buttonCssClass()).toEqual("icon-pause");
                 expect(controller.buttonText()).toEqual("Pause");
             });
 
-            it("performs correct start/pause action on click", function () {
+            it("shows cssclass & name for the stop action", function () {
+                invokeWatch('domainObject', mockDomainObject);
+                expect(controller.stopButtonCssClass()).toEqual("");
+                expect(controller.stopButtonText()).toEqual("");
+
+                testModel.timestamp = 12321;
+                testModel.timerState = 'started';
+                invokeWatch('model.modified', 1);
+                expect(controller.stopButtonCssClass()).toEqual("icon-box");
+                expect(controller.stopButtonText()).toEqual("Stop");
+            });
+
+            it("performs correct start/pause/stop action on click", function () {
+                //test start
                 invokeWatch('domainObject', mockDomainObject);
                 expect(mockStart.perform).not.toHaveBeenCalled();
                 controller.clickButton();
                 expect(mockStart.perform).toHaveBeenCalled();
 
+                //test pause
                 testModel.timestamp = 12321;
+                testModel.timerState = 'started';
                 invokeWatch('model.modified', 1);
                 expect(mockPause.perform).not.toHaveBeenCalled();
                 controller.clickButton();
                 expect(mockPause.perform).toHaveBeenCalled();
+
+                //test stop
+                expect(mockStop.perform).not.toHaveBeenCalled();
+                controller.clickStopButton();
+                expect(mockStop.perform).toHaveBeenCalled();
             });
 
             it("stops requesting animation frames when destroyed", function () {