366842 - Land Pinstripe Theme Update for Mac OS X

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Scott MacGregor]

Kevin Gerich has been working on a theme update for pinstripe. it's mostly complete. He just sent me a JAR file which I'll be mapping back to CVS so we can get it landed. Thanks Kevin!

Comment 1

[Scott MacGregor]

Attachment: first cut at porting the theme update

This is my first cut at taking the JAR file Kevin sent me and mapping it back to CVS.

There are some toolkit changes here that we may have to move into mail/themes/pinstripe.

Comment 2

[Scott MacGregor]

The following parts of the theme haven't been finished yet and probably won't be addressed until after beta 2:

* Collapsed message header pane, the subject and date have a grey background
* back and forward buttons for new history feature haven't bee updated and styled yet
* Flags haven't been replaced by stars yet.
* Don't have a small icon version for tag yet
* Some splitters might look strange
* Addons bottom toolbar button hasn't been finished yet and looks weird

Comment 3

[Scott MacGregor]

Attachment: updated patch for the branch

This is an updated patch that has minimal impact to shared code in toolkit since we don't want to risk any inadvertent changes to Firefox. The only toolkit change was to add some splitter images which are unreferenced by firefox and toolkit on the branch. For the trunk I'll be petitioning to change the pinstripe splitters in toolkit to use the new artwork from Kevin.

Comment 4

[Scott MacGregor]

This patch should be in today's Mac nightly branch build assuming I didn't miss anything!

Comment 6

[Kevin Gerich]

Attachment: mail/themes/pinstripe/mail patch

changes:

- stars instead of flags
- back and forward icons
- small version of tag icon
- margin and padding tweakage to message header area
- new warning icon to phishing/image notification bar

coming up:

- compose window style tweaks
- addons window style tweaks

Comment 7

[Scott MacGregor]

Comment on attachment 255999 [details] [diff] [review]
mail/themes/pinstripe/mail patch

Excellent, the screen shot you sent me looks great. 

Don't forget to add these icons to the source tree:
? mail/themes/pinstripe/mail/icons/flaggedmail-outline.png
? mail/themes/pinstripe/mail/icons/warning.png

Lemme know if you need me  to help land anything on the branch. I don't believe I've landed your theme changes from the first round on the trunk yet. I'll try to do that by next week.

Comment 8

[Kevin Gerich]

Checking in mail/themes/pinstripe/mail/jar.mn;
/cvsroot/mozilla/mail/themes/pinstripe/mail/jar.mn,v  <--  jar.mn
new revision: 1.6.2.8; previous revision: 1.6.2.7
done
Checking in mail/themes/pinstripe/mail/messageHeader.css;
/cvsroot/mozilla/mail/themes/pinstripe/mail/messageHeader.css,v  <--  messageHeader.css
new revision: 1.2.2.6; previous revision: 1.2.2.5
done
Checking in mail/themes/pinstripe/mail/primaryToolbar.css;
/cvsroot/mozilla/mail/themes/pinstripe/mail/primaryToolbar.css,v  <--  primaryToolbar.css
new revision: 1.3.2.9; previous revision: 1.3.2.8
done
Checking in mail/themes/pinstripe/mail/icons/flagcol.png;
/cvsroot/mozilla/mail/themes/pinstripe/mail/icons/flagcol.png,v  <--  flagcol.png
new revision: 1.1.4.1; previous revision: 1.1
done
Checking in mail/themes/pinstripe/mail/icons/flaggedmail.png;
/cvsroot/mozilla/mail/themes/pinstripe/mail/icons/flaggedmail.png,v  <--  flaggedmail.png
new revision: 1.1.4.1; previous revision: 1.1
done
Checking in mail/themes/pinstripe/mail/icons/mail-toolbar-small.png;
/cvsroot/mozilla/mail/themes/pinstripe/mail/icons/mail-toolbar-small.png,v  <--  mail-toolbar-small.png
new revision: 1.1.4.1; previous revision: 1.1
done
Checking in mail/themes/pinstripe/mail/icons/mail-toolbar.png;
/cvsroot/mozilla/mail/themes/pinstripe/mail/icons/mail-toolbar.png,v  <--  mail-toolbar.png
new revision: 1.1.4.2; previous revision: 1.1.4.1
done
RCS file: /cvsroot/mozilla/mail/themes/pinstripe/mail/icons/Attic/warning.png,v
done
Checking in mail/themes/pinstripe/mail/icons/warning.png;
/cvsroot/mozilla/mail/themes/pinstripe/mail/icons/Attic/warning.png,v  <--  warning.png
new revision: 1.1.2.1; previous revision: 1.1
done

Comment 9

[Scott MacGregor]

Attachment: port to the trunk

This patch ports Kevin's first round of pinstripe changs to the trunk. It doesn't include what he just checked in.

Before I can land this I have to figure out from the toolkit owners if it's ok to add the splitter images to toolkit and if Firefox wants to use them.

Comment 10

[Kevin Gerich]

Can you drop the splitter changes in your patch, Scott? I think I'm going to put the new splitter image into pinstripe/mail instead of the global sheet, because I think the image doesn't really work on all splitters, but I want it on the thread pane/message pane splitter in the main window.

Comment 11

[Scott MacGregor]

want me to remove splitter.css and the splitter icons in toolkit from the trunk patch Kevin? 

Comment 12

[Kevin Gerich]

Yes please. 

Comment 13

[Scott MacGregor]

Attachment: [fixed on the trunk]updated trunk port

well that sure makes this patch easier. I'll land this right now. :)

Comment 14

[Kevin Gerich]

Attachment: Pinstripe tweaks round 2

changes:

- threadpane splitter style
- new news & blogs folder icons
- toolbar gradient on compose, pref, address book windows
- colorized security icon on compose window
- flat search bar icon

next:

- addons window tweaks
- find bar tweaks

Comment 15

[snowcrash+mozilla]

not entirely certain _this_ is the right thread, but it *is* part of my *theme*, so ...

i use bonecho nightlies on osx. atm, i still "fancify" bonecho by dropping the executable on,

  "Firefoxy: Firefox fancy widget applicator"
    http://www.amake.us/software/firefoxy/

even though it's 'dusty', and technically only supported for v1.5 (iiuc), it still works nicely.

any chance/plans for these widgets to be included in Pinstripe?

Comment 16

[Scott MacGregor]

Comment on attachment 256172 [details] [diff] [review]
Pinstripe tweaks round 2

Don't forget to cvs add -kb these new images:

? mail/themes/pinstripe/mail/icons/flaggedmail-outline.png
? mail/themes/pinstripe/mail/icons/message-news-orig.png
? mail/themes/pinstripe/mail/icons/server-newsblog-orig.png
? mail/themes/pinstripe/mail/icons/threadpane-splitter-bg.gif

thanks a lot kevin.

Comment 17

[snowcrash+mozilla]

> any chance/plans for these widgets to be included in Pinstripe?

just learned that Kevin Gerich is the 'father' of these widgets, and is already 'here'.

excellent.

thanks!

Comment 18

[Kevin Gerich]

Round 2 patch checked in on branch.

We shouldn't discuss form widgets in this bug, but Josh Aas is working on bringing native form widgets to Firefox: http://weblogs.mozillazine.org/josh/archives/2007/01/native_form_widgets_getting_cl.html

Comment 19

[snowcrash+mozilla]

> We shouldn't discuss form widgets in this bug

sorry

> but Josh Aas is working on bringing native form widgets to Firefox:

thanks.

Comment 20

[Kevin Gerich]

Attachment: Pinstripe - override some images in the toolkit theme

overrides a few images in the toolkit theme to make the addons window and find bar look consistent with the rest of the mail theme

Comment 21

[Kevin Gerich]

(In reply to comment #20)
> Created an attachment (id=256400) [details]
> Pinstripe - override some images in the toolkit theme

Checked in on the branch. I think I'm done :) aside from bringing the changes over to the trunk.  

Comment 22

[Scott MacGregor]

adding the fixed keyword since this is done on the branch (and looking great I might add). Leaving open until we get the last couple patches on the trunk.

Comment 23

[Markus S.]

Sorry guy, but you are not done. The new Pinstripe somewhat mimicks the Unified Aqua style -- propably as much as much as possible with XUL.
Considering this, a few remaining glitches should be sorted out first.

1.) The status bar. It's not a "pure" status bar, because it also contains that Online-Offline button. That's fine, but it's not really obvious that this is a button, right? See http://indiehig.com/wiki/Unified_Aqua (No. 3) how that status/button bar should look like.

2.) I don't know if this can be done in XUL, but the Message Filter Rule Editor also looks kinda out of place. It should look more like this: http://indiehig.com/wiki/Image:Finder_Find_Rule_Editor.png
According graphics (under public domain) can be optained from http://indiehig.com/wiki/Unified_Aqua_Elements

3.) The address book. The "Send Instant Message" icon is identical to the "Reply to All" button. Maybe some kind of chat bubble?

4.) Is it just me or was the Add-Ons window left out of any improvements? At least it's not really consistent. Extensions without an own icon (like Talkback) have a glossy puzzle piece, but the puzzle piece in the toolbar looks more like a comic drawing. IMHO these two icons should look the same.
The preview image in the Themes tab is also still the old one.

5.) The Stop button still looks like in Firefox 0.8 -- without that white X found in more recent Pinstripe versions for FF.

That's what I've seen so far.

Comment 24

[Kevin Gerich]

