Closed Bug 393718 Opened 14 years ago Closed 14 years ago

Style searchbar splitter nicely

Categories

(Firefox :: Toolbars and Customization, defect, P3)

x86
All
defect

Tracking

()

RESOLVED FIXED
Firefox 3 beta3

People

(Reporter: aja+bugzilla, Assigned: dao)

References

(Depends on 1 open bug)

Details

(Keywords: polish)

Attachments

(1 file, 15 obsolete files)

6.13 KB, patch
mano
: review+
Details | Diff | Splinter Review
Searchbar's splitter looks bad since landing of bug 373696 (which styles toolbarbar separators natively).
Keywords: polish
OS: Windows XP → All
Duplicate of this bug: 394022
it looked bad even before landing of bug 373696.
Flags: blocking-firefox3?
Flags: blocking-firefox3? → blocking-firefox3+
FWIW, "Separator" would look better styled natively as well.
Want a separate bug?
Target Milestone: --- → Firefox 3 M10
I have something which will make the splitter look a lot better and more native. If I post it here now would it get M9 approval?
(In reply to comment #3)
> FWIW, "Separator" would look better styled natively as well.
> Want a separate bug?

The separator is style natively on trunk (attachment 278239 [details] [diff] [review]).

(In reply to comment #4)
> I have something which will make the splitter look a lot better and more
> native. If I post it here now would it get M9 approval?

Depends on how good your solution is ;)
Attached image splitter-ew.png (obsolete) —
I think this is the style that Windows uses for toolbar splitters. This was made in the GIMP using very transparent black, so it should fit in with any theme and still look "punctured" in.

This goes in toolkit/themes/winstripe/global/splitter/.
Attached image splitter-ns.png (obsolete) —
This goes in the same folder.
Target Milestone: Firefox 3 M10 → Firefox 3 M9
Attached patch Patch (obsolete) — Splinter Review
This uses the images above, so it takes the same approach as the Pinstripe splitter. But I added an extra class to Winstripe since the current solid line is still needed for places other than the toolbar, such as the sidebar.
Assignee: nobody → ventnor.bugzilla
Status: NEW → ASSIGNED
Attachment #283954 - Flags: review?(mano)
BTW, the images follow the style of Office and IE on their toolbars.
Comment on attachment 283952 [details]
splitter-ew.png

This won't work with a dark background. I also don't think the CSS belongs into toolkit or that this is generally a good fix (as in: it doesn't style splitters natively, which would involve -moz-appearance), but I'll leave that to mano.
Attachment #283952 - Flags: review-
Attachment #283953 - Flags: review-
Comment on attachment 283954 [details] [diff] [review]
Patch

whathesaid, except the style rules will likely end up in toolkit's toolbar.css.
Attachment #283954 - Flags: review?(mano) → review-
(In reply to comment #10)
> (From update of attachment 283952 [details])
> This won't work with a dark background. I also don't think the CSS belongs into
> toolkit or that this is generally a good fix (as in: it doesn't style splitters
> natively, which would involve -moz-appearance), but I'll leave that to mano.
> 

Pinstripe uses images as well instead of a native style. I don't know Windows widgets and I don't think GTK has an equivalent of this.
Pinstripe doesn't have to support different color schemes, as far as I know. Your images will just disappear with a black high-contrast theme, for example. Maybe use something like |border-left: 3px dotted ThreeDShadow| instead?
(In reply to comment #11)
> (From update of attachment 283954 [details] [diff] [review])
> whathesaid, except the style rules will likely end up in toolkit's toolbar.css.
> 

If it is put there, wouldn't the splitter on the customize dialog not pick up the styles?
I styled this up with userchrome on my home machine to be just two pixels wide, with a 1px left border threedshadow, and 1 px right border threedhighlight (and a few extra pixels of margin to increase the hover target... not sure if that actually works or not), but the results work fine here. Screenshot attached.
Attached patch Patch 2 (obsolete) — Splinter Review
This uses a dotted border approach. I can't put the rules in toolbar.css because the splitter in the toolbar doesn't pick it up for some reason.
Attachment #283952 - Attachment is obsolete: true
Attachment #283953 - Attachment is obsolete: true
Attachment #283954 - Attachment is obsolete: true
Attachment #284120 - Flags: review?(mano)
(In reply to comment #15)
> Created an attachment (id=284119) [details]
> Screenshot of a themed splitter on XP Classic
> 
> I styled this up with userchrome on my home machine to be just two pixels wide,
> with a 1px left border threedshadow, and 1 px right border threedhighlight (and
> a few extra pixels of margin to increase the hover target... not sure if that
> actually works or not), but the results work fine here. Screenshot attached.

This looks like a normal separator.
Attachment #284119 - Attachment is obsolete: true
Attached file another possible splitter look (obsolete) —
Comment on attachment 284120 [details] [diff] [review]
Patch 2

>+splitter.dotted {
>+  min-width: 3px;
>+  margin: 3px 0;
>+  border: 0;
>+  border-left: 2px dotted ThreeDShadow;
>+}

This looks wrong. I think you want the width to be exactly zero.
Or perhaps the following with a 1px wide transparent background image

border-left:  3px dotted ButtonShadow;
border-right: 3px dotted ButtonShadow;

This was always visible in the testing I've done -- default XP theme with default (blue), olive green, and silver color schemes, and b/w and w/b.

It gives two columns of dots (staggered at most heights), like the XP taskbar's splitters have.

FWIW, whatever the color effect used on the status line's grippy's dots is looks even better to me than ButtonShadow.
(In reply to comment #19)
> (From update of attachment 284120 [details] [diff] [review])
> >+splitter.dotted {
> >+  min-width: 3px;
> >+  margin: 3px 0;
> >+  border: 0;
> >+  border-left: 2px dotted ThreeDShadow;
> >+}
> 
> This looks wrong. I think you want the width to be exactly zero.
> 

Really? It seems to me that min-width includes the border, from some testing I did with my cursor. Plus we mustn't forget Fitts' Law...
(In reply to comment #21)
> (In reply to comment #19)
> > This looks wrong. I think you want the width to be exactly zero.
> 
> Really? It seems to me that min-width includes the border, from some testing I
> did with my cursor.

It does, but since the width should be zero (otherwise you have an asymmetric padding), the min-width shouldn't be bigger than the border width.

> Plus we mustn't forget Fitts' Law...

Yet I don't think asymmetric padding is the way to go. You could create a theme-specific binding to add padding on both sides, but we want to avoid that for performance reasons. So I guess I would go without any padding, but maybe use a border width of 3px.
(In reply to comment #22)
> (In reply to comment #21)
> > (In reply to comment #19)
> > > This looks wrong. I think you want the width to be exactly zero.
> > 
> > Really? It seems to me that min-width includes the border, from some testing I
> > did with my cursor.
> 
> It does, but since the width should be zero (otherwise you have an asymmetric
> padding), the min-width shouldn't be bigger than the border width.
> 
> > Plus we mustn't forget Fitts' Law...
> 
> Yet I don't think asymmetric padding is the way to go. You could create a
> theme-specific binding to add padding on both sides, but we want to avoid that
> for performance reasons. So I guess I would go without any padding, but maybe
> use a border width of 3px.
> 

If I do that then the splitter will no longer look familiar. The toolbar splitters of other Windows apps have 2px dots.
It doesn't look familiar to me either way. Toolbar splitters are usually attached to a toolbar border.
(In reply to comment #24)
> It doesn't look familiar to me either way. Toolbar splitters are usually
> attached to a toolbar border.
> 

But it was the closest thing I could find as a consistent design for a toolbar splitter.
The Windows splitters don't even work similarly. Maybe we should abandon the idea to style the search bar splitter natively, and chose a layout that works best for us.

I like this one:

border-style: dotted;
border-color: ThreeDShadow;
border-width: 0 1px;
margin: 3px 0;
min-width: 4px;
I don't think two lines is very suitable. The user would probably think its a flexible space between, and each line is used to expand/shrink it. At least thats what I would think. Or they could probably think its an empty toolbar inside.

IMO, the 2px dotted approach is best.
Let's go with this for now, I don't think we're going to have the splitter visible in non-customizing mode past b1 anyway so I don't think this needs to be final.
Whiteboard: [has patch][need review mano]
Comment on attachment 284120 [details] [diff] [review]
Patch 2


>+  templateNode.className = "dotted";

If you're going to use a class here, it should have a name like 'toolbar-splitter', not 'dotted', so it doesn't imply that it has to appear in a specific way, which won't be true on other platforms.
(In reply to comment #28)
> Let's go with this for now, I don't think we're going to have the splitter
> visible in non-customizing mode past b1 anyway so I don't think this needs to
> be final.

In the customizing mode, wouldn't the current splitter actually do a good job?
Attached patch Change class name (obsolete) — Splinter Review
Don't assume the splitter will take on a dotted look.
Attachment #284120 - Attachment is obsolete: true
Attachment #285159 - Flags: review?(mano)
Attachment #284120 - Flags: review?(mano)
Comment on attachment 285159 [details] [diff] [review]
Change class name

>+splitter.toolbar-splitter {
>+  min-width: 3px;
>+  margin: 3px 0;
>+  border: 0;
>+  border-left: 2px dotted ThreeDShadow;
>+}

http://developer.mozilla.org/en/docs/Writing_Efficient_CSS#Don.27t_qualify_class-categorized_rules_with_tag_names
Attachment #285159 - Attachment is obsolete: true
Attachment #285159 - Flags: review?(mano)
Attached patch Acceptable CSS (obsolete) — Splinter Review
Can you get the review done very soon please, Mano?
Attachment #285210 - Flags: review?(mano)
Comment on attachment 285210 [details] [diff] [review]
Acceptable CSS

Mano might not have enough time... mconnor if you ever have time before Mano gets to review this can you do it?
Attachment #285210 - Flags: review?(mconnor)
Where's the orient attribute for a .toolbar-splitter set?
I guess I did that for consistency, someone might want it in the future.
For vertical toolbars? ;)

One more thing:
(In reply to comment #28)
> Let's go with this for now, I don't think we're going to have the splitter
> visible in non-customizing mode past b1 anyway so I don't think this needs to
> be final.

The current development (bug 400327) looks like the splitter could eventually remain in the non-customizing mode.
(In reply to comment #37)
> For vertical toolbars? ;)

It was a leftover from the original "dotted" class :)
It doesn't have to be for a toolbar, but I guess you're right.
Attached patch New patch (obsolete) — Splinter Review
Attachment #285210 - Attachment is obsolete: true
Attachment #285931 - Flags: review?(mano)
Attachment #285210 - Flags: review?(mconnor)
Attachment #285210 - Flags: review?(mano)
Comment on attachment 285931 [details] [diff] [review]
New patch

s/border-left/-moz-border-start/
s/border: 0/border: none

r=mano otherwise.
Attachment #285931 - Flags: review?(mano) → review+
Attached patch For checkin (obsolete) — Splinter Review
This patch is meant to improve the look of the thick and ugly splitter for Winstripe users. The reason I'm asking for M9 is because this patch is mostly no-risk CSS and this will fix the inevitable flood of complaints about this high-exposure splitter should it keep its current look into our first beta.
Attachment #285931 - Attachment is obsolete: true
Attachment #285983 - Flags: approvalM9?
Whiteboard: [has patch][need review mano]
Comment on attachment 285983 [details] [diff] [review]
For checkin

this is an M9 blocker, doesn't need approval, but just so there's no confusion...
Attachment #285983 - Flags: approvalM9? → approvalM9+
Checking in toolkit/content/customizeToolbar.js;
/cvsroot/mozilla/toolkit/content/customizeToolbar.js,v  <--  customizeToolbar.js
new revision: 1.37; previous revision: 1.36
done
Checking in toolkit/content/widgets/toolbar.xml;
/cvsroot/mozilla/toolkit/content/widgets/toolbar.xml,v  <--  toolbar.xml
new revision: 1.38; previous revision: 1.37
done
Checking in toolkit/themes/winstripe/global/splitter.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/splitter.css,v  <--  splitter.css
new revision: 1.6; previous revision: 1.5
done
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007102504 Minefield/3.0a9pre. The splitter nows looks like the screenshot in https://bugzilla.mozilla.org/attachment.cgi?id=284141.
Status: RESOLVED → VERIFIED
It's great that we got this cleaned up for M9, but I'm Reopening for M10 since this didn't go through ui-r for the first iteration and this design appears in a very high visibility interface. I'll follow up with a suggested design after we have made a little more progress in determining the planned control scheme and I've had a chance to get beltzner's input.

My initial take on the issue is that the small circle works well in the new mac theme, but we should not have any affordance on windows (outside of a cursor change on hover) to reduce overall visual complexity.  Discoverability should be proportional to how much something actually needs to be discovered.  Discoverability is also a zero-sum game, every affordance we add reduces the discoverability of surrounding affordances.
Status: VERIFIED → REOPENED
Flags: blocking-firefox3+ → blocking-firefox3?
Resolution: FIXED → ---
Summary: Style searchbar splitter natively → Style searchbar splitter nicely
Depends on: 393733, 400327
Flags: blocking-firefox3? → blocking-firefox3+
Target Milestone: Firefox 3 M9 → Firefox 3 M10
Target Milestone: Firefox 3 M10 → Firefox 3 M11
Attached image toolbar-splitter.png (obsolete) —
This is taken from the current Mac theme, not the new one. It would work well for all kinds of toolbar backgrounds.

I do however think we shouldn't always show the splitter as it's not generally needed. If we follow bug 393733, I would suggest a different, less subtle appearance.
Attached patch patch for previous image (obsolete) — Splinter Review
Attached image screenshot (obsolete) —
Priority: -- → P3
Attachment #288153 - Flags: ui-review?(beltzner)
Comment on attachment 288153 [details]
screenshot

I think this is fine, but want to give Alex a chance to weigh in as he's directing the theme work for Firefox 3
Attachment #288153 - Flags: ui-review?(beltzner) → ui-review?(faaborg)
Attached image another screenshot (obsolete) —
That's the native appearance on Ubuntu. We could imitate this on Windows.
Attachment #295445 - Flags: ui-review?(beltzner)
Attachment #295445 - Flags: ui-review?(beltzner) → ui-review?(faaborg)
(In reply to comment #50)
> Created an attachment (id=295445) [details]
> another screenshot
> 
> That's the native appearance on Ubuntu. We could imitate this on Windows.

What do you mean by "native appearance on Ubuntu"? This screenshot isn't what I see on Ubuntu for current trunk.
(In reply to comment #51)
> What do you mean by "native appearance on Ubuntu"?

-moz-appearance: splitter
Comment on attachment 288153 [details]
screenshot

I personally believe that we should remove the affordance entirely for simplicity.  This isn't a feature that needs to be discoverable, but if you happen to know about it or notice the cursor change on hover, it's nice to have.  This is one area where we can even pull ahead of Safari in terms of clean design.
Attachment #288153 - Flags: ui-review?(faaborg) → ui-review-
I've been in favor of bug 393733, but making the splitter invisible sounds like an interesting idea, actually... It's also the easiest thing to do.
Comment on attachment 295445 [details]
another screenshot

While consistent with the visual affordance for resizing on GTK splitters and the GNOME panel and window list, I still think this is visually too heavy.  Is there any way we could get this affordance to appear only when the customization pallet is being displayed?  I'm pretty sure that is the only situation where I personally want it to be visible.  Even though it is a platform convention, the grippy part is just striking me as feeling more Netscape 4.x than Firefox 1.
Attachment #295445 - Flags: ui-review?(faaborg) → ui-review-
Attachment #288150 - Attachment is obsolete: true
Attachment #288151 - Attachment is obsolete: true
(In reply to comment #55)
> Is there any way we could get this affordance to appear only when the
> customization pallet is being displayed?

Visually, yes, but I suppose making it functional in the customizing mode isn't trivial.
Attached patch remove the affordance (obsolete) — Splinter Review
Assignee: ventnor.bugzilla → dao
Attachment #284141 - Attachment is obsolete: true
Attachment #285983 - Attachment is obsolete: true
Attachment #288153 - Attachment is obsolete: true
Attachment #295445 - Attachment is obsolete: true
Status: REOPENED → ASSIGNED
Attachment #295517 - Flags: review?(mano)
Comment on attachment 295517 [details] [diff] [review]
remove the affordance

>Index: browser/themes/gnomestripe/browser/browser.css

>+/*XXX The adjacent selector isn't working reliably

Is there a bug filed on that?
I think it's known, but I couldn't find a bug.
I suppose it's bug 229915
Yeah, that's the one. Now that I know the bug, I can actually hack around it.
Attachment #295517 - Attachment is obsolete: true
Attachment #295517 - Flags: review?(mano)
Attachment #295672 - Flags: review?(mano)
Hrm, so, why are you changing splitter.css? Other consumers may still want this. Also, what's the resize-interaction like after this change? would it "start" at some random point left to the search-bar?
Or you should keep the toolbar-splitter class set and claim in splitter.css that the element is invisible... What do other windows applications do here?
(In reply to comment #63)
> Hrm, so, why are you changing splitter.css?

The style was set in splitter.css because the class was set in toolkit code. That's not the case anymore.

> Other consumers may still want this.

Well, I don't see other consumers. Also, the styling wasn't ui-reviewed, which is why Alex reopened this bug.

> Also, what's the resize-interaction like after this change? would it
> "start" at some random point left to the search-bar?

Right at the edge.

(In reply to comment #64)
> Or you should keep the toolbar-splitter class set and claim in splitter.css
> that the element is invisible... What do other windows applications do here?

I don't know other applications with a toolbar splitter. If there are any, they are probably not using an OS widget, because (as far as I know) there is none.
(In reply to comment #65)
> (In reply to comment #64)
> > Or you should keep the toolbar-splitter class set and claim in splitter.css
> > that the element is invisible... What do other windows applications do here?
> 
> I don't know other applications with a toolbar splitter.

Oh, there's Safari 3 for Windows, but obviously it doesn't even try to look native.
>I don't know other applications with a toolbar splitter. If there are any, they
>are probably not using an OS widget, because (as far as I know) there is none.

We still see splitters in cases like when you want to modify the toolbars on your task bar in windows, but the task bar now needs to be unlocked first, since user's were accidently resizing things without intending to.  Since you can't actually drag something out of our toolbar hopefully this will be less of an issue, but I still wonder how often people will resize the search field by accident.

(In reply to comment #67)
> >I don't know other applications with a toolbar splitter. If there are any, they
> >are probably not using an OS widget, because (as far as I know) there is none.
> 
> We still see splitters in cases like when you want to modify the toolbars on
> your task bar in windows, but the task bar now needs to be unlocked first

These splitters as well as the Explorer ones are different in that they are there to resize (and move) toolbars, not toolbar elements.
>These splitters as well as the Explorer ones are different in that they are
>there to resize (and move) toolbars, not toolbar elements.

Yeah, I can't recall any controls to resize toolbar elements.  Let's drop the visual affordance on windows as well to get a cleaner look.
Comment on attachment 295672 [details] [diff] [review]
remove the affordance v2

r=mano code wise, this hurts usability IMO though :(
Attachment #295672 - Flags: review?(mano) → review+
Keywords: checkin-needed
FWIW, I've been using this for the past few days on Windows and I think it works well.
Checking in browser/themes/gnomestripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/gnomestripe/browser/browser.css,v  <--  browser.css
new revision: 1.152; previous revision: 1.151
done
Checking in browser/themes/pinstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/pinstripe/browser/browser.css,v  <--  browser.css
new revision: 1.104; previous revision: 1.103
done
Checking in browser/themes/winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.css
new revision: 1.146; previous revision: 1.145
done
Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.924; previous revision: 1.923
done
Checking in toolkit/themes/winstripe/global/splitter.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/splitter.css,v  <--  splitter.css
new revision: 1.7; previous revision: 1.6
done
Status: ASSIGNED → RESOLVED
Closed: 14 years ago14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
(In reply to comment #65)

> 
> I don't know other applications with a toolbar splitter. If there are any, they
> are probably not using an OS widget, because (as far as I know) there is none.
> K-Meleon uses a whole bunch of splitters on their toolbars, which look very similar to the one that this bug is about.  

K-Meleon's splitters are similar to those of Windows Explorer.
With Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008011105 Minefield/3.0b3pre there isn't a splitter anymore. Got it removed somewhere else?
(In reply to comment #75)
> With Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre)
> Gecko/2008011105 Minefield/3.0b3pre there isn't a splitter anymore. Got it
> removed somewhere else?

It's invisible. :(

I preferred the old one...
In the Mac default theme, the address bar and search bar are now too close together.
(In reply to comment #77)
Last I heard the new Mac theme should land for beta 3, so I'm not sure if tweaking this now is worthwhile. Note that the width of the splitter can be anything above 8 px.
Blocks: proto
(In reply to comment #78)
> Last I heard the new Mac theme should land for beta 3, so I'm not sure if
> tweaking this now is worthwhile. Note that the width of the splitter can be
> anything above 8 px.

Dao, I didn't only talk about OS X. There is still no splitter visible at least on Windows. Is this the final decision? IMO it makes hard to understand why an empty are inside the location bar could be used to resize only this two elements.
(In reply to comment #79)
> There is still no splitter visible at least on Windows. Is this the final
> decision?

Nothing is set in stone, but it's the most recent decision and could likely be the final one.

I also support this decision, as it makes the UI less cluttered. The splitter isn't something that you need to see all the time. The cursor makes it discoverable, as Alex already pointed out rightly.
The splitter is a functional area of the UI. These areas are in my opinion better marked as such.

For example, have you ever visited a website which had links styled with no underline? It looks less cluttered, but very clumsy to use.

I believe this is true in general. If the look on Windows Vista appeared too cluttered or «heavy», that is a flaw of the Windows widget designs or theme.

Now, a toolbar splitter is not a very important UI element, but I am using GNOME with the Fedora Nodoka theme, and splitters already have a very subtle appearance, almost invisible. Still, I much prefer them to be there instead of having an invisible UI element.

And if this is an issue of trying to make Firefox look native, and prominent splitters are part of the native look, such as on NeXT/Cocoa, doesn't it make Firefox feel foreign not to use them?
Duplicate of this bug: 409541
Depends on: 418463
You need to log in before you can comment on or make changes to this bug.