restructure <download> content for when the progress meter is visible to better use the UI space

NEW
Assigned to

Status

()

--
enhancement
12 years ago
10 years ago

People

(Reporter: piratepenguin, Assigned: piratepenguin)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

12 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a2pre) Gecko/20070102 Minefield/3.0a2pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a2pre) Gecko/20070102 Minefield/3.0a2pre

Currently the progress bar doesn't fill as much of the window as it can, due to it's structure.

The download-downloading and download-paused bindings in chrome://mozapps/content/downloads/download.xml could be better restructured to use the UI space more efficiently.


Reproducible: Always

Steps to Reproduce:
1.
2.
3.
(Assignee)

Comment 1

12 years ago
Created attachment 250155 [details] [diff] [review]
Do the restructuring! v1


it also does some general tidyup.. replacing vars with consts and removing excess trailing whitespace as I went along.
(Assignee)

Comment 2

12 years ago
Created attachment 250156 [details]
Screenshot of restructured elements with the patch.

Note that the Cancel buttons don't line up with the Open/Remove/Retry ones. I don't know how to fix that. It seems to be the basis of our current structure, hopefully we can have it both ways.
(Assignee)

Comment 3

12 years ago
Comment on attachment 250155 [details] [diff] [review]
Do the restructuring! v1

Slightly inconsistent, but tested and working (on GNU/Linux, but I'd expect it to work on all platforms). See screenshot.
Attachment #250155 - Attachment description: Do the restructuring! Not ready for review - slightly inconsistent. → Do the restructuring! v1
(Assignee)

Comment 4

12 years ago
Created attachment 250157 [details] [diff] [review]
as above + missing status on pause state

same patch as above, just with the missing status beside the resume button in the paused state. whoops.
Attachment #250155 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Summary: restructure <download> content for when the progress meter is visible → restructure <download> content for when the progress meter is visible to better use the UI space
(Assignee)

Comment 5

12 years ago
Created attachment 250214 [details] [diff] [review]
Restructures the relevant download manager bindings so that the progress bar fills to the end of the window

(whenever I mentioned inconsistent behaviour before.. that was in error. Cancel links were never lined up with remove/retry links.)

This patch also removes a bit of excess whitespace, and it #ifdef 0's out unused code (the icon animation. To my knowledge, nobody uses it. If it works at all.).

I added style="border: 0" to labels of class text-link where the link is horizontally next to another non-text-link label (whether a spacer separates them or not), because text-links appear to have a 1px border where labels do not, and would therefore otherwise not match up horizontally with the line. I'm not sure if this is ideal - if it hurts themability.

Tested on GNU/Linux - looks the part, but for OS X and Windows, and for different themes, I wouldn't be so sure things will match up due to the border thing I mentioned....

(this theme stuff I'm not more than only a little familiar with. Feel free to fix & pick up the patch if you like.)
Attachment #250157 - Attachment is obsolete: true
Attachment #250214 - Flags: review?(gavin.sharp)
Sorry that it took so long for me to get to this, I really shouldn't have left it in my review queue for so long without commenting. Subsequent iterations should be quicker, I promise.

(In reply to comment #1)
> it also does some general tidyup.. replacing vars with consts and removing
> excess trailing whitespace as I went along.

This "extra cleanup" makes the patch harder to review. I think the s/var/const/ changes are gratuitous, and if you're making many whitespace changes it's a good idea to post a -w patch for review.

It seems like you're making a behavioral change to the "removable" getter, could you explain it? Or perhaps move it over to another bug if it's a bug fix.

I'm not sure if this still applies, hopefully my review delay has not put a damper on your interest in fixing this bug :/.
Assignee: nobody → piratepenguin
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: unspecified → Trunk
Attachment #250214 - Flags: review?(gavin.sharp)
I think this has been fixed for Firefox 3, no?
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.