(In reply to comment #23)
> Sorry guy, but you are not done. 

I agree that we could have gone farther, but this is what we had time for before the TB 2 code freeze. Please file separate bugs on these issues.

> 3.) The address book. The "Send Instant Message" icon is identical to the
> "Reply to All" button. Maybe some kind of chat bubble?

This is an actual bug. The IM icon image is wrong when it's on the customize toolbar dialog, but it's correct when you put it on the toolbar. Scott, do we have time to fix this?

Comment 25

[Scott MacGregor]

Yeah, we've got till Tuesday morning.

Comment 26

[Kevin Gerich]

Attachment: point addressbook toolbar at correct image

Here's a fix for the wrong IM icon showing up on the addressbook customize dialog. Thanks for pointing this out, KAMiKAZOW.

Comment 27

[Scott MacGregor]

Comment on attachment 257294 [details] [diff] [review]
point addressbook toolbar at correct image

I'm not sure I understand why using !important changes the image we use for the IM icon in the customize toolbar icon. Is something else causing us to use the reply all icon instead of the IM icon?

Comment 28

[Phil Ringnalda (:philor)]

Attachment: [fixed on trunk & branch]like so?

Using !important there "works" by regressing the icons when you customize the main 3pane window to either wrongness, or blank space :)

Because everything piles its stylesheet into customizeToolbar.xul, the last one to define .toolbarbutton-1 wins, so lacking something more specific (or !important) than that class, primaryToolbar.css's list-style-image wins.

In msgCompSMIMEOverlay.css and messengercompose.css, and qute/addressbook.css, and pinstripe/addressbook.css with small icons, that doesn't matter because each button redefines the list-style-image to its image, and #button-foo is more specific than .toolbarbutton-1, but somehow large-icon pinstripe never got that memo.

(Feel free to land for me if you want to beat Tuesday, since Monday's a bad day for tree-watching for me.)

Comment 29

[Wayne Woods]

 #attachment-splitter {
-  min-height: 7px;
+  min-height: 3px;
   border-top: 1px solid #C8C8C8;
+  background-color: #E6E6E6;
 }

Kevin, the theme updates are great :) Though personally, I think the attachment splitter looked better with the transparent background, so that it merged with the box margins, but maybe I'm biased ;). But if you really want it to be a different colour, I think you should at least probably put back the splitter bottom border that I removed, and add back a top margin to the attachmentView so that it doesn't run right against the box and make the margins look unbalanced. It originally had a 6px margin-top to match the 6px margins on the other three sides. I only removed it because the "merged" splitter substituted for it.

See attachment 255533 [details] [diff] [review] for what was done to it previously.

Comment 30

[Scott MacGregor]

Comment on attachment 257330 [details] [diff] [review]
[fixed on trunk & branch]like so?

Ah this makes more sense to me. Thanks Phil. I'll land this for you.

Comment 31

[Scott MacGregor]

Comment on attachment 257294 [details] [diff] [review]
point addressbook toolbar at correct image

I think Phil's patch is a better solution...

Comment 32

[Kevin Gerich]

Yep, Phil's patch makes more sense. Thanks Phil

Comment 33

[Phil Ringnalda (:philor)]

Comment on attachment 257330 [details] [diff] [review]
[fixed on trunk & branch]like so?

Had a moment of panic when I saw this still modified in my tree, until I realized it was my *trunk* tree.

mail/themes/pinstripe/mail/addrbook/addressbook.css: 1.3

Comment 34

[Phil Ringnalda (:philor)]

Attachment: To the trunk, v.1

I hope this gets everything on the trunk, without clobbering anything that's been done between then and now - unless I'm missing something, everything in a branch-to-trunk diff with this applied is something I can explain wanting to be there.

The one thing I didn't port was

+#msgcomposeWindow {
+  -moz-appearance: none !important;
+}
+

in mail/themes/pinstripe/mail/primaryToolbar.css, because I was unable to explain to myself what it was doing there: primaryToolbar.css seems to only apply in message windows, and I don't think we open compose windows inside them, unless I'm missing something.

Comment 35

[Phil Ringnalda (:philor)]

Attachment: Branch-trunk diff

The aforementioned diff between branch pinstripe and trunk pinstripe with my patch.

Comment 36

[Phil Ringnalda (:philor)]

Attachment: To the trunk, v.2

Version 2, now with what I hope are _all_ the affected files :(

Comment 37

[Scott MacGregor]

Comment on attachment 281140 [details] [diff] [review]
To the trunk, v.2

thanks phil.

+#msgcomposeWindow {
+  -moz-appearance: none !important;
+}
+

I can't see how that would effect anything either. Thanks for leaving it out.

Comment 38

[Phil Ringnalda (:philor)]

Fixed. To be followed by a slew of followup bugs, once I find my notes, but yes, I know about the purple plug-in plugin and have plans to make it only slightly horrible instead :)