Closed Bug 983681 Opened 10 years ago Closed 10 years ago

Show some content in the downloads panel when it is empty

Categories

(Firefox :: Downloads Panel, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 31

People

(Reporter: jaws, Assigned: jaws)

References

Details

Attachments

(1 file, 2 obsolete files)

From bug 934075, we should add some content to the downloads panel when it is empty.
We could do something similar to about:downloads, maybe reusing the same string too
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Attached patch Patch (obsolete) — Splinter Review
I couldn't use the same string from about:downloads because it would be confusing to users. When the menu panel is blank, it just means that there are no current/recent downloads, whereas when about:downloads is blank it means that there is no history of downloads IIUC.
Attachment #8391312 - Flags: review?(mak77)
The panel originally had this exact string, but it was removed from the UX mockups at some point in favor of just displaying "Show All Downloads". Has this been revisited?
This came up in bug 934075. I think the renewed interest is adding something more useful here actually, like some type of Hint or Tip.
Comment on attachment 8391312 [details] [diff] [review]
Patch

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

::: browser/components/downloads/content/downloadsOverlay.xul
@@ +104,5 @@
>                     onmouseout="DownloadsView.onDownloadMouseOut(event);"
>                     oncontextmenu="DownloadsView.onDownloadContextMenu(event);"
>                     ondragstart="DownloadsView.onDownloadDragStart(event);"/>
>  
> +      <vbox id="emptyDownloads">

please add mousethrough="always", since it's just a placeholder. it won't be useful now but may become in future.

@@ +107,5 @@
>  
> +      <vbox id="emptyDownloads">
> +        <label id="emptyDownloadsLabel"
> +               value="&downloadsPanelEmpty.label;"/>
> +      </vbox>

I'd like this to just "mimic" the same we are doing in the content view (having similar code may help maintenance), may you just use a description inside a flex=1 stack here? Do we need the vbox for something specific?
(see http://mxr.mozilla.org/mozilla-central/source/browser/components/downloads/content/contentAreaDownloadsView.xul)

::: browser/locales/en-US/chrome/browser/downloads/downloads.dtd
@@ +86,5 @@
>  
> +<!-- LOCALIZATION NOTE (downloadsPanelEmpty.label):
> +     This string is shown when there are no items in the Downloads Panel.
> +     -->
> +<!ENTITY downloadsPanelEmpty.label        "There are no current downloads.">

A text simpler to understand for users would be "No downloads for the current session.", since it's hard to describe the concept of a "current download" (let alone translate it).
Now, the fact here is that the panel is small, some translations for this string will be long (in french downloads is téléchargement for example) and will very likely go multiline.

Did you test how this looks when the text goes multiline?
Attachment #8391312 - Flags: review?(mak77)
Attached patch Patch v2 (obsolete) — Splinter Review
Attachment #8391312 - Attachment is obsolete: true
Attachment #8409195 - Flags: review?(mak77)
Attachment #8409195 - Flags: review?(mak77)
Attached patch Patch v2.1Splinter Review
Tested with long strings to make sure that the text wraps. I'm not sure what benefit we would get from using a <stack> here, so I removed it.
Attachment #8409195 - Attachment is obsolete: true
Attachment #8409202 - Flags: review?(mak77)
Comment on attachment 8409202 [details] [diff] [review]
Patch v2.1

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

I think the advantage of a stack would be mostly to take the width of the largest content, though in this case the alternative content is an empty richlistbox, so we may avoid it, I guess.

It looks ok.

::: browser/locales/en-US/chrome/browser/downloads/downloads.dtd
@@ +86,5 @@
>  
> +<!-- LOCALIZATION NOTE (downloadsPanelEmpty.label):
> +     This string is shown when there are no items in the Downloads Panel.
> +     -->
> +<!ENTITY downloadsPanelEmpty.label        "No downloads for the current session.">

or "for this session" if we want to shorten/simplify it even more (I think it would also be easier to translate)
Attachment #8409202 - Flags: review?(mak77) → review+
https://hg.mozilla.org/mozilla-central/rev/2a9a945276ae
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 31
Blocks: 1001324
QA Whiteboard: [good first verify]
You need to log in before you can comment on or make changes to this bug.