[10.5] Update Bookmark Bar and Tab bar for Leopard

RESOLVED FIXED in Camino2.0

Status

RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: froodian, Assigned: phiw2)

Tracking

unspecified
Camino2.0
x86
Mac OS X
Bug Flags:
camino1.5.4 -
camino1.5.5 -
camino1.6b3 -

Details

(Whiteboard: [camino-2.0])

Attachments

(15 attachments, 10 obsolete attachments)

7.10 KB, patch
Jeff.Dlouhy
: review-
Details | Diff | Splinter Review
104.88 KB, image/png
Details
100.73 KB, image/png
Details
4.69 KB, image/png
Details
4.72 KB, image/png
Details
3.80 KB, image/png
Details
4.22 KB, image/png
Details
9.18 KB, patch
mikepinkerton
: superreview+
Details | Diff | Splinter Review
90.94 KB, image/png
Details
61.69 KB, image/png
Details
104.29 KB, image/png
Details
100.99 KB, image/png
Details
1.25 KB, patch
stuart.morgan+bugzilla
: superreview+
Details | Diff | Splinter Review
2.57 KB, application/zip
Details
3.40 KB, application/zip
Details
(Reporter)

Description

11 years ago
Created attachment 286377 [details] [diff] [review]
patch v1

Our bookmark bar gradient needs to be color-updated for 10.5 (made it have the same top/bottom values as the regular toolbar, like we did for 10.4), as does the separating line (made it the same as the one at the bottom of the toolbar for both active and inactive).  Also made the titles etched.

This depends on the NSWorkspace method implemented in bug 401312.
Flags: camino1.5.4?
Attachment #286377 - Flags: review?(Jeff.Dlouhy)
Summary: Update bookmark bar for leopard [10.5] → [10.5] Update bookmark bar for leopard
Can we get a screenshot here, too?

Comment 2

11 years ago
If we were to do the bookmark bar this color, we should do it the way I discussed in bug 381494 (I have some partially finished code I can dust off). However, because of the favicons, it's not clear that we want to do this.

We need to spend some more time discussing what we want to do with the bookmark bar, and trying different mock-ups.
(Reporter)

Comment 3

11 years ago
I agree that we should do it the right way.  If your code is quickly brush-offable, then I welcome it.  If not, I think this is an important stop-gap fix for making Camino not ass-ugly on 10.5.  The favicons (as with all the icons) are a separate issue, and fixing the UI here shouldn't depend on that (just like the aqua progress bar in the status bar).

Comment 4

11 years ago
(In reply to comment #3)
> The favicons (as with all the icons) are a separate issue

I don't agree at all. The fact that favicons (I'm talking about actual favicons, not our bookmark icons) are designed with the expectation that they will be on light backgrounds, and we show favicons in the bookmark bar, means that it's not at all clear that making the bookmark bar look like the toolbar is desirable on 10.5. The goal isn't to have a bookmark bar that looks like a toolbar; the goal is to have a bookmark bar that looks good, and a dark bookmark bar is not clearly the right solution to that.
(Reporter)

Comment 5

11 years ago
Ah, I see.  Yeah, that's a valid point...
(Reporter)

Comment 6

11 years ago
Created attachment 286415 [details]
Screenshot

Here's a screenie of what I coded, for folks at home.
(Reporter)

Comment 7

11 years ago
Created attachment 286925 [details]
Flat-look screenshot

Here's another thought I had.  This *really* depends on the evil color hardcoding (which could still be made significantly less evil), as it just goes from 197 (the lightest shade in the toolbar) to 190 (an arbitrary value somewhat less light to keep it from being totally flat).
On the other hand, our bookmark bar currently looks almost exactly like the bar at the top of Finder windows, so in that sense it doesn't look "wrong" or "out of place" at all.
[12:31pm] ardissone|away: http://img100.imageshack.us/img100/812/picture6zh5.png
[12:31pm] ardissone|away: http://img150.imageshack.us/img150/6138/picture10wr1.png
[12:31pm] ardissone|away: http://img240.imageshack.us/img240/7482/picture11vk7.png
[12:31pm] ardissone|away: http://img502.imageshack.us/img502/6776/picture12tw6.png
[12:31pm] ardissone|away: http://img245.imageshack.us/img245/4761/picture16ry7.png

