613358 - Polish the mailNews search dialog on Mac and also fix some obsolete styles (all OS)

Status: RESOLVED FIXED

Description

[Stefan [:stefanh]]

No point of filing 2 bugs ;-)

The search dialog needs some polish on Mac (css file needs to be forked):
- The +/- buttons should not be push buttons, they should rather look like gradient buttons
- I don't think the listrows should be highlighted when selected (that would have been OK if it was one remove button, though)
- The resultstree lacks borders

We also have a bunch have bunch of obsolete/wrong style rules:
#dirtree is "plain", so we don't need to remove borders etc, resultsTree should be abResultsTree (win/mac and Modern), we also don't seem to have any .tree-container-treerows anymore and we don't need to import dialog.css.

Comment 1

[Stefan [:stefanh]]

Created attachment 491678 [details] [diff] [review]
v1.0

Btw selectAddressesDialog.css contains this rule:
 #resultsBox {
   border: 1px solid #000000;
 }

Does that looks nice on win/nix (another bug, just asking)?

Comment 2

[Stefan [:stefanh]]

Created attachment 491679 [details]
Screenshot of dialog after/before

Comment 3

[Stefan [:stefanh]]

Btw, Karsten: I've tried to mimic the +/- buttons in the OS prefs (Language/text). No bold label text, though.

Comment 4

[Stefan [:stefanh]]

JFTR, I have some slight modifications of the .small-button's coming up in the next days.

Comment 5

[Stefan [:stefanh]]

Created attachment 492189 [details] [diff] [review]
v1.1 - Slightly adjusted version

OK, so I made some adjustments to the buttons (mac default). That includes adjusting the width of the remove button. The "native" ones have an extra top/bottom border, but the visual effect is nearly the same (makes it look less high than wide) and I can't get the same effect anyway since the "content" background is of a different color. Note that the label text isn't 100% horizontally centered, but I don't think it's noticeable.

I did some changes to the widths of the menulists in the dialog (mac default):
+menulist {
+  width: 18em;
+}

That is for the abPopup and the searchableFolders popups. I figured they could be a bit wider so we at least can see the whole labels of the included addressbooks in en-US.

I then overrule the above rule with this, to keep the width of the search condition menulists:

+.search-menulist {
+  width: 12em;
 }

Comment 6

[Stefan [:stefanh]]

Btw, the filter dialog has the same issue and the css also contains obsolete rules. I'll file a separate bug for that.

Comment 7

[Karsten Düsterloh]

Comment on attachment 492189 [details] [diff] [review]
v1.1 - Slightly adjusted version

> - The +/- buttons should not be push buttons, they should rather look like
> gradient buttons

If I tab through the widget items, the buttons now don't give visual feedback anymore where the keyboard focus is. This is bad.

Comment 8

[[e-mail]]

Comment on attachment 492189 [details] [diff] [review]
v1.1 - Slightly adjusted version

>-/* This should not be needed on trunk once Mac styles are moved to button.css */
I wonder whether I ever intended to land bug 520371 on the 1.9.1 branch.
I guess it's a bit too late for that now ;-)

Comment 9

[Stefan [:stefanh]]

