Last Comment Bug 710831 - Tab selector does not blend well with Gloda search pane or mail header
: Tab selector does not blend well with Gloda search pane or mail header
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Theme (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: Thunderbird 12.0
Assigned To: Andreas Nilsson (:andreasn)
:
Mentors:
Depends on: tb-tabsontop
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-14 12:09 PST by Mike Conley (:mconley) - (needinfo me!)
Modified: 2012-04-20 03:46 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+
fixed


Attachments
Notice the awkward transition from tabselector to the tab content (91.94 KB, image/png)
2011-12-14 12:09 PST, Mike Conley (:mconley) - (needinfo me!)
no flags Details
What about moving the search input into the tab toolbar by default? (371.34 KB, image/png)
2011-12-14 12:18 PST, Mike Conley (:mconley) - (needinfo me!)
no flags Details
Fixes Windows headers (724 bytes, patch)
2011-12-15 06:52 PST, Andreas Nilsson (:andreasn)
bwinton: ui‑review-
Details | Diff | Splinter Review
screenshot of above patch (249.71 KB, image/png)
2011-12-15 07:03 PST, Andreas Nilsson (:andreasn)
no flags Details
Fix for Linux (580 bytes, patch)
2011-12-15 11:36 PST, Andreas Nilsson (:andreasn)
no flags Details | Diff | Splinter Review
screenshot of above patch (95.28 KB, image/png)
2011-12-15 11:37 PST, Andreas Nilsson (:andreasn)
no flags Details
new patch (wip) (1.26 KB, patch)
2012-01-19 08:16 PST, Andreas Nilsson (:andreasn)
no flags Details | Diff | Splinter Review
new patch for Linux and Windows (1.46 KB, patch)
2012-01-19 12:57 PST, Andreas Nilsson (:andreasn)
bwinton: ui‑review+
Details | Diff | Splinter Review
win screenshot to ease review (385.14 KB, image/png)
2012-01-23 06:42 PST, Andreas Nilsson (:andreasn)
no flags Details
patch v3 (2.11 KB, patch)
2012-01-24 08:16 PST, Andreas Nilsson (:andreasn)
bugs: ui‑review+
Details | Diff | Splinter Review
patch v4 (2.11 KB, patch)
2012-01-24 08:22 PST, Andreas Nilsson (:andreasn)
mconley: review+
bugs: ui‑review+
standard8: approval‑comm‑aurora+
Details | Diff | Splinter Review

Description Mike Conley (:mconley) - (needinfo me!) 2011-12-14 12:09:23 PST
Created attachment 581742 [details]
Notice the awkward transition from tabselector to the tab content

There is a slight discolouration from the tab selector to the tab content for both the Gloda search panes, as well as the message header, when viewing an individual message.

Proposed solutions:

1)  Have the message header use a gradient to transition smoothly from the tab selector.  This is fine if this persists in the normal 3pane view...it might actually look kind of nice.

2)  The right half of the Gloda search pane (the big white chunk) could also stand to be transitioned with a gradient.
Comment 1 Mike Conley (:mconley) - (needinfo me!) 2011-12-14 12:12:20 PST
Squib also suggested an alternative to my Gloda solution above - we have a toolbar that contains a Gloda search input.  That'd also resolve the issue of the Gloda search input being unavailable when viewing the search results.

Paenglab:  feel like tackling the message pane bit?
Comment 2 Mike Conley (:mconley) - (needinfo me!) 2011-12-14 12:18:31 PST
Created attachment 581746 [details]
What about moving the search input into the tab toolbar by default?

Regarding the idea of including another Gloda search input in a toolbar at the top of the Gloda pane - perhaps we could be less duplicate-y by having the Gloda search input default to being in the tabs toolbar... see attached screenshot.

Thoughts?

-Mike
Comment 3 Andreas Nilsson (:andreasn) 2011-12-14 13:03:11 PST
Taking this on.
Comment 4 Andreas Nilsson (:andreasn) 2011-12-15 06:52:32 PST
Created attachment 581950 [details] [diff] [review]
Fixes Windows headers

Mike suggested just changing the message header color, so this patch does that.
I think this looks a bit slightly odd in the 3-pane window, so I'll publish another patch doing this for just the single message case, then we can figure out what one to use.
Comment 5 Andreas Nilsson (:andreasn) 2011-12-15 07:03:48 PST
Created attachment 581953 [details]
screenshot of above patch

from top to bottom:
1. aero glass - single tab
2. aero glass - 3-pane
3. basic - single tab
4. basic - 3-pane
5. classic - single tab
6. classic - 3-pane
Comment 6 Mike Conley (:mconley) - (needinfo me!) 2011-12-15 07:28:56 PST
Comment on attachment 581953 [details]
screenshot of above patch