[12:31pm] ardissone|away: those are some of froodian's bookmarks bar mockups
[12:33pm] ardissone|away: we continue to need more ideas
[12:33pm] mento: each time i look at these, i think we need to fix the tab bar
[12:33pm] ardissone|away: yes
[12:33pm] ardissone|away: sam said "one at a time"
[12:34pm] mento: these bad boys are right next to each other
[12:34pm] ardissone|away: tell that to sam 
[12:34pm] mento: i think they should be part of a visually coherent desig
[12:34pm] ardissone|away: i agree
[12:34pm] mento: ss: come out from under your rock, you coward

[12:44pm] pinkerton: http://img150.imageshack.us/img150/6138/picture10wr1.png isn't bad
[12:44pm] pinkerton: not awesome, but not horrid
We haven't reached a real consensus on this yet, so it needs to get on the train at the next 1_5 station, not this one.
Flags: camino1.5.5?
Flags: camino1.5.4?
Flags: camino1.5.4-
Since everyone except Sam agrees we need to look at them together, adjusting the summary to include the Tab bar.  mento believes this is a must for 1.6.
Flags: camino1.6b1?
Summary: [10.5] Update bookmark bar for leopard → [10.5] Update Bookmark Bar and Tab bar for Leopard

Comment 13

11 years ago
Comment on attachment 286377 [details] [diff] [review]
patch v1

>+- (void)setTitle:(NSString *)aString
>+{
>+  // Give the title etched attributes on >=10.5
>+  if (aString != nil && [NSWorkspace isLeopardOrHigher])
>+    aString = [NSAttributedString etchedStringWithString:aString];
>+  
>+  [super setTitle:aString];
>+}
>+

By setting the title here as a NSAttributedString we are losing the font size '[NSFont labelFontOfSize:11.0]' which is set when initialized. This causes the etched text to look big and awkward as you can see in the attached screenshot.

I suggest adding a parameter withAttributes: to the function below so that we can pass the font attribute along.

>+@implementation NSAttributedString (CaminoAttributedStringUtils)
>+
>++ (NSString*)etchedStringWithString:(NSString *)string
>+{
>+  static NSDictionary* shadowAttributes = nil;
>+  if (!shadowAttributes) {
>+    NSShadow* shadow = [[[NSShadow alloc] init] autorelease];
>+    [shadow setShadowOffset:NSMakeSize(0.0, -2.0)];
>+    [shadow setShadowColor:[NSColor colorWithCalibratedWhite:1.0 alpha:0.6]];
>+    shadowAttributes = [[NSDictionary dictionaryWithObject:shadow forKey:NSShadowAttributeName] retain];
>+  }
>+
>+  return [[[NSAttributedString alloc] initWithString:string attributes:shadowAttributes] autorelease];
>+}

Shouldn't the return type be NSAttributedString since that is type you will always get back? 

For calarity change the function to something like
(NSAttributedString*)etchedAttributedStringWithString:(NSString*)string withAttributes:(NSDictionary*)attributes


I still think the gradient is too dark at the bottom and there is too much of a difference in color from top to bottom. When the bookmark toolbar is only one line it appears to "jut out" too far. The screenshots so far show what it looks like with multiple lines; however, since we ship with one line of bkmks as default, I think that should be the area of our focus and not these fringe cases.

Are we going to go with this as a quick fix or are we going to go the HITheme route as mentioned in Bug 381494 ?
Attachment #286377 - Flags: review?(Jeff.Dlouhy) → review-
Flags: camino1.5.5? → camino1.5.5-

Comment 14

11 years ago
Created attachment 302991 [details] [diff] [review]
bookmark bar incremental improvement

I'd like to land this as a short-term improvement to the bookmark bar while we rethink the whole theme for 2.0. This changes to using the "light metal" gradient that is used in a few places in the OS (the Mail and Finder search bars come to mind), which is a very subtle change from the current gradient, but one that I think help us feel less out of place.

It also changes to draw that gradient even in inactive windows, which is the behavior of the light metal in the OS--I've found that this makes a pretty noticeable improvement in the feel of background windows.

Since the adjustments are so minor we don't have to tackle any sticky issue of text, favicons, or blending with the tab bar, making it an easy change.
Attachment #302991 - Flags: superreview?(mikepinkerton)

Comment 15

11 years ago
Created attachment 302992 [details]
current active look

Comment 16

11 years ago
Created attachment 302993 [details]
new active look

Comment 17

11 years ago
Created attachment 302994 [details]
current inactive look

Comment 18

11 years ago
Created attachment 302995 [details]
new inactive look

Comment 19

11 years ago
Created attachment 303038 [details] [diff] [review]
bookmark bar incremental, v2 [landed]

