Closed Bug 915039 Opened 7 years ago Closed 6 years ago
[Video] There is no easy way to navigate back to the activity caller if the library is empty
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.22 (KHTML, like Gecko) Ubuntu Chromium/25.0.1364.160 Chrome/25.0.1364.160 Safari/537.22 Steps to reproduce: 1.Open the message composer 2.Ensure that there are no video on the device 3.Try to attach a video file to the message 4.A message is shown basically stating that no videos are available and this message overlays the back button at the top of the screen which can therefore not be selected 5.Home button needs to be used which will move the MMS composer into the background Actual results: There is no easy way to navigate back to the MMS composer if an empty category has been selected to attach an item. Expected results: Need add a back key
Summary: [Video]There is no easy way to navigate back to the MMS composer if an empty category has been selected to attach an item → [Video] There is no easy way to navigate back to the activity caller if the library is empty
Asking John Hu to review this patch since he has done simillar fixes. Here is the link to PR on github: https://github.com/mozilla-b2g/gaia/pull/12258 I usually attach html with autoredirect to PR; but someone mentioned to me that it is ok to attach the patch here instead. Let me know if I can improve my patch submission process in any way. Thanks!
In gaia, we always use the PR which is auto redirected by html. Thanks for providing both. To have a patch is helpful to apply the patch in my local branch.
Comment on attachment 805762 [details] [diff] [review] bug915039-back-button-for-open-video-activity-when-n.patch Aras, Thanks for taking this bug. I suggest we view this bug more broadly. When dealing with pick activity with blocking overlay, we got some situations that user have no way to go back to previous state or to do something to hide the overlay, like no sdcard, no video, mounted, etc. We have the following cases: error case pick buttons -------------------------------------------- no sdcard no media storage settings no sdcard yes media storage settings [close] usb plugged in no none usb plugged in yes none [close] empty list no none empty list yes none [close] upgrading no none (waiting is fine) upgrading yes none (waiting is fine) scanning no none (waiting is fine) scanning yes none (waiting is fine) I had tested the top 6 cases. Your patch handles both usb plugged in and empty list with one shot. I suggest to have the no sdcard case included. So, I reset the review flag. Once you add it, please reset it again. I will open a bug for requesting a "go to camera" button like you did before in the case of empty list in Gallery app.
John Hu, thanks for the quick feedback! I will try to test the no sdcard case and cover it as well, then will get back to you.
I have updated the patch based on PR feedback and added the no sdcard case
Attachment #805762 - Attachment is obsolete: true
Without the close button, user experience when trying to access video using a video pick activity is brocken. The above patch address the problem by adding the close button only to those cases, so user can discard the overlay and go back to the app that triggered the pick avtivity. I would like to get feedback from UX about this, and make sure it is an acceptable fix. These are not very commong cases, but when user get there, they should not get stuck. I think we can improve the visual representation, and maybe come up with a better layout for the case when both setting button and close button are presented.
blocking-b2g: --- → koi?
Flags: needinfo?(firefoxos-ux-bugzilla) → needinfo?(rmacdonald)
Thanks for drawing our attention to this one. Jacqueline will be putting together some wires for this in the next few days and I've flagged her on the bug. Also adding David Flanagan as a heads-up. David, Jacqueline may be in touch with you in the next couple of days with questions. David, feel free to clear the flag if it's not relevant for you.
Aras, Thank you for working on this bug! I don't know why everyone (including me) has been dragging their feet so much on it. It is pretty disheartening to see Hema's nominations of this bug for 1.2, 1.3 and now 1.4 all go ignored. Jacqueline: I don't actually think we need your input on this. We've fixed the equivalent bug in gallery already, and it is clear what the right thing to do here is. Aras: are you still interested in working on this? If not, please reassign the bug to John. If you are, I'd like to see some changes to what John proposed in comment 3: - The button should read "Cancel" instead of "Close" because the user is choosing to cancel the pick, not just close an app. - In any pick activity we should have a "Cancel" option. Even for the scanning overlay. (If cancelling an upgrade could cause data loss, then we shouldn't have a cancel button on that screen. Needinfo John to ask about that case). - During a pick activity, Cancel should be the only button shown. An inline activity like this needs to be able to go back to its caller, but it should not give the user any way to go to some other app. So please do not display the Storage Settings button in the pick activity case. - Similarly, please don't add a Camera button to the "no videos" screen in the pick activity case. But you can add it in the non-pick case. And you could update the message so that in addition to saying "load videos" it says "download or record videos" or something like that. That should probably just be a different bug, however, and we will want Jacqueline or Chris's help to decide on the wording.
Aras, Please see also bug 919834. In that bug, I claim that on devices that have and sdcard and internal memory, you will never get into the "nocard" state and will never see the "Go to Media Storage Settings" button. And on devices that have only a sdcard slot, there won't be any interesting settings there. If there is no card, you're just stuck and going to settings does not help. So I think there should never settings button on the nocard overlay. Just a cancel button in the pick activity case. Let me know if that does not make sense.
Hi David, Thanks for looking into this. I think it would be best if John or someone else would wrap this up. The earliest I can work on it would be next weekend. I will assign this bug to him. I would love to get back into gaia and contribute more. If you would be kind enough to assign a couple of slightly more challenging bugs to me to work on over the holidays, I would really appreciate it. I need a safe haven to scape to after dealing with so much horrible code at my day job.
(In reply to David Flanagan [:djf] from comment #8) > - In any pick activity we should have a "Cancel" option. Even for the > scanning overlay. (If cancelling an upgrade could cause data loss, then we > shouldn't have a cancel button on that screen. Needinfo John to ask about > that case). We can cancel the upgrade procedure with throwing an error and the database will rollback to its original state. So, when we close the activity during upgrading, the whole updated records will be rolled back.
This patch does the following change: 1. remove storage setting button, sync with bug 919834, we don't need this button anymore. 2. add cancel/camera button 3. change the showOverlay to show cancel button when pick mode and show camera only when non-pick mode and id is empty 4. update text
Comment on attachment 8348610 [details] [review] add cancel button r+ if you remove the unused code, remove the string change (or change the id) and at least consider an alternative to the CSS overrides. See github for details.
Attachment #8348610 - Flags: review?(dflanagan) → review+
merged to master: https://github.com/mozilla-b2g/gaia/commit/a523c1ee4130a7883981924f8e68fb94be02428a
Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Can we back this out or revert the string change, like David requested?
Axel, Sorry about that. I thought I was removed the change of string before. I will revert the string change within this bug.
Reopen this bug to revert the patch and remove the string change.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: FIXED → ---
reverting this patch: https://github.com/mozilla-b2g/gaia/commit/0fe11036d1d7128a7288e8efa3b73dab7497b8e9
move the r+ to this patch and remove the string change.
re-land the code to master without string change: https://github.com/mozilla-b2g/gaia/commit/28a3012b8dc4a7794b25798ac151fc25a9d42e36
Status: REOPENED → RESOLVED
Closed: 6 years ago → 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.