I don't really mind this - but then again, it's kinda hard to tell what it looks like in total context...thoughts, Blake?
Comment 7 Mike Conley (:mconley) - (needinfo me!) 2011-12-15 07:29:14 PST
Comment on attachment 581953 [details]
screenshot of above patch

I don't really mind this - but then again, it's kinda hard to tell what it looks like in total context...thoughts, Blake?
Comment 8 Mike Conley (:mconley) - (needinfo me!) 2011-12-15 07:29:45 PST
Comment on attachment 581953 [details]
screenshot of above patch

Argh, submitted too soon.
Comment 9 Blake Winton (:bwinton) (:☕️) 2011-12-15 08:23:45 PST
Comment on attachment 581950 [details] [diff] [review]
Fixes Windows headers

So, I dunno.  I agree that it looks weird in the 3-pane, but it looks way better in the message tab.  And I think it looks a little better in the message window.

Perhaps we could do something like we do on Mac, and fade over the first 15-20 pixels from the darker blue of the active tab to the lighter blue of the current header?
Comment 10 Jim Porter (:squib) 2011-12-15 09:22:16 PST
I actually like it. Since that's not a real reason for it to be checked in, here's a more objective one: if we use the tab bar color for the header everywhere, we'll help minimize the number of different (meaningless) colors in the UI, which should make us look more internally consistent.

This might imply some other background color changes too, e.g. the quick filter bar, and maybe the folder pane.
Comment 11 Blake Winton (:bwinton) (:☕️) 2011-12-15 10:06:25 PST
Oh, right, the fewer different colours _was_ one thing I liked that I wanted to mention, but forgot to.  Thanks for pointing that out, Jim!  :)
Comment 12 Andreas Nilsson (:andreasn) 2011-12-15 11:19:30 PST
Good point. It seems I was unable to special case the single message tab case anyhow :)
Comment 13 Andreas Nilsson (:andreasn) 2011-12-15 11:36:20 PST
Created attachment 582049 [details] [diff] [review]
Fix for Linux

Here is the fix for the header for Linux.
Comment 14 Andreas Nilsson (:andreasn) 2011-12-15 11:37:22 PST
Created attachment 582050 [details]
screenshot of above patch
Comment 15 Richard Marti (:Paenglab) 2011-12-15 11:52:06 PST
(In reply to Andreas Nilsson (:andreasn) from comment #12)
> Good point. It seems I was unable to special case the single message tab
> case anyhow :)

Andreas, you can try this rule:

#folderPaneBox[collapsed="true"] + splitter + vbox #displayDeck[collapsed="true"] +
splitter + #messagepaneboxwrapper #msgHeaderView {
  background: something;
}
Comment 16 Jim Porter (:squib) 2011-12-20 16:52:33 PST
(In reply to Richard Marti [:paenglab] from comment #15)
> (In reply to Andreas Nilsson (:andreasn) from comment #12)
> > Good point. It seems I was unable to special case the single message tab
> > case anyhow :)
> 
> Andreas, you can try this rule:
> 
> #folderPaneBox[collapsed="true"] + splitter + vbox
> #displayDeck[collapsed="true"] +
> splitter + #messagepaneboxwrapper #msgHeaderView {
>   background: something;
> }

It would probably be sufficient to do this:

  #displayDeck[collapsed="true"] + splitter + #messagepaneboxwrapper #msgHeaderView

This way would be nice, since then if you collapse the threadpane in the 3pane tab, the colors will match up nicely there too. :)
Comment 17 Jim Porter (:squib) 2011-12-20 20:19:27 PST
(In reply to Jim Porter (:squib) from comment #16)
> This way would be nice, since then if you collapse the threadpane in the
> 3pane tab, the colors will match up nicely there too. :)

Wait... actually you can't collapse the threadpane. Of course, that means the CSS in comment 16 is still sufficient.
Comment 18 Andreas Nilsson (:andreasn) 2011-12-21 11:12:59 PST
Thanks for the tip, will do that!
Comment 19 Andreas Nilsson (:andreasn) 2012-01-19 08:16:59 PST
Created attachment 589874 [details] [diff] [review]
new patch (wip)

Only puts the gradient on if the message is in a tab so that we maintain the flat appearance in the 3-pane view.
Comment 20 Andreas Nilsson (:andreasn) 2012-01-19 12:57:08 PST
Created attachment 589957 [details] [diff] [review]
new patch for Linux and Windows

Personas not yet fixed, coming up!
Comment 21 Andreas Nilsson (:andreasn) 2012-01-23 06:14:06 PST
Trying to apply the persona to the message header just made it look weird, so I'm going to ask for review on the last patch.
Comment 22 Andreas Nilsson (:andreasn) 2012-01-23 06:42:15 PST
Created attachment 590693 [details]
win screenshot to ease review
Comment 23 Mike Conley (:mconley) - (needinfo me!) 2012-01-23 07:20:00 PST
Andreas:

Does this affect the appearance of the Gloda tab in any way?

