902999 - [Clock] Add Time To In-Progress Timer, "+1 Min"

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Clock, defect)

Description

[Peter Dolanjski [:pdol]]

User Story:

As a user I want to be able to add time to a count down that is already in progress so that I can adjust the duration on the fly.


Acceptance Criteria:

1. After my timer count down is in progress, I can add time to the remaining count down.
2. After my timer count down completes (reaches 0), I can no longer add time to the count down.

Comment 1

[Rick Waldron [:rwaldron]]

This is neither in the latest spec or in the UI mock ups. Since it adds little to no value, I recommend eliminating the feature. If a user wants a timer to go longer, they only have to tap "Cancel" and make a longer Timer.

- https://mozilla.app.box.com/s/wzgsb3lkqglv0dnfdgzs/1/867805696/9286060691/1
- https://mozilla.app.box.com/s/wzgsb3lkqglv0dnfdgzs/1/1092493904/9862011878/1

Comment 2

[Peter Dolanjski [:pdol]]

Jacqueline, was this ever implemented in the spec?

Comment 3

[Jacqueline Savory [:jsavory] UX]

I have the wires for this story in the attached document on page 6. Eric Pang, I don't believe you have seen this one, so any feedback is welcome.

Comment 4

[Jacqueline Savory [:jsavory] UX]

Attachment: clock-timer-130820.pdf

Comment 5

[Eric Pang [:epang] UX]

(In reply to jsavory from comment #3)
> I have the wires for this story in the attached document on page 6. Eric
> Pang, I don't believe you have seen this one, so any feedback is welcome.

Looks good to me, I've also added the "+ 1 Min" to the mock ups on box.

Comment 6

[Rick Waldron [:rwaldron]]

Attachment: Pull request https://github.com/mozilla-b2g/gaia/pull/12402

- Moves value accessor property to Picker.prototype
    - Defines set accessor operation for Picker.prototype.value

- Defines Timer.prototype.plus(seconds)
    - Add an arbitrary number of seconds to the current timer.
    - Seconds are auto-converted to milliseconds
    - duration and endAt are extended accordingly

- Tests for
    - Picker.prototype.value
    - Timer.prototype.plus()

Comment 7

[Rick Waldron [:rwaldron]]

Requesting koi+ since this is a feature defined for 1.2 that was late on explanation.

[Bug caused by] (feature/regressing bug #): None.
[User impact] if declined: Not able to add time to an in-progress timer, as defined by this user story and 1.2 visual spec https://mozilla.app.box.com/s/wzgsb3lkqglv0dnfdgzs/1/1092493904/9862018492/1
[Testing completed]: Tested on devices and desktop
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: adds "+1 Min"

Comment 8

[Dylan Oliver [:doliver]]

triage: not blocking, too late to take new features for 1.2, will include this with 1.3

Comment 9

[Mike Pennisi [:jugglinmike]]

Comment on attachment 809333 [details] [review]
Pull request https://github.com/mozilla-b2g/gaia/pull/12402

Hey Rick!

Thanks for you patience--we're finally in a good position to land this. The "koi+" patches that have landed since you wrote this patch introduce a conflict, but don't worry! It's extremely easy to resolve: just a matter of deleting a set of conflict markers.

In the interest of expediency (which sounds a little ironic at this point), I've given you feedback on GitHub based on this patch applied to `master` and after the conflict resolution.

Comment 10

[Rick Waldron [:rwaldron]]

NI? [e-mail]


There is a UI element that looks like "+1" which is a string and currently is one string, but the "+" is actually designed to be a bit larger and therefore needs its own container and subsequently is a second string. Can l10n advise? Should we bother making two strings for this, or punt on trying to make the "+" bigger?

Comment 11

[Axel Hecht [:Pike]]

I'd suggest to punt on the +, I think it's gonna be tricky to make sure it's always at the same position, etc.

Comment 12

[Mike Pennisi [:jugglinmike]]

Comment on attachment 809333 [details] [review]
Pull request https://github.com/mozilla-b2g/gaia/pull/12402

Hey Rick,

As I mentioned on the GitHub pull request, lets wait until Eric's work for bug 898354 to land. That will make it easier to ensure the new "+1 Minute" button is responsive. I'll mark this bug as blocked by that one in just a minute

Comment 13

[Mike Pennisi [:jugglinmike]]

Blocked by bug 898354 (see comment 12)

Comment 14

[Dylan Oliver [:doliver]]

Is this one free to resume now that bug 898354 has landed?

Comment 15

[Mike Pennisi [:jugglinmike]]

Yes, but Rick may be too busy to finalize this patch. Since the blocking bug landed less than an hour ago, he probably just hasn't noticed yet.

Rick: if you don't have the bandwidth for this, feel free to assign it to me :)

Comment 16

[Rick Waldron [:rwaldron]]

I thought we had discussed this some time ago? All yours

Comment 17

[Mike Pennisi [:jugglinmike]]

Attachment: Pull request on GitHub.com

Hey Rick,

Since this is directly based on your work, I thought I'd ask for your feedback before requesting a review.

Thanks!

Comment 18

[Mike Pennisi [:jugglinmike]]

Comment on attachment 8361452 [details] [review]
Pull request on GitHub.com

Hey Marcus: would you mind looking over this patch for me?

Comment 19

[Marcus Cavanaugh [:mcav]]

Comment on attachment 8361452 [details] [review]
Pull request on GitHub.com

Looks good to me; tested on nightly. r+ provided Travis passes before merge.

Comment 20

[Mike Pennisi [:jugglinmike]]

Unrelated Marionette failure (see below). I'm re-running the Marionette tests before merging.

  1) navigation opens rocketbar from homescreen:
     
  NoSuchElement: (7) Unable to locate element: iframe[mozapptype="mozsearch"]
  Remote Stack:
  <none>
      at Error.MarionetteError (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/error.js:67:13)
      at Object.Client._handleCallback (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:474:19)
      at /home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:508:21
      at TcpSync.send (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/drivers/tcp-sync.js:94:10)
      at Object.send (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:455:36)
      at Object.Client._sendCommand (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:501:19)
      at Object._findElement (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:1283:19)
      at Object.findElement (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:1332:32)
      at Object.MarionetteHelper.waitForElement (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-helper/index.js:128:19)
      at Object.Search.goToResults (/home/travis/build/mozilla-b2g/gaia/apps/search/test/marionette/lib/search.js:101:8)
      at Context.<anonymous> (/home/travis/build/mozilla-b2g/gaia/apps/search/test/marionette/navigation_test.js:65:12)
      at Test.Runnable.run (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:211:32)
      at Runner.runTest (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:372:10)
      at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:448:12
      at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:297:14)
      at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:307:7
      at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:245:23)
      at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:269:7
      at Hook.Runnable.run (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:213:5)
      at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:257:10)
      at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:269:7
      at done (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:185:5)
      at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:197:9
      at Object.executeHook (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:370:18)
      at process._tickCallback (node.js:415:13)

Comment 21

[Mike Pennisi [:jugglinmike]]

master: https://github.com/mozilla-b2g/gaia/commit/e22f9415886554999f2a9f234e7f3f6b91ebad2f

(Tests passed on TravisCI.) Thanks for the help, Rick and Marcus!