I decided to pull the major chunks of drawRect: out into their own pieces, as we do in the tab bar, just to keep things more manageable. The code is still all the same (except that we only draw the background in the horizontal range of the invalidation rect, just to avoid pointless work), just moved into new methods.
Attachment #302991 - Attachment is obsolete: true
Attachment #303038 - Flags: superreview?(mikepinkerton)
Attachment #302991 - Flags: superreview?(mikepinkerton)

Comment 20

11 years ago
-'ing for 1.6, given time/resource constraints
Flags: camino1.6b3? → camino1.6b3-
Comment on attachment 303038 [details] [diff] [review]
bookmark bar incremental, v2 [landed]

sr=pink
Attachment #303038 - Flags: superreview?(mikepinkerton) → superreview+

Comment 22

11 years ago
Comment on attachment 303038 [details] [diff] [review]
bookmark bar incremental, v2 [landed]

Landed on trunk and MOZILLA_1_8_BRANCH.
Attachment #303038 - Attachment description: bookmark bar incremental, v2 → bookmark bar incremental, v2 [landed]

Comment 23

11 years ago
philippe, would you be insterested in submitting your medium-grey version as a possibility for the official build (at least until/unless we completely rethink the look of both bars)? I think it's worth seriously considering. If so, could you attach your screenshot here to made discussion easier (and for posterity)?
(In reply to comment #23)

> the look of both bars)? I think it's worth seriously considering. If so, could
> you attach your screenshot here to made discussion easier (and for posterity)?

I think for anything we're seriously considering we really need 3 screenshots:

1) Toolbar, single-line bookmarks bar (with assorted site icons), tab bar (3-4 tabs + empty bar)
2) Toolbar, multi-line (4-5) bookmarks bar (with assorted site icons), tab bar (3-4 tabs + empty bar)
3) Toolbar, tab bar (3-4 tabs + empty bar)

Obviously the chances of reaching the 10.3 nirvana again are slim, but it's hard to make decisions about what will work best overall based on just one of those screenshots.

