[Download Manager] Allow downloaded files to be shared

RESOLVED FIXED in 1.4 S1 (14feb)

Status

Firefox OS
Gaia::Settings
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: djf, Assigned: crdlc)

Tracking

(Blocks: 1 bug)

unspecified
1.4 S1 (14feb)
x86
Mac OS X
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [systemsfe])

Attachments

(8 attachments)

(Reporter)

Description

4 years ago
Once a file has been downloaded and appears in the Downloads section of the Settings app, tapping on it opens it with an open or view activity of some kind.

I would like to also have a way to invoke a Share activity on a downloaded file.

This will be important for ringtones. The user downloads a file and then wants to make that file into a ringtone. The Share activity is how we do that.

The same situation exists if the user downloads a photo and then wants to upload to twitter or share via bluetooth: all of those things need a share activity.

The basic issue is that when apps are invoked via open activity to display a file, they don't offer activities themselves. The user can't preview an image or audio file with the open activity and then decide to share the image or music with a share activity.  The downloads panel of the settings app is going to need to offer the sharing option, too.

I think the issue is pretty fundamental to the way activities work. As far as I know we can't chain them.  But even if we could, we wouldn't want to because the low memory killer is not allowed to kill the invoking apps, so if activities could be chained, we'd end up with a chain of unkillable apps and could not free up memory.
(Reporter)

Comment 1

4 years ago
Setting 1.4? because I think it will be critical for the ringtones work we want to do in 1.4.

Also setting needinfo for francisco (who I discussed this with via IRC) and for Francis who is the UX designer for downloads.
blocking-b2g: --- → 1.4?
Flags: needinfo?(francisco.jordano)
Flags: needinfo?(fdjabri)
Sounds like a valid use case to me.

Francis, Cristian, what about maintaining a list of mimetypes that are susceptible of being 'shared' and 'viewed', offering a first screen where we let the user share or open. Something like we do on the call log when we have a new number, that we give a chooice for adding to a new contact or existing one.

wdyt?
Flags: needinfo?(francisco.jordano)
(Assignee)

Comment 3

4 years ago
It sounds goods to me for sure. I used this file [1] in order to know which mime types are supported in Gaia or not for opening files

[1] https://github.com/mozilla-b2g/gaia/blob/master/shared/js/mime_mapper.js
Blocks: 926955
Whiteboard: [systemsfe]
(Assignee)

Updated

4 years ago
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
(Assignee)

Comment 4

4 years ago
Created attachment 8359735 [details]
share step 1.png

Step 1 (download list)
(Assignee)

Comment 5

4 years ago
Created attachment 8359736 [details]
share step 2.png

Step 2 (click on image)
(Assignee)

Comment 6

4 years ago
Created attachment 8359737 [details]
share step 3.png

Step 3 (set as wallpaper)
(Assignee)

Comment 7

4 years ago
Created attachment 8359738 [details]
share step 4.png

Step 4 (new wallpaper in home)
(Assignee)

Comment 8

4 years ago
I've implemented in my repo [1] the new feature to share downloaded files

[1] https://github.com/crdlc/gaia/commit/ef58ae7e2f3eff81bf243f1f1043a7009c9b3a3e

I attached some screenshots in order to see how it works. We need the input from UX. 

Thanks Francis
(Assignee)

Comment 9

4 years ago
The use case shows how a downloaded image can be set as wallpaper from the download list in settings app
(Assignee)

Comment 10

4 years ago
Previously added the commit but he is alive and it is changing continuously. This is the branch for the feature:

https://github.com/crdlc/gaia/tree/bug-951281
(Reporter)

Comment 11

4 years ago
Cristian,

Thanks for this implementation!  Does sharing work for the audio file, too?  I think "set as ringtone" should be one of the options you see if so.

From a UX standpoint, it seems unfortunate that the screen in comment #5 is necessary.  I hope that Francis can come up with something there.

Francis,

Is there going to be delete functionality?  If so, then you've got view, share and delete to think about here.  Also, now that we're getting NFC, have you thought about enabling sharing of downloaded files via NFC?  In order to make that work, you need some kind of single item view for a file, I think.  So if the user could tap on a filename to see the size and download date and expose the view, share, and delete buttons, then you could also use that view as the way to launch NFC sharing.

Of course, you probably want view as the default when tapping on a filename, so could each item in the list have some kind of "more" button that exposes share, delete, and NFC?
(Assignee)

Comment 12

4 years ago
(In reply to David Flanagan [:djf] from comment #11)
> Cristian,
> 
> Thanks for this implementation!  Does sharing work for the audio file, too? 
> I think "set as ringtone" should be one of the options you see if so.

Yes, the ringtone is working fine as well

> 
> From a UX standpoint, it seems unfortunate that the screen in comment #5 is
> necessary.  I hope that Francis can come up with something there.

Yes, sure

> 
> Francis,
> 
> Is there going to be delete functionality?  If so, then you've got view,
> share and delete to think about here.  Also, now that we're getting NFC,
> have you thought about enabling sharing of downloaded files via NFC?  In
> order to make that work, you need some kind of single item view for a file,
> I think.  So if the user could tap on a filename to see the size and
> download date and expose the view, share, and delete buttons, then you could
> also use that view as the way to launch NFC sharing.
> 
> Of course, you probably want view as the default when tapping on a filename,
> so could each item in the list have some kind of "more" button that exposes
> share, delete, and NFC?

The current wireframes allow client to delete downloaded files from edit more, although I agree with adding a delete option in that view too

Thanks
(Assignee)

Comment 13

4 years ago
Created attachment 8360243 [details]
Set as ringtone.pdf

This document shows the wireframes implemented for setting a ringtone
(Assignee)

Comment 14

4 years ago
> So if the user could tap on a filename to see the size and download date

These info is available in the download list thought (comment 4)
(Assignee)

Comment 15

4 years ago
Created attachment 8360335 [details]
Patch v1

This is a good point to start with a review of code IMHO
Attachment #8360335 - Flags: ui-review?(fdjabri)
Attachment #8360335 - Flags: review?(francisco.jordano)
Attachment #8360335 - Flags: review?(dflanagan)
Comment on attachment 8360335 [details]
Patch v1

Tried on the phone and working pretty nice.

Left some comments in github, no blocking, so r+ :)

