Open Bug 830182 Opened 7 years ago Updated 5 years ago

cancel button should be aligned with progress bar in Downloads panel

Categories

(Firefox :: Downloads Panel, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

People

(Reporter: heycam, Assigned: mconley)

References

(Blocks 1 open bug)

Details

(Keywords: polish)

Attachments

(7 files, 11 obsolete files)

13.40 KB, image/png
Details
6.44 KB, patch
Details | Diff | Splinter Review
42.62 KB, image/png
Details
36.92 KB, image/png
Details
42.30 KB, image/png
Details
15.89 KB, image/png
Details
72.04 KB, image/png
Details
Attached image screen shot
The cancel button (that looks like an "x") seems misaligned when the progress bar is shown next to it.  They should be vertically aligned IMO.
I noticed this on Saturday, it's the progressbar that is likely not in center. Should also be verified across all themes.
We won't block the feature on this but it's a good polish bug.
Keywords: polish
Blocks: 831772
Assignee: nobody → mconley
Attached patch Patch v1 (obsolete) — Splinter Review
So each download item is a horizontally oriented -moz-box, with the download item icon, a vbox (containing the description, progress bar, and details), and the button stack.

Each item in the -moz-box is correctly centered vertically - but the problem is (as Marco suspected) that the progress bar is not in the center of the vbox. The margin for the download description is pushing it out of the way.

However, instead of removing that margin (which crunches up all of the contents in an ugly way), or adding another margin to the bottom of the details node (which brings the entire vbox up a number of pixels, which looks odd), I propose simply adding a number of pixels to the margin-top of the downloadButton class.

This patch does this. I'll have some screenshots up in a second.
Attached image Patch v1 on Windows 7 (obsolete) —
Attached image Patch v1 on Windows XP (obsolete) —
Attached image Patch v1 on Ubuntu (obsolete) —
Attached patch Patch v1.1 (obsolete) — Splinter Review
Noticed that pinstripe needed a 1px increase on that margin-top when getting the screenshot. No changes to winstripe or gnomestripe.
Attachment #712473 - Attachment is obsolete: true
Attached image Patch v1 on OSX (obsolete) —
Attachment #712480 - Flags: review?(mak77)
(In reply to Mike Conley (:mconley) from comment #2)
> Each item in the -moz-box is correctly centered vertically - but the problem
> is (as Marco suspected) that the progress bar is not in the center of the
> vbox. The margin for the download description is pushing it out of the way.

I'm not sure, but to me the bottom margin on the description looks quite bad (https://bug830182.bugzilla.mozilla.org/attachment.cgi?id=712481), it's like the description is floating and there's no vertical rithm, especially on Mac and Windows.
I'm not saying we should remove all of it, but maybe finding a compromise between that margin and the button top margin would be better.
I think you should ask Stephen first, the review is trivial but the ui-r is not
and maybe there is some more balancing to do, when the progress bar is not shown things "look" aligned
Stephen, could you please verify the vertical rhythm in these screenshots and provide your thoughts?
Flags: needinfo?(shorlander)
Comment on attachment 712480 [details] [diff] [review]
Patch v1.1

clearing until we get definition of how it should appear.
Attachment #712480 - Flags: review?(mak77)
Attached patch WIP Patch v2.0 (obsolete) — Splinter Review
WIP patch that updates OSX theme
Attachment #712474 - Attachment is obsolete: true
Attachment #712475 - Attachment is obsolete: true
Attachment #712477 - Attachment is obsolete: true
Attachment #712480 - Attachment is obsolete: true
Attachment #712481 - Attachment is obsolete: true
Attached patch WIP Patch v2.1 (obsolete) — Splinter Review
Further progress...
Attachment #719141 - Attachment is obsolete: true
Attached patch WIP Patch v2.2 (obsolete) — Splinter Review
Changes for Windows
Attachment #719187 - Attachment is obsolete: true
Ok, and Linux is done.

This patch will only apply to mozilla-central. I'll have to backport for mozilla-aurora and mozilla-beta (if we want to take this patch there) due to the theme name changes.

I'll have some screenshots up in the next few hours.
Attachment #719499 - Attachment is obsolete: true
Attached image Patch v2 on OSX (obsolete) —
Attached image Patch v2 on Ubuntu
Attached image Patch v2 on OSX
Showing more state variety in this screenshot.
Attachment #719674 - Attachment is obsolete: true
Comment on attachment 719537 [details] [diff] [review]
Patch v2.3 (for mozilla-central)

I've been staring at these panels for too long, and I think I need a second set of eyes. Check out the screenshots - am I acceptably close? (The alignment spec only took into account OSX, but I did my best to adapt the values for the other platforms).
Attachment #719537 - Flags: ui-review?(shorlander)
I messed up the previous version. Fixed version.
Attachment #719024 - Attachment is obsolete: true
Comment on attachment 719537 [details] [diff] [review]
Patch v2.3 (for mozilla-central)

Cancelling review for now.
Attachment #719537 - Flags: ui-review?(shorlander)
As my first bug to start up with would u please assign me this bug.
You need to log in before you can comment on or make changes to this bug.