Closed Bug 1710918 Opened 3 years ago Closed 3 years ago

Update the styling of the downloads panel progress bar

Categories

(Firefox :: Downloads Panel, enhancement, P3)

Desktop
All
enhancement
Points:
3

Tracking

()

VERIFIED FIXED
93 Branch
Tracking Status
firefox93 --- verified

People

(Reporter: sfoster, Assigned: sfoster)

References

(Blocks 1 open bug)

Details

(Whiteboard: [proton-cleanups])

Attachments

(1 file)

We have a new design for this progressbar that adopts some of the Proton colors and themes. This bug will track implementing those.

Severity: -- → N/A
Type: defect → enhancement
OS: Unspecified → All
Hardware: Unspecified → Desktop
Priority: -- → P3
Points: --- → 3
Assignee: nobody → sfoster
Status: NEW → ASSIGNED
Attachment #9236081 - Attachment description: WIP: Bug 1710918 - Update download panel progressbar styling → Bug 1710918 - Update download panel progressbar styling. r?harry

I've got a try build with the current patch:

It rounds the progressbar in the download panel and updates the colors used for the known-size downloads and indeterminate-size downloads.

I'm using the same attention color variable we use to show the download progress in the toolbarbutton download indicator as the fill color for the progress bar in the download panel. That seems to be the main bug we want to fix here - the mismatch between the current download panel's progressbar and the one we normally see right above it in the toolbarbutton. The progressbar fill is actually a gradient, from #9059FF to that attention color. It switches to the solid attention color in the last couple of % of progress.

While the first purple is hard-coded, the attention color does vary depending on your theme. In dark theme, its a lighter cyan for example. We're using the same purple elsewhere e.g. in about:protections, and in the zap gradient, but its not a part of any theme so while IMO it looks ok with the build in themes, it could look strange with custom themes.

We have a similar download progressbar in about:downloads, and in the places/library view (library > downloads). I looked at those, but didn't include a fix for them in this patch - they don't have access to the attention color variable we use so its need a little thought and probably its own bug.

Amy, would you be able to take this for a spin and provide some feedback. Or redirect to whoever owns this now?

Flags: needinfo?(amlee)

Redirecting to katiec

Flags: needinfo?(amlee) → needinfo?(kcaldwell)

Moving my NI to Janice.

Flags: needinfo?(kcaldwell) → needinfo?(jcramer)

The MR1 designers intended to have gradients for both 1) the download progress bars and 2) the download icon's progress animation—however, this was out of scope.

Currently, the gradient is implemented in the progress bar within the downloads panel but not within the library or the download icon.

To achieve uniformity, the gradients should be removed for now from the downloads panel.

Katie noted that the solid cyan/gray bar within dark theme may not be accessible due to contrast. I will check the contrast and circle back if we need to change either of those colors.

Flags: needinfo?(jcramer)

Sam, I misunderstood—see below:

(In reply to Janice Cramer from comment #6)

The MR1 designers intended to have gradients for both 1) the download progress bars and 2) the download icon's progress animation—however, this was out of scope.

Currently, the gradient is implemented in the progress bar within the downloads panel but not within the library or the download icon.

To achieve uniformity, the gradients should be removed for now from the downloads panel.

Katie's clarified that if you've implemented gradients, to leave them.

Katie noted that the solid cyan/gray bar within dark theme may not be accessible due to contrast. I will check the contrast and circle back if we need to change either of those colors.

Here's what I know right now:

  • The library downloads view and about:downloads do not get the current theme applied. Though they do have some simple light vs. dark color scheme CSS rules. In those views we also have a selected state for the focused list item which reverses the light on dark / dark on light. It means that while using a translucent color the progressbar background (the "tray") seems to work well, I'll probably need 2 separate fill colors for the selected and not-selected state.
  • The download panel doesnt have a selected state - only a hover state. So this is not a concern there.
  • In the toolbar download indicator and the download panel, the progress fill color comes from the theme's icons_attention color property. That color is used to define the --toolbarbutton-icon-fill-attention CSS variable which gives use the blue/teal pie (for light/dark theme variations)
  • The purple color in the gradient from the mockups is not part of any theme. I can arrange things so non-built in themes just get a solid color. For our themes, we can define a "flare"/"highlight" color that would complement progress fill color. Flare colors that aren't purple are not yet defined...
  • Making all the downloads views fully consistent with each other is probably outside the scope of this bug. And with the need to support a selected state, there's more to define and implement here. I can make some simple adjustments to the progressbar shape (rounded ends) etc. for now and revisit the other details later.

