Update Photon download toolbar icon progressbar to fill horizontally

VERIFIED FIXED in Firefox 56

Status

()

P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: sfoster, Assigned: sfoster)

Tracking

(Blocks: 2 bugs)

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

Firefox Tracking Flags

(firefox56 verified)

Details

(Whiteboard: [photon-animation])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

2 years ago
Bug 1371834 added the new Photon theme download toolbar icon, but the download progress currently still fills the arrow from bottom->up as before. This needs to match the spec in which the horizontal bar under the arrow in the icon fills from start-end based on %age progress. 

This work was a part of  bug 1352065 but is being broken out into its own sub-task to facilitate incremental development and easier review of smaller steps towards the whole.
Whiteboard: [photon-animation] → [photon-animation] [triage]
Comment hidden (mozreview-request)
Status: NEW → ASSIGNED
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [photon-animation] [triage] → [photon-animation]
Iteration: --- → 56.1 - Jun 26
Iteration: 56.1 - Jun 26 → 56.2 - Jul 10
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8880200 - Attachment is obsolete: true
Attachment #8880200 - Flags: review?(paolo.mozmail)
(Assignee)

Updated

2 years ago
Blocks: 1376519
(Assignee)

Comment 3

2 years ago
I reworked the patch in attachment #8881663 [details] to be based on the refactor in bug 1376515, and to eliminate some of the flicker as the indicator overlay is a loaded and when a download starts. The icon and progressbar are split out into separate elements, this is in part to allow for animations on the arrow part of the icon which will land in bug 1376519. 

One further change here - I added a frame with the progressbar at the same width/fill as the static icon so we can load this single image and avoid some of extra image loading we were seeing previously. This means the progress steps start at -16px.

Comment 4

2 years ago
mozreview-review
Comment on attachment 8881663 [details]
Bug 1375309 - New download progressbar, split out the arrow and progressbar in the indicatorOverlay for individual animation.

https://reviewboard.mozilla.org/r/152830/#review157974

This still flickers when the overlay is loaded.

Since this film strip animation is nothing more than a clip-path, we can use the same technique we use for filling the arrow from bottom to top, but from side to side, with just a single image for the bar instead of a film strip. We could very easily put the standalone bar image in the same file as the bar plus the arrow, without a significant additional performance impact for rendering.
Attachment #8881663 - Flags: review?(paolo.mozmail)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8881663 - Attachment is obsolete: true
(Assignee)

Comment 6

2 years ago
:paolo, I made the changes we discussed to move the icon and progressbar images into a single svg file. 
The markup change is in preparation for bug 1376519 where we need to animate the arrow separately from the progressbar.
Comment hidden (mozreview-request)

Comment 8

2 years ago
mozreview-review
Comment on attachment 8882123 [details]
Bug 1375309 - Implement Photon download progressbar

https://reviewboard.mozilla.org/r/153240/#review158706

I've not tested the non-Photon version, r+ with the changes below assuming the non-Photon version works.

::: browser/themes/shared/downloads/indicator.inc.css:47
(Diff revision 2)
> +%endif
> +#downloads-button[attention="success"] > #downloads-indicator-anchor > #downloads-indicator-progress-outer {
>    fill: var(--toolbarbutton-icon-fill-attention);
>  }
> +%ifdef MOZ_PHOTON_ANIMATIONS
> +#downloads-button[progress]:not([attention]) > #downloads-indicator-anchor > #downloads-indicator-progress-outer {

:not([attention]) is unneeded.

::: browser/themes/shared/toolbarbutton-icons.inc.css:83
(Diff revision 2)
>    list-style-image: url("chrome://browser/skin/history.svg");
>  }
>  
>  #downloads-button[cui-areatype="toolbar"] {
>  %ifdef MOZ_PHOTON_THEME
> -  list-style-image: url("chrome://browser/skin/download-arrow-with-bar.svg");
> +  list-style-image: url("chrome://browser/skin/downloads/download-icons.svg#arrow-with-bar");

download-arrow-with-bar.svg should be removed.
Attachment #8882123 - Flags: review?(paolo.mozmail) → review+
Comment hidden (mozreview-request)

Comment 10

2 years ago
Pushed by sfoster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2a80b7cda62d
Implement Photon download progressbar r=Paolo

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2a80b7cda62d
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Flags: qe-verify? → qe-verify+
QA Contact: jwilliams
I can see this new download progress animation implemented in latest Nightly 56.0a1 in 64-bit Linux.

Build ID 	20170706130309
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0
QA Whiteboard: [bugday-20170705]

Comment 13

2 years ago
I can see this feature "new download progress animation" has been implemented in Latest Nightly 56.0a1  on Windows 10, 64 bit!

Build ID :   20170706060058
User Agent : Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0

[bugday-20170705]
This bug is verified as fixed in both linux(comment 12) and windows(comment 13). So I am marking this as verified fixed.
Status: RESOLVED → VERIFIED
status-firefox56: fixed → verified
Flags: qe-verify+

Updated

2 years ago
Duplicate of this bug: 1379347

Updated

2 years ago
Depends on: 1386800
(Assignee)

Updated

2 years ago
Blocks: 1387530
(Assignee)

Updated

2 years ago
Blocks: 1387557
You need to log in before you can comment on or make changes to this bug.