(I do think philippe's setup is worth serious consideration.)
(Assignee)

Comment 25

11 years ago
Created attachment 316155 [details]
medium grey BMbar, single row

single row bookmarks bar, with 2nd window in the background
(Assignee)

Comment 26

11 years ago
Created attachment 316156 [details]
medium grey, no BM bar
(Assignee)

Comment 27

11 years ago
Created attachment 316158 [details]
medium grey, multi row BM bar

4 rows, assorted icons.
(Assignee)

Comment 28

11 years ago
Created attachment 316160 [details]
medium grey multi-row BM bar, background window

multi-row BM bar in a background window
(Assignee)

Comment 29

11 years ago
Created attachment 316162 [details] [diff] [review]
the color values used in BookmarkToolbar.mm [landed]
(Assignee)

Updated

11 years ago
Attachment #316162 - Attachment mime type: application/octet-stream → text/pain
(Assignee)

Updated

11 years ago
Attachment #316162 - Attachment mime type: text/pain → text/plain
(Assignee)

Comment 30

11 years ago
Created attachment 316163 [details]
zipped images used for the tab bar

the images used for the active tab and the background of the tab bar

Comment 31

10 years ago
Somehow we forgot about these for a year :(

Do you have updated images for the taller tab bar? We want to go ahead and land your version and see what everyone things in day-to-day use.
Target Milestone: --- → Camino2.0
(Assignee)

Comment 32

10 years ago
Created attachment 390130 [details]
images for tab bar, 2009version
Attachment #316163 - Attachment is obsolete: true
Comment on attachment 316162 [details] [diff] [review]
the color values used in BookmarkToolbar.mm [landed]

Patch still applies ;)

The smfr-bar+tabs looks a lot worse here (because you have a much bigger expanse of darker color before you hit the active tab), which I think is unfortunate because we're the only browser that supports it, but I don't know what we can do.

For the single-line case, it's an improvement without going too dark :)

We may also want to consider later making the active tab duller/slightly darker still; it turns out it's much brighter than I expected, or it just sticks out more since the bmbar is now much darker.

But this still seems to escape the dark foreboding grey-black clouds of Steve's toy and its chief copy-cat.
Attachment #316162 - Attachment is patch: true
Attachment #316162 - Flags: superreview?(stuart.morgan+bugzilla)
(Assignee)

Comment 34

10 years ago
(In reply to comment #33)

> The smfr-bar+tabs looks a lot worse here (because you have a much bigger
> expanse of darker color before you hit the active tab), which I think is
> unfortunate because we're the only browser that supports it, but I don't know
> what we can do.

My partner is a smfr-bar user :-( 300miljon bm's and counting.
She's been surfing with those colours for a long while and it doesn't annoy here. I experimented once to make it lighter at the bottom, but that looked poor for the single-line case.
 
> We may also want to consider later making the active tab duller/slightly darker
> still; it turns out it's much brighter than I expected, or it just sticks out
> more since the bmbar is now much darker.

It contrast more with the BM bar, yes. I think I mentioned once in the forum making it darker, it then vanished into the tab-bar.
Note that with 10.6, Apple is said to set the monitor gama to 2.2 (from the traditional mac-gama of 1.8). On one of the iMacs here, I did the switch, and the contrast is much less evident.
(System Prefs > Displays > Color > Calibrate - don't turn expert mode, just check the box and save. Restart the browser)

> But this still seems to escape the dark foreboding grey-black clouds of Steve's
> toy and its chief copy-cat.
LOL.
(Assignee)

Comment 35

10 years ago
Created attachment 390388 [details]
 images for tab bar, 2009version-1b

There was an odd pixel in the tab_bar_bg image.
Attachment #390130 - Attachment is obsolete: true
(Assignee)

Comment 36

10 years ago
Created attachment 390390 [details]
 images for tab bar, 2009version-2a

darker active tab, take 1
(In reply to comment #33)
(In reply to comment #36)
> Created an attachment (id=390390) [details]
>  images for tab bar, 2009version-2a
> 
> darker active tab, take 1
> (In reply to comment #33)

:sigh: I think the 2a active tab "fits" better with the darker bmbar, but I find it's now much harder to pick out from among the inactive tabs, which makes the change less useful :-(  It also seems flatter (as in less rounded), which may also be part of the problem.
One other thing I've noticed: the old problem with the tab corner is back; it's not smooth, or less defined, or something.  I see it on both the left and right sides, but I just Pixie-capped the right side:

http://www.ardisson.org/smokey/moz/tab-corner-orig.png (current tabs)
http://www.ardisson.org/smokey/moz/tab-corner-phiw-1.png (tabs 1b)
http://www.ardisson.org/smokey/moz/tab-corner-phiw-2.png (tabs 2a)

I can't identify exactly what pixel is wrong, even with Pixie (maybe the 2nd pixel below the bmbar/tab bar dividing line, on the rightmost column of that tab_right_side image?), but I can tell something's wrong there in both v1 and v2 active tabs ;)
(Assignee)

Comment 39

10 years ago
(In reply to comment #37)
> (In reply to comment #36)
> > Created an attachment (id=390390) [details] [details]
> >  images for tab bar, 2009version-2a
> > 
> > darker active tab, take 1
> > (In reply to comment #33)
> 
> :sigh: I think the 2a active tab "fits" better with the darker bmbar, but I
> find it's now much harder to pick out from among the inactive tabs, which makes
> the change less useful :-(  It also seems flatter (as in less rounded), which
> may also be part of the problem.

maybe something in between 1 and 2 ? Lighter at the top ? I'll give that a try.
And yes, 2a is less rounded. Making the middle (darkest) part darker quickly went to far. I'll have a look at that tab corner.
(Assignee)

Comment 40

10 years ago
Created attachment 390423 [details]
images for tab bar, 2009version-3a   

The top part of the tab is a wee little bit lighter than v2a to accentuate the rounding.
(Assignee)

Comment 41

10 years ago
Created attachment 390446 [details]
images for tab bar, 2009version-1c

The lighter active tab, with better edges - I actually reuse the existing image here.
Attachment #390388 - Attachment is obsolete: true
(Assignee)

Comment 42

10 years ago
Created attachment 390447 [details]
images for tab bar, 2009version-3b

medium grey (v3), better contrast and rounding, I hope. Still feels a wee little dark overall to me.
Attachment #390423 - Attachment is obsolete: true
(In reply to comment #38)
http://www.ardisson.org/smokey/moz/tabsv2/tab-corner-orig.png (current tabs)
http://www.ardisson.org/smokey/moz/tabsv2/tab-corner-phiw-1c.png (1c tabs)
http://www.ardisson.org/smokey/moz/tabsv2/tab-corner-phiw-3b.png (3b tabs)

The corner in 1c is still fuzzy; I think it's the lighter pixel that's in the same row as the "shadow" (that appears to be coming out of under the bmbar) that's the problem; that pixel blended better with the tab bar bg colors next to it in the current tabs.

The corner in 3b is much better; maybe not quite perfect, but not terribly noticeable.  I think it's the same pixel there; it's darker, so better, but could be slightly darker?

I'm going to leave the 3b tabs in that build for a while and see how they feel (particularly when I'm not tired); in the meantime, there are some screenshots of 1c and 3b:

http://www.ardisson.org/smokey/moz/tabsv2/tabs-1c-bm-tab.png
http://www.ardisson.org/smokey/moz/tabsv2/tabs-1c-smfr-tab.png
http://www.ardisson.org/smokey/moz/tabsv2/tabs-1c-tab.png

http://www.ardisson.org/smokey/moz/tabsv2/tabs-3b-bm-tab.png
http://www.ardisson.org/smokey/moz/tabsv2/tabs-3b-smfr-tab.png
http://www.ardisson.org/smokey/moz/tabsv2/tabs-3b-tab.png
(Assignee)

Comment 44

10 years ago
(In reply to comment #43)

So what would be the preferred colour for the active tab background ? 1 or 3 ?
the corner pixel is fixable (I see it with the 'light' -1c tabs, but not with the darker 3b tabs on my monitors & my brightness settings. Pushing the brightness settings to full max on the iMac reveals the pixel slightly.)

Comment 45

10 years ago
I prefer 3 to 1. In both cases, the sharp glossy look seems out of place next to the softer gradients though. How would it look if the active tab just used the bookmark bar gradient again?
(In reply to comment #44)

> the corner pixel is fixable (I see it with the 'light' -1c tabs, but not with
> the darker 3b tabs on my monitors & my brightness settings. Pushing the
> brightness settings to full max on the iMac reveals the pixel slightly.)

I don't "see" the pixel itself in 3b like I do in 1c; I can just tell there's something not smooth about the corners.

1 is too bright, for sure.  I think 3 is OK, and it's not too hard to distinguish from the background tabs/bar.  I don't see any "sharp glossy look" to it, though.
(Assignee)

Comment 47

10 years ago
Created attachment 392024 [details]
screenshot: reuse BM bar gradient

(In reply to comment #45)
> I prefer 3 to 1. In both cases, the sharp glossy look seems out of place next
> to the softer gradients though. How would it look if the active tab just used
> the bookmark bar gradient again?

Something like that ?
(Assignee)

Comment 48

10 years ago
Created attachment 392027 [details]
screenshot - reuse gradient

reuse the gradient from the BM bar, try 2
(Assignee)

Comment 49

10 years ago
Created attachment 392051 [details]
images for tab-bar, v3d

glossy tabs, medium grey.
Attachment #390390 - Attachment is obsolete: true
Attachment #390446 - Attachment is obsolete: true
Attachment #390447 - Attachment is obsolete: true
Attachment #392024 - Attachment is obsolete: true
Attachment #392027 - Attachment is obsolete: true
Attachment #392051 - Flags: review?
(Assignee)

Comment 50

10 years ago
Created attachment 392052 [details]
images for tab-bar, v4a [landed]

the active tab reuses the BM bar gradient
(Assignee)

Comment 51

10 years ago
A couple of screenshots with the tabs in context:
1. the glossy tab - single line BM bar
http://dev.l-c-n.com/camino/tab_bar/t3-1.png
2. the glossy tab - smfr style BM bar
http://dev.l-c-n.com/camino/tab_bar/t3-2.png
3. the full gradient tab - single line BM bar
http://dev.l-c-n.com/camino/tab_bar/t4-1.png
4. the full gradient tab - smfr style BM bar 
http://dev.l-c-n.com/camino/tab_bar/t4-2.png

Updated

10 years ago
Attachment #316162 - Flags: superreview?(stuart.morgan+bugzilla) → superreview+

Comment 53

10 years ago
Comment on attachment 316162 [details] [diff] [review]
the color values used in BookmarkToolbar.mm [landed]

Hm, I could have sworn I sr'd this a while ago.

sr=smorgan; I'd like to try with the 4-style tabs. I think we may end up wanting to darken the bottom divider since it seems more disconnected from the active tab than it does with the current graphics, but let's land, use it for a while and see what we think.
Checked in attachment 316162 [details] [diff] [review] and images from attachment 392052 [details] on cvs trunk and CAMINO_2_0_BRANCH.

We should keep an eye (ear) out for problems on 10.4 and for comment 53, but marking this FIXED as we have a solution we no longer feel is entirely stop-gap.
Assignee: froodian → phiw
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Whiteboard: [camino-2.0]
Attachment #316162 - Attachment description: the color values used in BookmarkToolbar.mm → the color values used in BookmarkToolbar.mm [landed]
Attachment #392052 - Attachment description: images for tab-bar, v4a → images for tab-bar, v4a [landed]
You need to log in before you can comment on or make changes to this bug.