So, after poking at this a bit, I think it might be best to put aside the gradient for now. When we implement relative colors it should be possible to have a pretty generic rule or two to define this "flare" color for whatever the accent/attention color happens to be, in the current theme, state and mode.

Until then I either have to get specs for this color in each theme, each state (selected, not selected) and each mode (dark/light) - or - use a calculated hsl color, which would require breaking out the relevant colors into separate hue and luminosity variables so we can calc() a relative hsl color from them. I started prototyping this approach in this jsfiddle fwiw.

Hard-coding the necessary extra colors into our built-in themes is practical, using theme experiment variables, e.g this appmenu_warning_icon_color definition but seems a relatively low priority detail that has more open questions before we can implement.

(In reply to Sam Foster [:sfoster] (he/him) from comment #10)

So, after poking at this a bit, I think it might be best to put aside the gradient for now. When we implement relative colors it should be possible to have a pretty generic rule or two to define this "flare" color for whatever the accent/attention color happens to be, in the current theme, state and mode.

Until then I either have to get specs for this color in each theme, each state (selected, not selected) and each mode (dark/light) - or - use a calculated hsl color, which would require breaking out the relevant colors into separate hue and luminosity variables so we can calc() a relative hsl color from them. I started prototyping this approach in this jsfiddle fwiw.

Hard-coding the necessary extra colors into our built-in themes is practical, using theme experiment variables, e.g this appmenu_warning_icon_color definition but seems a relatively low priority detail that has more open questions before we can implement.

Sam, thank you so much for all of this info and for the due diligence in exploring gradient execution.

To clarify, when you say "put aside the gradient for now," do you mean to remove the existing gradients so that we can re-address this in the future according to your notes above?

QA Note: My patch had a conflict with the one from Bug 1722582. The new changes supercede those: I've explicitly addressed the invisible progress bar in selected items, and the new style means we no longer have the bar being taller than the element its contained in. But please check the STR on there.

The spec shows a purple highlight color in the progress bar. That has not been implemented here. We'll want to add that in another bug where we can get consistency across the different progress views and themes.

While this bug is primarily about the downloads panel, it also updates progress bar styling in the other downloads views:

  • Downloads panel
  • Library > Downloads
  • about:downloads (all downloads view from a private window)

We'll need to check determinate/known-size downloads and indeterminate (unknown size) for each of views in each of the built-in themes as well as high-contrast mode. about:downloads and the library downloads view allow you to select a list item, which inverts the colors so that's good to check too.

Blocks: 1722582
Pushed by sfoster@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cb24192713ff Update download panel progressbar styling. r=harry,desktop-theme-reviewers
Backout by ccozmuta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b50d04b3a0a5 Backed out changeset cb24192713ff for causing bc failures on browser_parsable_css.js. CLOSED TREE

Thanks.
I looks like that test examines each rule looking for un-used variables, and .cssText is kinda busted on shorthand properties like background:. I've split it out into background-color and background-image properties, and this seems to make it happy (try).

Flags: needinfo?(sfoster)
Pushed by sfoster@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ab5b37f83023 Update download panel progressbar styling. r=harry,desktop-theme-reviewers
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 93 Branch
Regressions: 1728636

We still have this issue on Linux:

If there is no selected item in the downloads list and I move the window then the first item gets selected again but that should not happen. If there is a selected item then during window movement the selection color changes and the dotted outline is removed which is weird and should not happen too. If there is no selected item but dotted outline then this line will disappear during window movement.

Looks like it's normal that selection color changes during window movement. But the first sentence I wrote in my last comment still looks like a bug.

The new styling has been verified as part of the QA-1075 work.
Marking the issue as verified fixed.

Please note that the download panel will also be covered in the upcoming monochromatic theme work (QA-1135) with all the new themes.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: