458199 - Polish the download manager on Mac OS X

Status: VERIFIED FIXED

(Toolkit :: Themes, defect)

Description

[Markus Stange [:mstange]]

Attachment: Screenshots

See screenshot:
 - Gradient on selected item (changes according to blue / graphite theme,
   focused / unfocused list and active / inactive window)
 - Smaller font for the file name
 - text-shadow on selected item
 - Minor padding fix

Comment 1

[Markus Stange [:mstange]]

Attachment: patch v1

Comment 2

[Markus Stange [:mstange]]

The richlist in the "Settings" pane of Terminal's preferences window also uses these gradients.

Comment 3

[Alex Faaborg [:faaborg] (Firefox UX)]

Comment on attachment 341429 [details]
Screenshots

Personally I think this looks great, but I want Stephen Horlander or Kevin Gerich to also weigh in since they drove the graphic design work for Firelight.

Comment 4

[Alex Faaborg [:faaborg] (Firefox UX)]

Stephen and Kevin: what do you think of the change?

Comment 5

[Kevin Gerich]

It's a very nice look, but I think the gradient adds unnecessary emphasis to the selection. 

The extra emphasis seems appropriate when you're making a selection which affects the content of the window, such as the Terminal theme picker that Markus mentioned, or the account picker in System Preferences/Accounts. But it doesn't make sense to me as a more global style.

Comment 6

[Alex Faaborg [:faaborg] (Firefox UX)]

>The extra emphasis seems appropriate when you're making a selection which
>affects the content of the window, such as the Terminal theme picker that
>Markus mentioned, or the account picker in System Preferences/Accounts. But it
>doesn't make sense to me as a more global style.

That's an interesting distinction.  What about using these gradients on the location bar autocomplete?  In that case there is a need to give visual emphasize to the selected item in the richlist view, and there is a less of a sense of platform unity since this type of auto complete is unique to Firefox.

Other richlist views to consider are the addons manager, and the error console.

Comment 7

[Markus Stange [:mstange]]

(In reply to comment #5)
> The extra emphasis seems appropriate when you're making a selection which
> affects the content of the window, such as the Terminal theme picker that
> Markus mentioned, or the account picker in System Preferences/Accounts. But it
> doesn't make sense to me as a more global style.

I haven't thought of this before - it really is an interesting distinction.
I looked through different Mac apps to see if other apps make this distinction for richlists, but the only non-window-affecting richlists I've found are Safari's and Adium's downloads lists. Adium uses a gradient, Safari doesn't.
Kevin, are there other examples of richlists without gradients in OS X?

Personally I think the nice look of the gradient is worth the unnecessary emphasis :-).

(In reply to comment #6)
> What about using these gradients on the
> location bar autocomplete?  In that case there is a need to give visual
> emphasize to the selected item in the richlist view, and there is a less of a
> sense of platform unity since this type of auto complete is unique to Firefox.

For some reason I wouldn't use it there. I think it's because the gradients add some kind of "stability" to the look that's inappropriate for something as transient as the autocomplete popup. It's worth a try, though.

> Other richlist views to consider are the addons manager, and the error
> console.

I'm definitely in favor of using gradients there, although these lists have the "unnecessary emphasis" problem Kevin mentioned, too.

Comment 8

[Alex Faaborg [:faaborg] (Firefox UX)]

>other examples of richlists without gradients in OS X?

Perhaps an important consideration is the appearance of the different file system views in the Finder.  The download manager is meant to be file system-ish in that it is a list of files, and you double click an item to open it.  The content area of the finder doesn't use gradients, (although the window controlling source view on the left does).

People probably won't be too focused on this similarity since users rarely use the larger icon size in the Finder's list view, but figured I would mention it in case it sways people's opinions.

>Personally I think the nice look of the gradient is worth the unnecessary
>emphasis :-).

Overall quality of appearance is an important consideration, even in cases where there might be a slight downside to external consistency or usability.

Stephen and Kevin should make the final call since they have done so much work to create Firelight.

Comment 9

[Mike Connor [:mconnor]]

Comment on attachment 341430 [details] [diff] [review]
patch v1

Kevin, this is your call.

Comment 10

[Markus Stange [:mstange]]

Kevin, is this on your radar?

Comment 11

[Markus Stange [:mstange]]

Attachment: v2

Comment 12

[Alex Faaborg [:faaborg] (Firefox UX)]

If the patch passes review, let's go ahead and take this, it really looks nice.

Comment 13

[Dão Gottwald [:dao]]

Are there plans to use the images for other (rich)listboxes? If not, they should probably be in pinstripe/mozapps/downloads/.

Comment 14

[Markus Stange [:mstange]]

Attachment: v3

I don't have such plans, so let's move them to downloads for now.

Comment 15

[Markus Stange [:mstange]]

oh, maybe I should update the urls in the CSS, too

Comment 16

[Markus Stange [:mstange]]

Attachment: v3.1

Comment 17

[Markus Stange [:mstange]]

Attachment: 191 version, with button polish

This also includes some changes to the "Clear List" button that bug 477827 added on trunk.

Comment 18

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 380604 [details] [diff] [review]
191 version, with button polish

Are we sure the button changes won't cause theme compatibility issues?

Comment 19

[Dão Gottwald [:dao]]

This is theme-only code -- it won't be used at all when another theme is selected.

Comment 20

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 380604 [details] [diff] [review]
191 version, with button polish

a191=beltzner

Comment 21

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/f014c39be5dc

Comment 22

[Markus Stange [:mstange]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/eae597c1e498

Comment 23

[Shawn Wilsher :sdwilsh]

This patch means we no longer respect the system selection color.  I've filed bug 495686 about this.

Comment 24

[Markus Stange [:mstange]]

Backed out.

Comment 25

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #23)
> This patch means we no longer respect the system selection color.  I've filed
> bug 495686 about this.

Did we respect that before? Does Safari? Is that the right thing to be respecting for complex listboxes like this?

Those are the questions that led to my decision to back-out and re-open. Need to make sure we've considered them before re-landing.

Comment 26

[Mike Beltzner [:beltzner, not reading bugmail]]

Shawn says that we did, indeed, used to respect the selection colour, as does Safari.

Comment 27

[Dão Gottwald [:dao]]

Comment on attachment 341429 [details]
Screenshots

I think it's fine to use the blue / graphite appearance colors instead of the selection color. It's suboptimal to hardcode colors, but that's not a rarity in pinstripe, and effectively it doesn't seem to reduce the nativeness. But then I don't use OS X.

Let's get this sorted out with another ui-review pass?

Comment 28

[Dão Gottwald [:dao]]

(In reply to comment #26)
> Shawn says that we did, indeed, used to respect the selection colour, as does
> Safari.

It should also be noted that Safari doesn't use gradients, as mentioned in comment 7. We seemed to be fine with not following Safari in that regard.

Comment 29

[Stefan [:stefanh]]

Looking at Safari (and Finder), I think those gradients are normally only used in source lists.

Comment 30

[Alex Faaborg [:faaborg] (Firefox UX)]

Comment on attachment 341429 [details]
Screenshots

It's not the literal selection color, but definitely in the spirit of the aqua / graphite selection color

Comment 31

[Shawn Wilsher :sdwilsh]

I don't feel like we should be taking this for 1.9.1 - it is a regression from Firefox 3.0 and I think we should get more user feedback on trunk before we consider taking it there (although I really like the way this looks compared to before).  Maybe on branch we should only take the changes that don't cause a regression from 3.0.

Comment 32

[Markus Stange [:mstange]]

(In reply to comment #30)
> (From update of attachment 341429 [details])
> It's not the literal selection color, but definitely in the spirit of the aqua
> / graphite selection color

Just to prevent a potential misunderstanding:
"Aqua / graphite" and "selection color" are two different things. They're separate options in the system appearance settings.

Comment 33

[Markus Stange [:mstange]]

Attachment: simpler 1.9.1 version

This patch only contains the font size, alignment and button changes; no gradients. Do we want it?

Comment 34

[Markus Stange [:mstange]]

Attachment: screenshot of "simpler 191 version"

This is what the latest patch looks like with purple selection color.

Comment 35

[Shawn Wilsher :sdwilsh]

(In reply to comment #34)
> This is what the latest patch looks like with purple selection color.
This is leaps and bounds better than what we currently have on 1.9.1.

Comment 36

[Alex Faaborg [:faaborg] (Firefox UX)]

I feel like we are being overly concerned with the extreme minority use case at the cost of not deploying something that makes the vast majority use case more polished.  Most of our users won't even know that an alternate selection color is possible.

Comment 37

[Mike Beltzner [:beltzner, not reading bugmail]]

The question is not whether or not we're being overly concerned, IMO, but *when* we are deciding to make these decisions. If you ask me to make up my mind on this sort of thing under the gun, without proper time or analysis or thought to the issue, then I'm going to say "do the least risky thing."

That's precisely what's happening here. There's no real agreement in this bug, the Firelight theme creators weren't thrilled with it in comment 5, and I don't see any reason why we are so sure of this change that we want to churn it now.

The smaller patch is fine, if it gets reviewed and tested.

a191- on the gradients as per this comment.

Comment 38

[Shawn Wilsher :sdwilsh]

Comment on attachment 380905 [details] [diff] [review]
simpler 1.9.1 version

>+:root:not([active]) #clearListButton {
>+  color: #7C7C7C !important; /* remove this when we support click-through */
Can we get a bug number added to this comment please.

r=sdwilsh with that change.

Comment 39

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 380905 [details] [diff] [review]
simpler 1.9.1 version

a191=beltzner

Comment 40

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/4430cae50dad

Comment 41

[Markus Stange [:mstange]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9d187f786ebb

Comment 42

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified fixed on trunk and 1.9.1 branch:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090711 Minefield/3.6a1pre ID:20090711031617

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5