Thanks Christian for the work!
Attachment #8360335 - Flags: review?(francisco.jordano) → review+
Created attachment 8363384 [details]
set as ringtone_Francis.pdf

As discussed over email, please see a proposal to streamline the interaction attached.
Flags: needinfo?(fdjabri)
(Assignee)

Comment 18

4 years ago
Created attachment 8364218 [details]
Set as ringtone already implemented in the patch

I've attached the current implementation following your wireframe. Although we don't have the preview mode in download list because it is almost impossible to do according to David's comments (and mine) on the e-mail thread. Are you comfortable with this implementation? Thanks a lot
Attachment #8364218 - Flags: ui-review?(fdjabri)
(Assignee)

Comment 19

4 years ago
David,

    Hi, how are you? I hope not bad ;) Quick question mate, are you comfortable reviewing this feature or do you prefer that I move this review to another person?

Thanks a lot
Flags: needinfo?(dflanagan)
(Reporter)

Comment 20

4 years ago
Cristian,

Yes, I'm totally comfortable reviewing.  I've just been on a workweek in Taipei, and now busy with 1.3 blockers. So I'm very behind on my review queue.  I'll get to it as soon as I can.  (But won't be offended if you find a different reviewer...)
Flags: needinfo?(dflanagan)
Target Milestone: --- → 1.4 S1 (14feb)
(Assignee)

Comment 21

4 years ago
David,

   How are you? Do you have any chance to review it during this week? If you cannot do it, do you have any person in you radar to take a look at this one?

Thanks a lot
Flags: needinfo?(dflanagan)
(Reporter)

Comment 22

4 years ago
I'm still completely busy with 1.3+ bugs.  I don't have any particular suggestion for reviewers. I would guess that anyone who worked on download manager would be a good reviewer, as the activity launching part ought to be simple.
Flags: needinfo?(dflanagan)
(Reporter)

Comment 23

4 years ago
Comment on attachment 8360335 [details]
Patch v1

r+ with the caveat that we don't yet have a workable UX spec from Francis, and this patch does not match the (un-implementable) UX spec we do have.

Note that it is only the activity and setringtone and wallpaper app stuff that I have actually reviewed carefully. I assume that francisco's review covers the DownloadHelper code, etc.  (Overall, that code strikes me as too highly abstracted and probably more complicated than it needs to be, but I'm not familiar with the download manager, so my opinion is not well-informed.)

Also, caution that having separate "set as ringtone" and "set as wallpaper" buttons may conflict with UX elswhere in the OS where I'd expect to have only a share activity.  (The music app, for example, just has a share button and one of the sharing choices is set as ringtone. There are no plans to add an explicit "set as ringtone" button in that app)

Finally, note that we are about to merge the setringtone app into the ringtones app, and by the time you get ui-review from Francis, the patch will likely not be good any more.

Setting needinfo for Jim Porter, who is working on integrating the setringtone app with the ringtone app.  Jim: this is a Download Manager patch to enable sharing downloaded audio as ringtones.  The incomplete UX spec calls for an explicit "set as ringtone" button in addition to a generic "share" button. So this patch adds a new "setringtone" activity handler to the setringtone app. It does the same thing as "share" but goes straight to the setringtone app without showing the user other choices.  If I can't convince Francis to remove that feature, you'll need to support this new activity type in your modified app. (It should be trivial however.)
Attachment #8360335 - Flags: review?(dflanagan) → review+
Flags: needinfo?(squibblyflabbetydoo)

Comment 24

4 years ago
Noted. I'm not sure I like adding a new "setringtone" activity either, although I suppose I don't have a strong opinion about it one way or the other.
Flags: needinfo?(squibblyflabbetydoo)
The only thing that cannot be implemented is the preview in first screen, that we already indicated. Francis, can you review the UI and confirm you agree to remove preview in first screen?
Flags: needinfo?(fdjabri)
Hi, for me the implementation is fine. As for dropping the "Set as ringtone" option - sorry, but only having "Share" sounds misleading to me. I will sync up with Jacqueline on this tomorrow.
Flags: needinfo?(fdjabri)
Attachment #8360335 - Flags: ui-review?(fdjabri) → ui-review+
(Assignee)

Comment 27

4 years ago
Merged in master:

https://github.com/crdlc/gaia/commit/eeae4d7ad411c0379c9885669a3307d00151955f
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Blocks: 972849
Blocks: 972854
Hi, I've synced up with Jacqueline and can confirm that we should keep the "Set as ringtone" option in this implementation.
Attachment #8364218 - Flags: ui-review?(fdjabri) → ui-review+

Updated

4 years ago
blocking-b2g: 1.4? → ---
Blocks: 976540
You need to log in before you can comment on or make changes to this bug.