(In reply to comment #7)
> If I tab through the widget items, the buttons now don't give visual feedback
> anymore where the keyboard focus is. This is bad.

Yeah, missed that. It's certainly doable, though - will look at it.

Comment 10

[Stefan [:stefanh]]

Created attachment 492754 [details] [diff] [review]
Now with focus indication

OK, so I spent the whole evening playing with colors since the -moz-mac-focusring color looks awful in the aqua color scheme. I think it's ok in graphite, and all the darker colors I tried made the corners look strange (there where a 1px darker spot in them).

Comment 11

[Stefan [:stefanh]]

(In reply to comment #6)
> Btw, the filter dialog has the same issue and the css also contains obsolete
> rules. I'll file a separate bug for that.

Filed bug 614382.

Comment 12

[Karsten Düsterloh]

Comment on attachment 492754 [details] [diff] [review]
Now with focus indication

>diff --git a/suite/themes/classic/messenger/searchDialog.css b/suite/themes/classic/mac/messenger/searchDialog.css
>copy from suite/themes/classic/messenger/searchDialog.css
>copy to suite/themes/classic/mac/messenger/searchDialog.css
>--- a/suite/themes/classic/messenger/searchDialog.css
>+++ b/suite/themes/classic/mac/messenger/searchDialog.css

I wish Bugzilla's visual diff would understand that... :-(

>+.small-button:focus {
>+  box-shadow: 0 0 3px #457FDE,
>+              inset 0 0 2px #457FDE;
> }

Hm, this colour doesn't fit in - and actually, the -moz-mac-focusring colour is a much better fit?
Like

.small-button:focus {
  box-shadow: 0 0 3px -moz-mac-focusring,
              inset 0 0 2px -moz-mac-focusring;
}

Comment 13

[Stefan [:stefanh]]

You mean that you don't think the blue -moz-mac-focusring have a different tone than the one in the focusring around the adjacent search field?

Comment 14

[Stefan [:stefanh]]

Created attachment 492836 [details]
custom

Comment 15

[Stefan [:stefanh]]

Created attachment 492837 [details]
-moz-mac-focusring

Comment 16

[Stefan [:stefanh]]

it's not the end of the world, but i think the custom one looks better.

Comment 17

[Stefan [:stefanh]]

pick the one you like!

Comment 18

[Karsten Düsterloh]

Comment on attachment 492754 [details] [diff] [review]
Now with focus indication

Minussing due to the ongoing colour discussion. 

>+.small-button:focus {
>+  box-shadow: 0 0 3px #457FDE,
>+              inset 0 0 2px #457FDE;
> }

I toyed around a bit and this colour and shadow looks okay for me (notice the spread value!):

.small-button:focus {
  box-shadow: 0 0 2px 2px #79b1dd;
}

It has issues, though, when applied to the [-] button - the left edge hides behind the [+] button then?! Maybe you know why... ;-)

Comment 19

[Stefan [:stefanh]]

(In reply to comment #18)
> Comment on attachment 492754 [details] [diff] [review]
> Now with focus indication
> 
> Minussing due to the ongoing colour discussion. 
> 
> >+.small-button:focus {
> >+  box-shadow: 0 0 3px #457FDE,
> >+              inset 0 0 2px #457FDE;
> > }
> 
> I toyed around a bit and this colour and shadow looks okay for me (notice the
> spread value!):
> 
> .small-button:focus {
>   box-shadow: 0 0 2px 2px #79b1dd;
> }
> 
> It has issues, though, when applied to the [-] button - the left edge hides
> behind the [+] button then?! Maybe you know why... ;-)

With this styling, I don't see any right shadow on the + button. This is why I used a blur of 3px and no spread value and an inset with 2px blur. Also, your suggested color looks too bright - it doesn't fit at all with the focus ring color of the text filed to the left.

Comment 20

[Stefan [:stefanh]]

None of the colors that you have suggested fits for me - this discussion can go on forever. In comment #12, you suggest that the -moz-mac-focusring color fits for you. I don't think it fits, but how about we go for that color?

Comment 21

[Stefan [:stefanh]]

Created attachment 493406 [details] [diff] [review]
New version, with sampled '-webkit-focus-ring-color'

Try this one, I think the color is the right. I played with the shadows and found out that I could have another shadow pointing right/left. For me, it didn't worked for the left button, though (shadow still appears behind right button). I then apply an inset shadow on the right button while the left button has focus (doesn't seem to be needed when the right button is focused).

I used the sampled webkit graphite color, but it might be better to use the -moz one, since I think the button borders shines through in an slightly ugly way.

Comment 22

[Karsten Düsterloh]

Created attachment 493441 [details]
colour difference of attachment #493406 [details] [diff] [review] on my MacBook

The colour difference is still quite visible for me, for whatever reason.
But I agree, this is going nowhere. We should just pick one static colour and probably file a bug against toolkit for a "-moz-appearance: focusring"...

Comment 23

[Stefan [:stefanh]]

Created attachment 493461 [details]
This is how it looks for me

Comment 24

[Karsten Düsterloh]

Comment on attachment 493406 [details] [diff] [review]
New version, with sampled '-webkit-focus-ring-color'

Okay, let's go with that for now and try to fix the actual issue (non-fitting hardcoded colour for Mac focusrings) on the toolkit level.

Comment 25

[Stefan [:stefanh]]

http://hg.mozilla.org/comm-central/rev/20fe92a74dc3