Closed Bug 301465 Opened 19 years ago Closed 16 years ago

Port bug 285288's CSS rules to qute: addressing widget appearance

Categories

(Thunderbird :: Message Compose Window, defect)

defect
Not set
trivial

Tracking

(Not tracked)

VERIFIED DUPLICATE of bug 398874

People

(Reporter: mcow, Assigned: mscott)

References

Details

Attachments

(1 file, 4 obsolete files)

Bug 285288 updated the CSS for the addressing-widget field-selection button, but 
only added the rules to the gnomestripe theme (attachment 189711 [details] [diff] [review]).  Altho the 
two added lines for list-style-image are in fact theme-specific, the other two 
added rules affecting the -moz-appearance attribute actually should be applied 
to qute, and to any other theme derived from Mozilla's Classic.
Attached patch Non-CVS diff -uw (obsolete) — Splinter Review
Is this theme even called qute anymore?  My 1.0+0715 doesn't have a qute.jar,
but it does have classic.jar; this patch shows the update I've made in that
file, which works as expected.
Attached patch Non-CVS diff -uw (obsolete) — Splinter Review
Is this theme even called qute anymore?  My 1.0+0715 doesn't have a qute.jar,
but it does have classic.jar; this patch shows the update I've made in that
file, which works as expected.
Comment on attachment 189914 [details] [diff] [review]
Non-CVS diff -uw

(Sorry for the lack of CVS-itude, Scott!)
Attachment #189914 - Flags: superreview?(mscott)
Attachment #189912 - Attachment is obsolete: true
(In reply to comment #0)
> Bug 285288 updated the CSS for the addressing-widget field-selection button, but 
> only added the rules to the gnomestripe theme (attachment 189711 [details] [diff] [review] [edit]). 
Altho the 
> two added lines for list-style-image are in fact theme-specific, the other two 
> added rules affecting the -moz-appearance attribute actually should be applied 
> to qute, and to any other theme derived from Mozilla's Classic.

Adding the appearance "button" for menulist-compact enlarges the height of the
listbox row significantly. It really doesn't look ok IMO. If we use this changes
for qute, we should make the button height smaller. Mike, you see that there are
differences between the OS. Therefor it has to be theme specific.

I have taken a look at qute but found no way to reduce the buttons height. Isn't
possible?
I hadn't noticed that; you're right; it looks to be about two pixels' difference 
in height, probably due to the padding for a button vs. that for a simple 
dropdown.  For the default four address rows, that's eight pixels -- almost a 
character height.  It's not clear to me that this is OS-specific, why do you say 
that?

At any rate, I cannot at all agree that it "doesn't look OK" -- it looks so much 
better that there should be no question that the patched version is superior.

Plus, it's more functional, because the ugly dropdown doesn't change visibly 
when it gets the focus, whereas the flat button does.
Flags: blocking-aviary1.5?
Attached patch Ported CSS styles for qute (obsolete) — Splinter Review
Here is the diff against current head. I still dislike it due to the height of
the button. Scott, it's your decission. If you agree please give us some facts
what we could do here. Thanks.
Attachment #189914 - Attachment is obsolete: true
Attachment #191511 - Flags: review?(mscott)
Assignee: mscott → hskupin
Attachment #189914 - Flags: superreview?(mscott)
Flags: blocking-aviary1.5? → blocking1.8b4?
Flags: blocking1.8b4? → blocking1.8b4-
Attached patch Complete Patch (obsolete) — Splinter Review
Ok, I have taken a further look at this missing feature for qute and have to
revert my opinions against the UI style. It looks good for me in using button
for menulist-compact. Additionally I included the focus box for qute and
gnomestripe. That is missed since a very long time!

Scott, do you have time to look at this? Perhaps it will find it's way into
1.8?
Attachment #191511 - Attachment is obsolete: true
Attachment #198130 - Flags: review?(mscott)
Attachment #191511 - Flags: review?(mscott)
(In reply to comment #7)
> Ok, I have taken a further look at this missing feature for qute and have to
> revert my opinions against the UI style. It looks good for me in using button
> for menulist-compact.

Yay!

I'm not sure why that dotted border on focus was added -- don't you get the dark 
border on focus automatically?  I don't care for the appearance of the dotted 
line just around the text, but I could live with it.

As for the color, this may not be of interest depending on theme and color, but 
I've added this rule to my userChrome.css:
===
/* default color of the label when the list is open is white -- override it */
.menulist-compact[open="true"] > .menulist-label
 { color : blue; }
===
This makes the text of the button (e.g. "To:") display, when the list is opened, 
as blue-on-grey, rather than white-on-grey which is insufficiently contrasted. 
Is that the same reason you added the color rule for the :focus case?  If so: 
That 'white' color must be specified somewhere, it would probably be better to 
find that rule and change it rather than overriding it.
(In reply to comment #8)
> I'm not sure why that dotted border on focus was added -- don't you get the dark 
> border on focus automatically?  I don't care for the appearance of the dotted 
> line just around the text, but I could live with it.

No, there is no focus rect drawn when the menulist has focus. Just try it out.
That's why I've added the :focus part. FYI the style is a proposal from Neil
which I covered on bug 184811.

> /* default color of the label when the list is open is white -- override it */
> .menulist-compact[open="true"] > .menulist-label
>  { color : blue; }

We never use blue afaik. Why it should stay here? I don't think thats a good
idea to use a hardcoded color for each theme.

> as blue-on-grey, rather than white-on-grey which is insufficiently contrasted. 
> Is that the same reason you added the color rule for the :focus case?  If so: 
> That 'white' color must be specified somewhere, it would probably be better to 
> find that rule and change it rather than overriding it.

Yes, I found the rule which set's it to white or especially Highlighted. It will
be covered within my upcoming patch.
Status: NEW → ASSIGNED
We can't use a focus around the whole menulist-compact. It doesn't look nicely.
Around the label it's more visible to the user. So I didn't touch that part
again. Now the displayed white color of the label when opening the menulist is
removed.
Attachment #198130 - Attachment is obsolete: true
Attachment #198141 - Flags: review?(mscott)
Attachment #198130 - Flags: review?(mscott)
(In reply to comment #9)
> (In reply to comment #8)
> > I'm not sure why that dotted border on focus was added -- don't you get the
> > dark border on focus automatically?  I don't care for the appearance of the 
> > dotted line just around the text, but I could live with it.
> 
> No, there is no focus rect drawn when the menulist has focus. Just try it out.

I *have* tried it out, and under Windows, there's a dark border, like the 
"default button" border, drawn around the 'button' when it gets focus.  This is 
consistent, display-wise, with Windows: a pushbutton that gets focus also gets 
the 'default' attribute.  Unfortunately, the 'default' keyhandling doesn't work: 
hitting Enter with the focus on that button does nothing, where it ought to 
behave as if the button were clicked.  Neither does the default 'focus' 
keyhandling, where it ought to behave as clicked if a space is typed.  I guess 
that's all a different bug.

At some point, while playing with the CSS from this bug, bug 285288 and 
bug 184811, I saw that when the .menulist-compact got focus, the display change 
was to invert the color of the entire button -- no border.  With this behavior, 
turning the text 'white' makes sense!  But I don't see this happening any longer 
(TB or Seamonkey).  In fact, I see such a change in the patch in attachment 
194821 [details] [diff] [review] (bug 184811), but that patch isn't marked.

Neil said there that he didn't like the button focus style, but the current 
styling (bold border, with or without dotted outline) breaks UI expectations, 
for the reasons described above.


Anyway: Once the patch is in, I'll try it unmodified for a while; if the dotted 
border really bugs me, I'll just turn it off in userChrome.


> We never use blue afaik. Why it should stay here? I don't think thats a good
> idea to use a hardcoded color for each theme.

I wasn't suggesting that 'blue' be incorporated into the patch; I was showing 
the selector I'm using, in comparison to your color rule.  Since your new patch 
simply removes the rule that turned the text to white, that's great; no color 
specification is necessary at all.

I notice, however, that the CSS from your current patch doesn't integrate well 
with a theme that specifies a specific "3D-Face" color; I tried it under 
Seamonkey with the Modern theme, and the button clashes.
While bug 184811 for SM comes to it's final state, what's the expected behavior for Thunderbird? Should we port the code from SM or what aims shall we follow? Scott, any chance to get an answer from your person?
Neil's patch at bug 184811 ended up quite a bit more complex than originally imagined when this bug was opened; there's new XUL code in addition to a lot of CSS changes.  The widget in Seamonkey looks really good, tho, in both Classic and Modern themes.  It'd be great to get that over to TB.
*** Bug 357653 has been marked as a duplicate of this bug. ***
Assignee: hskupin → mscott
Status: ASSIGNED → NEW
QA Contact: message-compose
Comment on attachment 198141 [details] [diff] [review]
Patch with corrected colors

Give Mike's comment 13, I think we should port the work Neil did for seamonkey, so I'm going to minus this patch in the hopes that someone will port 184811 instead.
Attachment #198141 - Flags: review?(mscott) → review-
Someone did.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → DUPLICATE
Not sure if all of the work from 184811 was ported over, but the rules in this bug are in place on the trunk.  I can now trim them from my userChrome.css file.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: