Update Photon download toolbar icon progressbar to fill horizontally

VERIFIED FIXED in Firefox 56

Status

()

Firefox
Theme
P1
normal
VERIFIED FIXED
2 months ago
13 days 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])

MozReview Requests

()

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

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

2 months 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.

Updated

2 months ago
Whiteboard: [photon-animation] → [photon-animation] [triage]
Comment hidden (mozreview-request)

Updated

2 months ago
Status: NEW → ASSIGNED
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [photon-animation] [triage] → [photon-animation]

Updated

2 months ago
Iteration: --- → 56.1 - Jun 26

Updated

2 months ago
Iteration: 56.1 - Jun 26 → 56.2 - Jul 10
Comment hidden (mozreview-request)
(Assignee)

Updated

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

Updated

2 months ago
Blocks: 1376519
(Assignee)

Comment 3

2 months 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 months 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 months ago
Attachment #8881663 - Attachment is obsolete: true
(Assignee)

Comment 6

2 months 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 months 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 months ago
Pushed by sfoster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2a80b7cda62d
Implement Photon download progressbar r=Paolo

Comment 11

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2a80b7cda62d
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56

Updated

a month ago
Flags: qe-verify? → qe-verify+
QA Contact: jwilliams

Comment 12

a month ago
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

a month 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]

Comment 14

a month ago
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

Updated

a month ago
Flags: qe-verify+

Updated

a month ago
Duplicate of this bug: 1379347

Updated

15 days ago
Depends on: 1386800
(Assignee)

Updated

13 days ago
Blocks: 1387530
(Assignee)

Updated

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