460756 - Put the search box in the main toolbar of bookmarks manager

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[Stefan [:stefanh]]

I think it'd look better if we had the search box in the main toolbar and removed the secondary toolbar. The current look is not so nice (at least on mac - the search box is stretched across the whole toolbar) and it feels a bit like an overkill to have a toolbar for just a search box.

Comment 1

[Stefan [:stefanh]]

Attachment: proposed patch

This looks good to me, but I haven't tested classic nix/win. For Modern, I use the same padding etc on the toolbaritem as we do on toolbarbuttons. Same goes for the label styles.  There's a slight glitch in that it looks like the distance between the "Search" label and the rightmost button label is not the same as the distance between the other button labels. Not sure how much this matters - it won't matter at all if people agree with bug 460694.

Comment 2

[Philip Chee]

+toolbaritem {
+  border: 1px solid transparent;
+  padding: 1px 2px;
+}
+
+toolbaritem > label {
+  margin: 0;
+  padding: 0;
+}

This isn't right. You should make this specific to your toolbaritem id="bookmarks-search-box"

Comment 3

[Stefan [:stefanh]]

Comment on attachment 343904 [details] [diff] [review]
proposed patch

I'll look at this tomorrow.

Comment 4

[Stefan [:stefanh]]

Attachment: New version with css in bookmarksWindow.css

OK, so I was wrong about needing to style the toolbaritem... I need to fix up the label, though - I do that in bookmarksWindow.css. Instead of doing "bookmarks-search-box" > label I used an id on the label. Note that fixing bug 460694 would make the css unnecessary (and of course I volunteer to fix 460694 now if you prefer that ;-)

Comment 5

[Philip Chee]

> +      <toolbaritem align="center">

Could you do me a favour and give the toolbaritem an ID? This is for the future when toolbar customization is possible. Thanks.

Comment 6

[Stefan [:stefanh]]

(In reply to comment #5)
> > +      <toolbaritem align="center">
> 
> Could you do me a favour and give the toolbaritem an ID? This is for the future
> when toolbar customization is possible. Thanks.

Sure, the id will be "bookmarks-search-box". I'll wait for Neil's reviews before I change anything, though.

Comment 7

[[e-mail]]

Comment on attachment 344518 [details] [diff] [review]
New version with css in bookmarksWindow.css

r+sr=me but only on the XUL changes. I think the CSS changes are wrong and may well become obsolete anyway.

Comment 8

[Stefan [:stefanh]]

Pushed changeset 659472bd1018 (with an id on the toolbaritem and no id on the label).

Comment 9

[Stefan [:stefanh]]

(and without css changes)

Comment 10

[[e-mail]]

It would be good to have Accel+F focus the search box (dao pointed this out).