951281 - [Download Manager] Allow downloaded files to be shared

Status: RESOLVED FIXED

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

Description

[David Flanagan [:djf]]

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.

Comment 1

[David Flanagan [:djf]]

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.

Comment 2

[Francisco Jordano [:arcturus] [:francisco]]

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?

Comment 3

[Cristian Rodriguez (:crdlc)]

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

Comment 4

[Cristian Rodriguez (:crdlc)]

Attachment: share step 1.png

Step 1 (download list)

Comment 5

[Cristian Rodriguez (:crdlc)]

Attachment: share step 2.png

Step 2 (click on image)

Comment 6

[Cristian Rodriguez (:crdlc)]

Attachment: share step 3.png

Step 3 (set as wallpaper)

Comment 7

[Cristian Rodriguez (:crdlc)]

Attachment: share step 4.png

Step 4 (new wallpaper in home)

Comment 8

[Cristian Rodriguez (:crdlc)]

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

Comment 9

[Cristian Rodriguez (:crdlc)]

The use case shows how a downloaded image can be set as wallpaper from the download list in settings app

Comment 10

[Cristian Rodriguez (:crdlc)]

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

Comment 11

[David Flanagan [:djf]]

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?

Comment 12

[Cristian Rodriguez (:crdlc)]

(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

Comment 13

[Cristian Rodriguez (:crdlc)]

Attachment: Set as ringtone.pdf

This document shows the wireframes implemented for setting a ringtone

Comment 14

[Cristian Rodriguez (:crdlc)]

> 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)

Comment 15

[Cristian Rodriguez (:crdlc)]

Attachment: Patch v1

This is a good point to start with a review of code IMHO

Comment 16

[Francisco Jordano [:arcturus] [:francisco]]

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!

Comment 17

[Francis Djabri [:djabber]]

Attachment: set as ringtone_Francis.pdf

As discussed over email, please see a proposal to streamline the interaction attached.

Comment 18

[Cristian Rodriguez (:crdlc)]

Attachment: 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

Comment 19

[Cristian Rodriguez (:crdlc)]

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

Comment 20

[David Flanagan [:djf]]

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...)

Comment 21

[Cristian Rodriguez (:crdlc)]

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

Comment 22

[David Flanagan [:djf]]

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.

Comment 23

[David Flanagan [:djf]]

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.)

Comment 24

[Jim Porter (:squib)]

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.

Comment 25

[Marcelino Veiga Tuimil [:sonmarce]]

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?

Comment 26

[Francis Djabri [:djabber]]

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.

Comment 27

[Cristian Rodriguez (:crdlc)]

Merged in master:

https://github.com/crdlc/gaia/commit/eeae4d7ad411c0379c9885669a3307d00151955f

Comment 28

[Francis Djabri [:djabber]]

Hi, I've synced up with Jacqueline and can confirm that we should keep the "Set as ringtone" option in this implementation.