Closed Bug 966837 Opened 11 years ago Closed 11 years ago

[User story] Add self-timer to camera settings

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::Camera
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-b2g:1.4+, b2g-v1.4 fixed, b2g-v2.0 fixed)

Status:
RESOLVED FIXED
Milestone:
1.4 S5 (11apr)
Project Flags:
blocking-b2g 1.4+
Tracking Flags:
Tracking Status
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: skasetti, Assigned: singhashish1887)

References

Details

(Whiteboard: [fxos:media])

User Story

1. As a user, I can set the self-timer mode in my camera settings to off, 2 sec, 10 sec
2. As a user, when I set the self timer to a value that value should persist until I change it again

Acceptance criteria:
1. The user can find self-timer as an option in the camera settings menu
2. Tapping the self-timer takes the user to the preview screen with 0 sec, 2 sec and 10 sec as options to select. The previously selected option should be highlighted (default is 0 sec)
3. Once the user selects an option and closes the view menu, the option is saved
4. Tapping on the shutter button should start the count down 
5. The image is captured when the count is '1'

Attachments

(2 files, 2 obsolete files)

pull-request (camera-new-features)
46 bytes, text/x-github-pull-request
djf
: review+
dmarcos
: ui-review?
amylee
tif
: ui-review-
tif
: ui-review-
Details | Review
Pull Request Rebased
46 bytes, text/x-github-pull-request
Details | Review
No description provided.
User Story: (updated)
Ashish! Please check the user stories and update the current status.
Assignee: nobody → singhashish1887
Self timer UI and functionality is implemeted
Blocks: 966764
Depends on: 971460
Attached file Pointer to Pull Request.html (obsolete) —
Hi David, We have uploaded the work in progress code for self timer. This code does not have test codes.
Attachment #8380120 - Flags: review?(dflanagan)
Comment on attachment 8380120 [details] Pointer to Pull Request.html Adding Diego and Wilson for feedback
Attachment #8380120 - Flags: feedback?(wilsonpage)
Attachment #8380120 - Flags: feedback?(dmarcos)
Comment on attachment 8380120 [details] Pointer to Pull Request.html This work has been reviewed, improved and now landed in camera-dev pending review/feedback from djf.
Attachment #8380120 - Flags: feedback?(wilsonpage)
Attachment #8380120 - Flags: feedback?(dflanagan)
Attachment #8380120 - Flags: feedback-
Attached file patch (camera-dev) (obsolete) —
Attachment #8380459 - Flags: feedback?(dflanagan)
Attachment #8380120 - Flags: feedback?(dflanagan)
Attachment #8380459 - Flags: feedback?(dflanagan)
Attachment #8380459 - Attachment is obsolete: true
Please ignore my last comments, incorrect bug.
Comment on attachment 8380120 [details] Pointer to Pull Request.html Feedback left on github. Clearing the review request In the future it would be helpful to attache screenshots or videos, also.
Attachment #8380120 - Flags: review?(dflanagan)
3/4 update: Ashish to work on incorporating review feedback and fix vis dev per spec
Wilson, please create PR for camera branch so David can review
Flags: needinfo?(wilsonpage)
Attachment #8385836 - Flags: review?(dflanagan)
PR Created
Flags: needinfo?(wilsonpage)
Comment on attachment 8385836 [details] [review] pull-request (camera-new-features) r- because: - there is unused code in controllers/camera.js left over from a previous version of the patch - there are changes in this patch that are not related to the timer - it is too risky to change js/vendor/evt.js - I think the architecture is wrong and it is the controller that should be doing the setInterval() call and actuall controlling the countdown. See the various comments on github: https://github.com/dmarcos/gaia/commit/5c5533ddc3ac14d6dbad03cbad31034a44748ac5 Also: file modes. Also: we want a way to turn this off from config/app.js in case it causes problems and we can't back it out cleanly. I know we can just remove the timer from the settings menu. But the timer indicator code (which is probably in another patch) may need a single boolean it can query to find out if the timer is enabled in the build or disabled in the build.
Attachment #8385836 - Flags: review?(dflanagan) → review-
Needinfo Ashish to be sure he sees the review.
Flags: needinfo?(singhashish1887)
David We are working on your comments .ASAP changes are done we will upload the patches
Flags: needinfo?(singhashish1887)
Changes are done by wilson in timer. So I request Wilson to please help me in resolving Davids comments.
Flags: needinfo?(wilsonpage)
I'm going to work with Ashish to resolve these issues.
Flags: needinfo?(wilsonpage)
Attachment #8380120 - Attachment is obsolete: true
Attachment #8380120 - Flags: feedback?(dmarcos)
Comment on attachment 8385836 [details] [review] pull-request (camera-new-features) Ready for re-review. Left commits un-squashed so that review changes can be seen.
Attachment #8385836 - Flags: review- → review?(dflanagan)
Comment on attachment 8385836 [details] [review] pull-request (camera-new-features) r+ with nits addressed. Note that the pull request cannot be merged cleanly, so I can't test this myself. But assuming that it is tested manually and works, and that the automated tests are working, and that my nits on github are addressed, it has my r+. Note that ux-review is still required, however. You might want to start by attaching some screenshots at least. I've left comments/nits on this main commit: https://github.com/dmarcos/gaia/commit/51f7caa8fd31d5f40ea90e3a657ee738fd025315 Also here: https://github.com/dmarcos/gaia/commit/380009229127c7e5fda79d121c6e662164eadae9 where the word 'imminent' is misspelled as the completely different word 'immanent'.
Attachment #8385836 - Flags: review?(dflanagan) → review+
Landed on camera-new-features: https://github.com/mozilla-b2g/gaia/commit/b85de97cc6e84084bc9141e64a8fd01294281136
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
I don't see explicit ux+ -- Flagging ux for reviews (amy/tif: if there are minor spec deviations, please file bugs so we can fix them -- reopen this bug if there are major issues) -- this feature has landed on camera-new-features branch only
Flags: needinfo?(tshakespeare)
Flags: needinfo?(amlee)
Attachment #8385836 - Flags: ui-review?(tshakespeare)
Attachment #8385836 - Flags: ui-review?(rmacdonald)
Attachment #8385836 - Flags: ui-review?(amlee)
You can test this patch in camera new features: git remote add mozilla https://github.com/mozilla-b2g/gaia git fetch mozilla git checkout mozilla/camera-new-features make install-gaia APP=camera
Comment on attachment 8385836 [details] [review] pull-request (camera-new-features) Put latest patch on my phone and see a few issues: - toggling the self-timer options, the confirmation text when menu closes is missing - according to the spec, "The camera sounds a quick beep when the self-timer hits exactly three, two and one seconds." I'm not hearing a beep, but I do hear the shutter noise - it also seems like there is a delay between the counter hitting zero and the photo being taken? I'm letting the camera sit for 30 to test that the timer resets to off. Will update...
Attachment #8385836 - Flags: ui-review?(tshakespeare)
Attachment #8385836 - Flags: ui-review?(rmacdonald)
Attachment #8385836 - Flags: ui-review-
Flags: needinfo?(tshakespeare)
Oh one more thing - self-timer is hyphenated.
So as far as reseting the self-timer to off, this is the behaviour I noticed. When turning the self-timer on, exiting the app and coming back after about 30min, the self-timer was still on. When I left the phone overnight and tried tapping on the camera app, it seemed like it was launching it again and the self-timer was off. According to the spec, "the self-timer setting resets to off after the Camera is not in the foreground for 30 minutes or more."
This is a surprisingly complex feature. - What should happen if the app is fully closed a re-opened before 30mins, should the timer stay on or off? - If we need to switch the timer off after 30mins we should perhaps set an `expires` type attribute for settings like this that get checked each time the app is launched / made visible. - Should this block 1.4?
Flags: needinfo?(tshakespeare)
This shouldn't block 1.4
Seems like this bug has actually been closed. Should we move the remaining issue to the other self-timer bug? https://bugzilla.mozilla.org/show_bug.cgi?id=985578
(In reply to Hema Koka [:hema] from comment #22) > I don't see explicit ux+ -- Flagging ux for reviews (amy/tif: if there are > minor spec deviations, please file bugs so we can fix them -- reopen this > bug if there are major issues) -- this feature has landed on > camera-new-features branch only UI has been reviewed and a bug has been opened for revisions. See bug 985578
Flags: needinfo?(tshakespeare)
Flags: needinfo?(amlee)
Bulk edit for camera bugs. If earlier comments do not show how this bug landed to master, it probably landed as part of https://github.com/mozilla-b2g/gaia/pull/17599 which merged the camera-new-features branch into master. This bug was uplifted from master to v1.4 as part of https://github.com/mozilla-b2g/gaia/commit/a8190d08e61316a86bba572ba8d894d081a20530
status-b2g-v1.4: --- → fixed
status-b2g-v2.0: --- → fixed
Target Milestone: --- → 1.4 S5 (11apr)
blocking-b2g: --- → 1.4+
Flags: in-moztrap?(mozillamarcia.knous)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: