949857 - [Gallery]Red 'Delete' shown when deleting in multiple selection mode, blue "Ok" shown when deleting in fullscreen mode

Status: RESOLVED FIXED

Description

[Eric Chang [:ericcc] [:echang]]

Created attachment 8347002 [details]
Delete2itemsButtonDelete.jpg

Description:
Red 'Delete' shown when deleting in multiple selection mode, blue "Ok" shown when deleting in fullscreen mode


Steps to Reproduce:

1. Open a photo in Gallery, full screen mode, and delete it
2. Blue "Ok" shown for confirmation. 
3. Enter multiple selection mode.
4. Select at least one photo, and delete it/them.
5. Red "Delete" shown for confirmation. 

Expected Results:
* Same for both


Actual Results:
* Blue "Ok" shown for confirmation in full screen mode. 
* Red "Delete" shown for confirmation in multiple selection mode.



Additional Notes:



Environmental Variables:

Version: 1.2
Device: Buri
Build Date: 12/12/2013
RIL Type (Mozilla RIL or Commericial RIL): Commericial RIL

Comment 1

[Eric Chang [:ericcc] [:echang]]

Created attachment 8347003 [details]
DeleteFullScreenModeButtonOkay.jpg

Comment 2

[John Hu [:johnhu][:johu][:醬糊小弟]]

Created attachment 8347848 [details] [review]
Use the same confirm dialog

Comment 3

[David Flanagan [:djf]]

Comment on attachment 8347848 [details] [review]
Use the same confirm dialog

r- because you didn't remember to git add dialogs.js so this patch won't actually run.

If it was me, I think I'd keep this patch simpler and just call showConfirmDialog() from frames.js without refactoring to a separate file.  I like the idea of having dialogs.js, but would like to see the overlay display code put into that file too, so it seems like a separate refactoring.

If you want to go ahead with the dialogs.js refactor here (and it is okay with me if you do), I'd suggest that you call the function Dialogs.confirm() instead of Dialogs.showConfirm().

Comment 4

[John Hu [:johnhu][:johu][:醬糊小弟]]

Comment on attachment 8347848 [details] [review]
Use the same confirm dialog

Sorry about that. And the movement of overlay is a good idea. I had moved it to dialogs. Please review it again. Thanks.

Comment 5

[David Flanagan [:djf]]

Sorry to hold this up. I won't be able to review it until January.

Comment 6

[John Hu [:johnhu][:johu][:醬糊小弟]]

Sure, no problem. This is not an urgent bug.

Comment 7

[David Flanagan [:djf]]

Comment on attachment 8347848 [details] [review]
Use the same confirm dialog

Looks good to me, though I did not test. One nit on github to optionally fix.

Comment 8

[John Hu [:johnhu][:johu][:醬糊小弟]]

merged to master:
https://github.com/mozilla-b2g/gaia/commit/ec1064218fef0fadcead7a64a9ae423b655c71d0