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

RESOLVED FIXED

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: echang, Assigned: johnhu)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fxos-bug-bash-1.2])

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
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
(Reporter)

Comment 1

5 years ago
Created attachment 8347003 [details]
DeleteFullScreenModeButtonOkay.jpg
Assignee: nobody → johu
Created attachment 8347848 [details] [review]
Use the same confirm dialog
Attachment #8347848 - Flags: review?(dflanagan)
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().
Attachment #8347848 - Flags: review?(dflanagan) → review-
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.
Attachment #8347848 - Flags: review- → review?(dflanagan)
Sorry to hold this up. I won't be able to review it until January.
Sure, no problem. This is not an urgent bug.
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.
Attachment #8347848 - Flags: review?(dflanagan) → review+
merged to master:
https://github.com/mozilla-b2g/gaia/commit/ec1064218fef0fadcead7a64a9ae423b655c71d0
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Depends on: 997481
You need to log in before you can comment on or make changes to this bug.