-Mike
Comment 24 Andreas Nilsson (:andreasn) 2012-01-23 07:25:48 PST
(In reply to Mike Conley (:mconley) from comment #23)
> Does this affect the appearance of the Gloda tab in any way?

No. I recall we've been talking about adding a toolbar to that tab in other bugs, so I've left it alone for now. I could add some kind of strip to it if needed.
Comment 25 Mike Conley (:mconley) - (needinfo me!) 2012-01-23 07:27:02 PST
Andreas:

Ok, cool.  Yeah, my work with the search-in-gloda-tab feature adds a toolbar.  Just wanted to make sure we were still on the same page there.  :)

Reviewing...

-Mike
Comment 26 Mike Conley (:mconley) - (needinfo me!) 2012-01-23 08:21:03 PST
Comment on attachment 589957 [details] [diff] [review]
new patch for Linux and Windows

Review of attachment 589957 [details] [diff] [review]:
-----------------------------------------------------------------

Just a few nits, but other than that, the code looks pretty good to me.  Before I give my r+, do we have to worry about qute's messageHeader.css?  We only seem to be dealing with Aero here.

::: mail/themes/gnomestripe/mail/messageHeader.css
@@ -51,2 +51,5 @@
> >  }
> >  
> > +/* :::::  message in a tab ::::: */
> > +#displayDeck[collapsed="true"] + splitter + #messagepaneboxwrapper .main-header-area {
> > +  background-image: -moz-linear-gradient(rgba(255, 255, 255, 0.3), rgba(255, 255, 255, 0) 19px);

Is this < 80 characters?  If not, we should split it up with a newline.

::: mail/themes/qute/mail/messageHeader-aero.css
@@ +51,5 @@
>  
> +/* :::::  message in a tab ::::: */
> +#displayDeck[collapsed="true"] + splitter + #messagepaneboxwrapper .main-header-area {
> +  background-image: -moz-linear-gradient(rgba(255, 255, 255, .5),
> +                    rgba(255, 255, 255, .5) 100%);

Since this is inside the -moz-linear-gradient parentheses, we should align it so that the other rgba(255, 255, 255, .5) is right above.

@@ -52,0 +52,9 @@
> > +/* :::::  message in a tab ::::: */
> > +#displayDeck[collapsed="true"] + splitter + #messagepaneboxwrapper .main-header-area {
> > +  background-image: -moz-linear-gradient(rgba(255, 255, 255, .5),
> > +                    rgba(255, 255, 255, .5) 100%);
NaN more ...

These two lines should be indented two spaces.
Comment 27 Blake Winton (:bwinton) (:☕️) 2012-01-23 10:04:31 PST
Comment on attachment 589957 [details] [diff] [review]
new patch for Linux and Windows

Based on the screenshots, I'm saying ui-r=me.  :)
Comment 28 Andreas Nilsson (:andreasn) 2012-01-24 07:54:15 PST
(In reply to Mike Conley (:mconley) from comment #26)
> Just a few nits, but other than that, the code looks pretty good to me. 
> Before I give my r+, do we have to worry about qute's messageHeader.css?  We
> only seem to be dealing with Aero here.

Good catch. I've finally learned how to trick TB into XP mode and it seems it's not picking up the styling properly. Will fix.
Comment 29 Andreas Nilsson (:andreasn) 2012-01-24 08:16:51 PST
Created attachment 591102 [details] [diff] [review]
patch v3

Mike: Did you mean like this?
Comment 30 Andreas Nilsson (:andreasn) 2012-01-24 08:22:16 PST
Created attachment 591106 [details] [diff] [review]
patch v4

Missed to indent line 63 and 64! :/
Now it should be Perfect. ;)
Comment 31 Mike Conley (:mconley) - (needinfo me!) 2012-01-24 12:47:25 PST
Comment on attachment 591106 [details] [diff] [review]
patch v4

Review of attachment 591106 [details] [diff] [review]:
-----------------------------------------------------------------

This looks right to me - though what are our plans for OSX?
Comment 32 Mike Conley (:mconley) - (needinfo me!) 2012-01-26 06:40:16 PST
Ping
Comment 33 Andreas Nilsson (:andreasn) 2012-01-26 08:12:54 PST
(In reply to Mike Conley (:mconley) from comment #32)
> Ping

As mentioned on IRC, I've opened a separate bug about the theme issue under OSX so we can land this while I'll figure out how do deal with that styling, as OSX sports white headers so needs another approach. Bug 721405 is for OS X.
Comment 34 Mike Conley (:mconley) - (needinfo me!) 2012-01-26 10:17:33 PST
Landed on comm-central as http://hg.mozilla.org/comm-central/rev/81f9de6c78e6
Comment 35 Mike Conley (:mconley) - (needinfo me!) 2012-01-31 10:31:21 PST
Committed to comm-aurora as http://hg.mozilla.org/releases/comm-aurora/rev/0dd648cb208c

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