1224124 - [Camera]The "Cancel" button on the action menu does not appear as an oval button.

Status: RESOLVED WONTFIX

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

Description

[Queena.Wen(Leave from Mozilla)]

Attachment: logcat_0208.txt

[1.Description]:
[Aries KK v2.5&2.6][Flame KK v2.5&2.6] The "Cancel" button on the action menu does not appear as an oval button after tapping the " ..."  icon on the preview page.

See attachments: Aries KK v2.6.3gp&Aries KK v2.6.png&logcat_0208.txt
Found Time: 02:08

[2.Testing Steps]: 
1. Launch the camera app.
2. Click the camera "shutter" button. 
3. Click the thumbnail of the photo just taken.
4. Tap the " ..." icon at the right top.

[3.Expected Result]: 
4. The "Cancel" button should appear as an oval button.

[4.Actual Result]: 
4. The Cancel button will appear as a rectangular button similar to other options in the menu, with its text left-aligned and there is a blank space below it at the bottom of the screen.

[5.Reproduction build]: 
Device: Aries KK v2.6 master( Affected )
Build ID               20151112014800
Gaia Revision          fdb66f75963fa9255f707af87f405d54892e5e7d
Gaia Date              2015-11-11 17:17:32
Gecko Revision         https://hg.mozilla.org/integration/mozilla-inbound/rev/a8ed7dd831d1969a5a1a8636e63bd93d6aeaf94a
Gecko Version          45.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151112.011957
Firmware Date          Thu Nov 12 01:20:05 UTC 2015
Bootloader             s1

Device: Aries KK v2.5( Affected )
Build ID               20151110094357
Gaia Revision          07baf613699fa6225359c7f04825c5caeb71d424
Gaia Date              2015-11-09 21:32:50
Gecko Revision         http://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/e14287b00a514a15418dfaa89287030c588ad19d
Gecko Version          44.0a2
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151110.090331
Firmware Date          Tue Nov 10 09:03:39 UTC 2015
Bootloader             s1

Device: Flame KK v2.6 master 512mb( Affected )
Build ID               20151111150236
Gaia Revision          22f8023b112dfae83531b0a075ab9eb9a5444dfa
Gaia Date              2015-11-10 23:35:38
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/84a7cf29f4f14c9b359db2f7f19c0abd6a8e178e
Gecko Version          45.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151111.182640
Firmware Date          Wed Nov 11 18:26:52 EST 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Flame KK v2.5 512mb( Affected )
Build ID               20151109004552
Gaia Revision          cf646c52bb947af28329b0a100df91d1b1f2a907
Gaia Date              2015-11-09 02:55:50
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/4eafef5b80f8985c94c4a067f130d37513e1a581
Gecko Version          44.0a2
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151109.041411
Firmware Date          Mon Nov  9 04:14:26 EST 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

[6.Reproduction Frequency]: 
Always Recurrence,10/10

[7.TCID]: 
Free Test

Comment 1

[Queena.Wen(Leave from Mozilla)]

Note: 
This is a regression bug because it can't be repro on the Flame KK v2.2 512mb.
Device: Flame KK v2.2 512mb( Unaffected )
Build ID               20151111032502
Gaia Revision          885647d92208fb67574ced44004ab2f29d23cb45
Gaia Date              2015-10-07 13:05:24
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/ac5fce5a78e5
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151111.064642
Firmware Date          Wed Nov 11 06:46:54 EST 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Comment 2

[Queena.Wen(Leave from Mozilla)]

Attachment: Aries KK v2.6.png

Comment 3

[Queena.Wen(Leave from Mozilla)]

Attachment: Aries KK v2.6.3gp

Comment 4

[GitHub Autolander Bot]

Attachment: [gaia] prateekjadhwani:1224124 > mozilla-b2g:master

Comment 5

[Kevin Grandon :kgrandon]

Hello Prateek,

Thank you very much for the patch. Unfortunately this is not quite right. Ideally we want to use the gaia-menu component which fixes this like in bug 1223684, or alternatively we can just adjust the markup to match what the building block CSS expects. Can you make the adjustment?

Comment 6

[Prateek Jadhwani [:prateekjadhwani]]

Thanks Kevin. Should I add you as a reviewr after the fix?

Comment 7

[Prateek Jadhwani [:prateekjadhwani]]

Comment on attachment 8686957 [details] [review]
[gaia] prateekjadhwani:1224124 > mozilla-b2g:master

Hi David, I was wondering if you could review the code and let me know if I need to make any changes.

Thanks

Comment 8

[Kevin Grandon :kgrandon]

David or a camera peer would be a better reviewer than I. Your fix looks like the appropriate one at this time, though I did leave a comment on github. Thanks!

Comment 9

[Prateek Jadhwani [:prateekjadhwani]]

(In reply to Kevin Grandon :kgrandon from comment #8)
> David or a camera peer would be a better reviewer than I. Your fix looks
> like the appropriate one at this time, though I did leave a comment on
> github. Thanks!

Thanks Kevin for pointing me towards correct practices.

Comment 10

[David Flanagan [:djf]]

Comment on attachment 8686957 [details] [review]
[gaia] prateekjadhwani:1224124 > mozilla-b2g:master

Prateek,

Thanks for taking on this bug. Unfortunately, the Camera app is very different from the Video app in the way it manages and loads its dependencies, so the code from bug 1223684 won't really work in this case.

I'm not really active with the Camera app, and I don't know anything about about gaia-menu, so I'm not fully qualified to review this, but I'm pretty sure that those who are would have given r- because you're not using Bower and requirejs like the rest of the camera app does.

In comment #5, Kevin wrote "or alternatively we can just adjust the markup to match what the building block CSS expects".  I don't know what the underlying bug is, so I don't know what markup needs to be adjusted. I agree with Kevin that switching to gaia-menu is the ideal, but I suspect that given the complexities of the Camera app, the easier way to get this bug fixed and uplifted into 2.5, will be to continue using the existing CSS and modify the markup in apps/camera/js/views/preview-gallery.js.

If you don't know what those necessary changes might be, please set needinfo for Kevin.

In the future, Justin or Wilson will be better reviewers than I will be. I'm setting needinfo for them now so that they know about this bug and your work on it.

Comment 11

[Wilson Page [:wilsonpage]]

Please add any new shared/ dependencies into the requirejs config [1] and then `require()` them where needed.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/config/require.js

Comment 12

[Kevin Grandon :kgrandon]

Prateek - Do you have time to address the review comments in comment 11? I would recommend switching to the gaia-menu component, up to camera peers about which one to use. (If we don't want UX changes, then using the one in shared/ is probably correct)

Comment 13

[Prateek Jadhwani [:prateekjadhwani]]

(In reply to Kevin Grandon :kgrandon from comment #12)
> Prateek - Do you have time to address the review comments in comment 11? I
> would recommend switching to the gaia-menu component, up to camera peers
> about which one to use. (If we don't want UX changes, then using the one in
> shared/ is probably correct)

Yep, I will be making the changes as per the comments this weekend.
Thanks :)

Comment 15

[BMO Automation]

Firefox OS is not being worked on