Last Comment Bug 613358 - Polish the mailNews search dialog on Mac and also fix some obsolete styles (all OS)
: Polish the mailNews search dialog on Mac and also fix some obsolete styles (a...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Themes (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1b2
Assigned To: Stefan [:stefanh]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-11-18 14:47 PST by Stefan [:stefanh]
Modified: 2010-11-30 09:02 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
v1.0 (12.78 KB, patch)
2010-11-18 14:53 PST, Stefan [:stefanh]
neil: superreview+
Details | Diff | Review
Screenshot of dialog after/before (117.26 KB, image/png)
2010-11-18 14:54 PST, Stefan [:stefanh]
no flags Details
v1.1 - Slightly adjusted version (12.80 KB, patch)
2010-11-21 09:06 PST, Stefan [:stefanh]
mnyromyr: review-
Details | Diff | Review
Now with focus indication (13.05 KB, patch)
2010-11-23 12:50 PST, Stefan [:stefanh]
mnyromyr: review-
Details | Diff | Review
custom (13.52 KB, image/png)
2010-11-23 15:36 PST, Stefan [:stefanh]
no flags Details
-moz-mac-focusring (13.38 KB, image/png)
2010-11-23 15:37 PST, Stefan [:stefanh]
no flags Details
New version, with sampled '-webkit-focus-ring-color' (13.65 KB, patch)
2010-11-26 07:19 PST, Stefan [:stefanh]
mnyromyr: review+
Details | Diff | Review
colour difference of attachment #493406 on my MacBook (1.36 KB, image/png)
2010-11-26 12:42 PST, Karsten Düsterloh
no flags Details
This is how it looks for me (4.86 KB, image/png)
2010-11-26 14:08 PST, Stefan [:stefanh]
no flags Details

Description Stefan [:stefanh] 2010-11-18 14:47:15 PST
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] 2010-11-18 14:53:50 PST
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] 2010-11-18 14:54:37 PST
Created attachment 491679 [details]
Screenshot of dialog after/before
Comment 3 Stefan [:stefanh] 2010-11-18 14:56:36 PST
Btw, Karsten: I've tried to mimic the +/- buttons in the OS prefs (Language/text). No bold label text, though.
Comment 4 Stefan [:stefanh] 2010-11-19 16:21:44 PST
JFTR, I have some slight modifications of the .small-button's coming up in the next days.
Comment 5 Stefan [:stefanh] 2010-11-21 09:06:37 PST
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] 2010-11-21 12:17:39 PST
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 2010-11-21 15:25:09 PST
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 neil@parkwaycc.co.uk 2010-11-22 04:09:47 PST
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] 2010-11-22 09:30:16 PST
(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] 2010-11-23 12:50:54 PST
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] 2010-11-23 13:38:16 PST
(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 2010-11-23 14:36:14 PST
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] 2010-11-23 15:25:05 PST
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] 2010-11-23 15:36:43 PST
Created attachment 492836 [details]
custom
Comment 15 Stefan [:stefanh] 2010-11-23 15:37:27 PST
Created attachment 492837 [details]
-moz-mac-focusring
Comment 16 Stefan [:stefanh] 2010-11-23 15:38:43 PST
it's not the end of the world, but i think the custom one looks better.
Comment 17 Stefan [:stefanh] 2010-11-23 15:40:11 PST
pick the one you like!
Comment 18 Karsten Düsterloh 2010-11-25 14:23:29 PST
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] 2010-11-25 14:44:52 PST
(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] 2010-11-26 04:53:03 PST
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] 2010-11-26 07:19:39 PST
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 2010-11-26 12:42:53 PST
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] 2010-11-26 14:08:27 PST
Created attachment 493461 [details]
This is how it looks for me
Comment 24 Karsten Düsterloh 2010-11-28 15:15:46 PST
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] 2010-11-30 09:02:41 PST
http://hg.mozilla.org/comm-central/rev/20fe92a74dc3

Note You need to log in before you can comment on or make changes to this bug.