Visual for download animation on download start whenever a download is started

VERIFIED FIXED in Firefox 54

Status

()

Firefox
Downloads Panel
VERIFIED FIXED
a year ago
3 months ago

People

(Reporter: yifan, Assigned: rexboy)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 54
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 verified)

Details

(Whiteboard: [CHE-MVP])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

a year ago
Add an animation to notify a user whenever a download is completed while a user is downloading multiple files.
(Reporter)

Comment 1

a year ago
Created attachment 8748517 [details]
Download Panel and Content Handler.pdf

Comment 2

a year ago
There is already an animation, and the design pdf does not show a design for a different animation. What is the goal of this bug?
Flags: needinfo?(yliao)

Updated

a year ago
Component: General → Downloads Panel
(In reply to :Gijs Kruitbosch from comment #2)
> There is already an animation, and the design pdf does not show a design for
> a different animation. What is the goal of this bug?

There will be a redefined animation per plan and Visual team is working on it. We should put that Visual spec (different from current UX one) here then, or at least clearly indicate where it is, for reference.

Thanks for point it out, Gijs! And let me know if there's any question.
Flags: needinfo?(yliao)
Comment on attachment 8748517 [details]
Download Panel and Content Handler.pdf

should refer to its parent bug 1269956[1] for the latest UX spec. 

https://bugzilla.mozilla.org/show_bug.cgi?id=1269956
Attachment #8748517 - Attachment is obsolete: true
(Assignee)

Updated

a year ago
Assignee: nobody → rexboy

Updated

11 months ago
Whiteboard: [CHE-MVP]
(Assignee)

Updated

10 months ago
Depends on: 1296554
(Assignee)

Updated

10 months ago
No longer depends on: 1296554

Comment 5

10 months ago
Created attachment 8789741 [details]
download_motion.gif

the attachment is the download animation mock up.
(Assignee)

Comment 6

10 months ago
Just figured out the required work remaining should be a "jumping" animation to the download button when a download starts. There's no changed needed for the complete animation. Change the title accordingly.
Summary: Show download complete animation on downloads button whenever a download is completed → Visual for download animation on download start whenever a download is started

Comment 7

6 months ago
As a reference, this bug will change only 1 UX item - start downloading to trigger jumping twice.
Comment hidden (mozreview-request)
(Assignee)

Comment 9

6 months ago
This is a simple patch that adds a bouncing animation to the arrow in toolbar when download starts.

(It should be land after bug 1270006. Now it bounces both the progressbar and the arrow. This behavior won't be the case anymore after bug 1270006 lands, which leaves just the arrow.)

Comment 10

6 months ago
Is this meant to be in addition to the fading/shrinking animation we already have, or is this a replacement?

Comment 11

6 months ago
Comment on attachment 8820202 [details]
Bug 1270012 - Show a bouncing animation for toolbar download icon whenever a download is started.

Looks good to me! Just waiting for design feedback on the starting animation.
Attachment #8820202 - Flags: review?(paolo.mozmail) → feedback+
(Assignee)

Comment 12

6 months ago
Carol confirmed the animation for the patch (on Mac and Windows) so I think that should be OK for them. But you can double confirm if you have any concern.

Comment 13

6 months ago
mozreview-review
Comment on attachment 8820202 [details]
Bug 1270012 - Show a bouncing animation for toolbar download icon whenever a download is started.

https://reviewboard.mozilla.org/r/99734/#review102190

The code looks good, just a small change needed. Thanks for creating the "indicator.inc.css" file!

Please show the animation to Carol and confirm the exact timing, it seems slow to me but if this is the intended effect then it's all good.

::: browser/components/downloads/content/indicator.js:366
(Diff revision 1)
>      notifier.setAttribute("notification", aType);
>      this._notificationTimeout = setTimeout(() => {
>        notifier.removeAttribute("notification");
>        notifier.style.transform = '';
>      }, 1000);
> +
> +    anchor.setAttribute("notification", aType);
> +    this._anchorNotificationTimeout = setTimeout(() => {
> +      anchor.removeAttribute("notification");
> +    }, 3000);

In practice the timeout determines the minimum interval between animations, so if a new event occurs within the timeout, an animation for it may not be shown. It's required because our CSS animations are triggered by the attribute changes.

I think you can simplify the code by keeping only one timeout and making it longer to include both animations.

By reading the CSS it seems to me that the longest animation is 2 seconds from when the attribute is set, so 2000 could be a good value to use here, or higher if you don't want two consecutive animations to occur.

It may be a good idea to add a comment here saying this value is based on the length of the animation in the CSS, and a comment in the CSS that reminds to update this value if the animation length changes.
Comment hidden (mozreview-request)
(Assignee)

Comment 15

6 months ago
Double confirmed the animation with visual. It seems the speed is good.

If we land this bug alone it ends up animating the progressbar, so we may need to land it together with bug 1270006 if we think that matters.

Comment 16

6 months ago
mozreview-review
Comment on attachment 8820202 [details]
Bug 1270012 - Show a bouncing animation for toolbar download icon whenever a download is started.

https://reviewboard.mozilla.org/r/99734/#review103804

::: browser/components/downloads/content/indicator.js:287
(Diff revision 2)
> +  _anchorNotificationTimeout: null,
> +

Remove this...

::: browser/components/downloads/content/indicator.js:345
(Diff revision 2)
> +    if (this._anchorNotificationTimeout) {
> +      clearTimeout(this._anchorNotificationTimeout);
> +    }
> +

...and this. After doing that check that you don't leave any unnecessary whitespace changes.

Looks good for the rest!
Attachment #8820202 - Flags: review?(paolo.mozmail) → review+

Comment 17

6 months ago
(In reply to KM Lee [:rexboy] from comment #15)
> If we land this bug alone it ends up animating the progressbar, so we may
> need to land it together with bug 1270006 if we think that matters.

I'm fine either way, feel free to land when you think is best. Thanks!
(Assignee)

Comment 18

5 months ago
Just discussed with Carol.
If we land this alone, it turns out that the old-styled progressbar is animated to jump twice, which doesn't look ok per visual's point of view. So let's land it when 1270006 is ready.
(Assignee)

Updated

4 months ago
QA Whiteboard: checkin-needed
(Assignee)

Updated

4 months ago
QA Whiteboard: checkin-needed
Keywords: checkin-needed
(Assignee)

Updated

4 months ago
Keywords: checkin-needed
Comment hidden (mozreview-request)
(Assignee)

Comment 20

4 months ago
Since bug 1270006 has been pushed, Let's checkin this bug too.
Keywords: checkin-needed

Comment 21

4 months ago
You'll likely need to rebase this patch, since bug 1270006 added "browser/themes/shared/downloads/indicator.inc.css" already.
Keywords: checkin-needed

Comment 22

4 months ago
Pushed by paolo.mozmail@amadzone.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eefb0717c7cb
Show a bouncing animation for toolbar download icon whenever a download is started. r=paolo
(Assignee)

Comment 23

4 months ago
Oh I'm sorry for the late reply. I was absent on Friday.
Thanks for pushing it for me!

I'll take care if there are any actions needed.

Comment 24

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/eefb0717c7cb
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54

Updated

3 months ago
Depends on: 1349096
I can confirm this issue is fixed, I verified Fx 54.0a2, build ID:20170323004002 on Windows 10 x64, Mac OS X 10.11.6 and Ubuntu 14.04.

Cheers!
Status: RESOLVED → VERIFIED
status-firefox54: fixed → verified
You need to log in before you can comment on or make changes to this bug.