1624214 - Make the download page more themeable

Status: RESOLVED FIXED

(Thunderbird :: Theme, task)

Description

[Richard Marti (:Paenglab)]

With the themeableDialog.css we can make the download page (opened by CTRL+j) themeable.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: 1624214-themeable-downloads.patch

The top part of the download page looks now like a toolbar. I also needed this approach because the image of LW-themes would bleed into the area around the richlistbox. On Mac a search glass was used to open the Finder. Now the same icon as on Linux/Windows are used.

I changed also that the richlistitems content is automatically centred and we no more use margin-top styles to manually center the content.

I also removed the sender code as I've never seen something in this element.

Comment 2

[Alessandro Castellani [:aleca]]

Comment on attachment 9134967 [details] [diff] [review]
1624214-themeable-downloads.patch

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

Great work, it already looks a million times better.
Some notes and changes listed below.

Lets update the `.size, .startDate` CSS to have a margin-block: 0 to create a better visual separation between the title and those smaller info.

Is there any chance to put the AppMenu to the right of the search box?
I don't know if it makes total sense in terms of context, but since the `#downloadTopBox` is styled to look like a toolbar, having the AppMenu might make sense.
On the other hand tho, this is not a customizable toolbar and we might not want to encourage users to interact with any other element in this page other than the downloads.
What do you think?

We could also use the occasion to restructure a bit this list and have a slightly better organized content.
What do you think about something like this?

--------------------------------------------------------------
               Filename                                         size
[image]                                                                    [folder]
               Sender name                                 date
--------------------------------------------------------------

or maybe

--------------------------------------------------------------
               Filename - size
[image]                                                                    [folder]
               Sender name - date
--------------------------------------------------------------

::: mail/components/downloads/content/aboutDownloads.js
@@ -183,5 @@
>    this._download = aDownload;
>    this._updateFromDownload();
>  
> -  if (aDownload._unknownProperties && aDownload._unknownProperties.sender) {
> -    this._sender = aDownload._unknownProperties.sender;

I'm not sure about this as I think showing the sender of the downloaded file is a pretty useful info.
Do you think we can try to figure out why it never worked?

@@ +246,5 @@
>      image.setAttribute("validate", "always");
>      image.classList.add("fileTypeIcon");
>  
>      let vbox = document.createXULElement("vbox");
>      vbox.setAttribute("pack", "center");

Since you're setting display: flex in CSS, the pack center attribute is not necessary.

::: mail/themes/shared/mail/aboutDownloads.css
@@ +57,5 @@
>  }
>  
> +#msgDownloadsRichListBox > .download {
> +  min-height: 6em;
> +  border-bottom: 1px solid hsla(0, 0%, 50%, .3);

Since we're repeating hsla(0, 0%, 50%, .3) a couple of times, could we maybe convert it into a variable?
Also, is there any current variable we could use for these borders without defining a new one?

Comment 3

[Richard Marti (:Paenglab)]

Do you see somewhere a sender name? Surely not with my patch as I removed it.

Comment 4

[Alessandro Castellani [:aleca]]

No, I've never seen it, with or without your patch, that's why I asked if you think we can troubleshoot it and make it work.
If it was coded it means that at some point it worked.

Comment 5

[Richard Marti (:Paenglab)]

Attachment: 1624214-themeable-downloads.patch

Like you wrote, this isn't a customizable toolbar and not really one that needs much AppMenu interaction. Then we should add the AppMenu on other tabs like the browser tab (opened for example from ATN) because they also look a bit like a toolbar.

How about this patch? I moved the folder button on the right of the image to make the mouse movement shorter, especially on wide screens. I don't think we should put the folder button below the image because this makes the richlistiten unneeded taller. We have enough horizontal space.

The informations is now on two lines like you proposed. The file name is a bit fatter than the others.

I re-added the sender code again. I'll try to find what's missing or when I don't find it I'll create a new bug for better experienced people.

Comment 6

[Richard Marti (:Paenglab)]

Attachment: richlistitem.png

Screenshot how it looks on Windows.

Comment 7

[Alessandro Castellani [:aleca]]

Comment on attachment 9135195 [details] [diff] [review]
1624214-themeable-downloads.patch

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

Sweet, this looks good, thanks.

Comment 8

[Richard Marti (:Paenglab)]

Filed bug 1624632 for the sender field. I failed to find why it is not filled.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e512016183ac
Make the download page more themeable. r=aleca