960484 - Download button should notify user on every completed download in case of multiple downloads (followup from bug 953433)

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: Downloads, defect, P2)

Description

[Oleg Zasypkin [:azasypkin]]

We need to somehow notify user on every completed download in case of multiple concurrent downloads. Currently we only show the progress of a single download even if two are occurring at the same time.

Steps to reproduce the issue:

1) Open Firefox Metro;
2) Start download anything from a website;
3) While the first file is downloading, start new download;
4) Download indicator will become dark blue only once both downloads finished.

Current Behavior:

- Download indicator will become dark blue only once both downloads are finished without any intermediate notifications on every finished download.

Expected Behavior:

- User should be notified somehow on every finished download.

Comment 1

[Oleg Zasypkin [:azasypkin]]

Attachment: poc.diff

Here is PoC of how we can notify user on every completed download - just using concentric effect not only for the case when when all downloads finished, but for every completed download. It's similar to what desktop version does. What do you think?

Comment 2

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8375085 [details] [diff] [review]
poc.diff

Review of attachment 8375085 [details] [diff] [review]:
-----------------------------------------------------------------

Code-wise this looks great. Note you'll want to dupe bug 967632 if you end up landing the concentric ring effect here. 
I think we'll want some UI review on the behavior though. If I start 2 simultaneous downloads, the notification tracks progress of those two together. And I only see the ring effect when the 2nd of those downloads is complete. If I then start a further download without dismissing the notification, that one notifies correctly. ISTM that the goal of this bug was to keep the summarized progress, but show the navbar and ring effect on the download button when each download completes. 

Possibly a separate issue - I still find it awkward that the only way to dismiss the download-complete notification is to actually run/open the file or view its location in explore.exe. If we hooked up the close button so it also cleared out the completed downloads, the navbar download button would become more useful. As it stands, its a bit annoying as clicking it only shows you the notification you've already seen. 

I've kicked off a build on try so people can give it a whirl: https://tbpl.mozilla.org/?tree=Try&rev=f0059c7b50c8, installer will show up at: https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-f0059c7b50c8/

::: browser/metro/theme/circularprogress.css
@@ +24,2 @@
>    background-size: 40px 40px;
> +  background: url(chrome://browser/skin/images/progresscircle-bg.png) no-repeat;

doesnt using the background shorthand here just reset background-size to auto auto?

@@ +34,5 @@
> +
> +.circularprogressindicator-progressNotification.progressNotification-active {
> +  visibility: visible;
> +  opacity: 0;
> +  transform: scale(2);

I love it :)

@@ +35,5 @@
> +.circularprogressindicator-progressNotification.progressNotification-active {
> +  visibility: visible;
> +  opacity: 0;
> +  transform: scale(2);
> +  transition: opacity 1s, transform 1s;

We have a @metro_animation_duration@ we use for a lot of animations, which you might try. 1 second looked ok to me though.

Comment 3

[Oleg Zasypkin [:azasypkin]]

.(In reply to Sam Foster [:sfoster] from comment #2)
> Comment on attachment 8375085 [details] [diff] [review]
> poc.diff
> 
> Review of attachment 8375085 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Code-wise this looks great. Note you'll want to dupe bug 967632 if you end
> up landing the concentric ring effect here. 
> I think we'll want some UI review on the behavior though. If I start 2
> simultaneous downloads, the notification tracks progress of those two
> together. And I only see the ring effect when the 2nd of those downloads is
> complete. If I then start a further download without dismissing the
> notification, that one notifies correctly. ISTM that the goal of this bug
> was to keep the summarized progress, but show the navbar and ring effect on
> the download button when each download completes. 

Yeah, actually we have one more bug, that looks the same - bug 960489, but I'm not sure how I can divide these two bugs on the "patch" level, as on logical level I think it's right to have them both.

Regarding to bug 967632 - I thought we already have UX proposal for concentric ring effect (later will use just "cr effect" :) ) itself provided by mmaslaney in bug 953433 that you converted to mov :) Still need hover\active styles for "download-finished" though.

Regarding to case with 2 simultaneous downloads, did you mean that while we have several downloads and navbar is hidden, then navbar will appear (+ cr effect) only once everything is downloaded? Yes, currently intermediate cr effect is visible only if navabar is visible. Agree that we need UX feedback to improve that. The easiest solution would be showing up navbar + cr effect on every completed download, but it may be distracting for the user. Probably we can use some less distracting (smth like toast?) notifications in case navbar is hidden and cr effect otherwise. 

> Possibly a separate issue - I still find it awkward that the only way to
> dismiss the download-complete notification is to actually run/open the file
> or view its location in explore.exe. If we hooked up the close button so it
> also cleared out the completed downloads, the navbar download button would
> become more useful. As it stands, its a bit annoying as clicking it only
> shows you the notification you've already seen. 

Totally agree on that, first I was thinking that was a bug :) I think it's a good candidate for the separate bug. Also I'm a bit confused with infobar all the time: in case we started 2 downloads, it displays "2 files downloading", once one of the files is downloaded it still displays "2 files downloading". Do we have some initial info on how it should be?

> I've kicked off a build on try so people can give it a whirl:
> https://tbpl.mozilla.org/?tree=Try&rev=f0059c7b50c8, installer will show up
> at:
> https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/sfoster@mozilla.
> com-f0059c7b50c8/
> 
> ::: browser/metro/theme/circularprogress.css
> @@ +24,2 @@
> >    background-size: 40px 40px;
> > +  background: url(chrome://browser/skin/images/progresscircle-bg.png) no-repeat;
> 
> doesnt using the background shorthand here just reset background-size to
> auto auto?

Oops, right. Btw, I'm trying to use shorthand properties where it's possible, for me it looks more readable even if I need to explicitly set some default values like "top left" in this case: 

background: url(chrome://browser/skin/images/progresscircle-bg.png)
            no-repeat
            top left / 40px 40px;

Do we have any Team wide preferences on that?

> @@ +34,5 @@
> > +
> > +.circularprogressindicator-progressNotification.progressNotification-active {
> > +  visibility: visible;
> > +  opacity: 0;
> > +  transform: scale(2);
> 
> I love it :)

I like it too :) But "visibility: visible" and "opacity: 0" together look kind of awkward and hacky :) 
 
> @@ +35,5 @@
> > +.circularprogressindicator-progressNotification.progressNotification-active {
> > +  visibility: visible;
> > +  opacity: 0;
> > +  transform: scale(2);
> > +  transition: opacity 1s, transform 1s;
> 
> We have a @metro_animation_duration@ we use for a lot of animations, which
> you might try. 1 second looked ok to me though.

Great, thanks! Will definitely use @metro_animation_duration@ and it will look even better.

Comment 4

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Hi Oleg, could you please attach screenshots of the UI you implemented to the bug, including all different states? 

That will help me review your UI request. Thanks!

Comment 5

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

After discussions on IRC, we agreed for bug 960484 we focus applying flash animation to download icon  when nav bar is visible. For v1 we don't notify the users on the individual download when they are on full-screen.

A idea Yuan proposed for v2: 
On full-screen, when a download is completed, download icon jumps up and down like a ball for a few seconds. No need of showing download info bar or nav bar.

Comment 6

[Sam Foster [:sfoster] (he/him)]

To summarize discussion in IRC: for this bug we want to: 

* Flash the download icon in the navbar whenever a download completes (if the navbar is visible)
* Only show the navbar and completed notification when all downloads complete
* Behavior for clicking on the navbar download status button doesn't change

Comment 7

[Oleg Zasypkin [:azasypkin]]

Attachment: patch.diff

Comment 8

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8376184 [details] [diff] [review]
patch.diff

Review of attachment 8376184 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, works nicely.

Comment 9

[Oleg Zasypkin [:azasypkin]]

Thanks for review! Created one more followup to further improve that thing :) - bug 973043.

Also it looks like we can close both bug 967632 and bug 960489 once this patch is landed.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/627fac32b0d5

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/627fac32b0d5

Comment 12

[Marco Mucci [:MarcoM]]

Flagged for testing and verification.  If defects found please reopen.

Comment 13

[Kamil Jozwiak [:kjozwiak]]

Went through the following verification process using the following build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-02-26-03-02-02-mozilla-central/

- Ensured that the animation is triggered when downloading a single file and the navigation app bar is visible
- Ensured that the animations are triggered when downloading multiple files at the same time and the navigation app bar is visible
- Ensured that the download button is visible under about:start and the animations are triggered when there's multiple downloads
- Ensured that once all the downloads are complete, the "download complete app bar" slides into view without any issues
- Ensured that the download button is cleared when "Open" or "Show in folder" is selected
- Ensured that the animation is still triggered if a user hits save after waiting and the download already completed in the background
- Went through all of the above test cases using different variations of snapped view

I found tons of issues with the download process and created the appropriate bugs, blocked Bug #898477.