Put the search box in the main toolbar of bookmarks manager

RESOLVED FIXED in seamonkey2.0a2

Status

RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: stefanh, Assigned: stefanh)

Tracking

Trunk
seamonkey2.0a2

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

10 years ago
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.
(Assignee)

Comment 1

10 years ago
Created attachment 343904 [details] [diff] [review]
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.
Attachment #343904 - Flags: superreview?(neil)
Attachment #343904 - Flags: review?(neil)
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → seamonkey2.0a2

Comment 2

10 years ago
+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"
(Assignee)

Comment 3

10 years ago
Comment on attachment 343904 [details] [diff] [review]
proposed patch

I'll look at this tomorrow.
Attachment #343904 - Flags: superreview?(neil)
Attachment #343904 - Flags: review?(neil)
(Assignee)

Comment 4

10 years ago
Created attachment 344518 [details] [diff] [review]
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 ;-)
Attachment #343904 - Attachment is obsolete: true
Attachment #344518 - Flags: superreview?(neil)
Attachment #344518 - Flags: review?(neil)

Comment 5

10 years ago
> +      <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.
(Assignee)

Comment 6

10 years ago
(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.

Updated

10 years ago
Attachment #344518 - Flags: superreview?(neil)
Attachment #344518 - Flags: superreview+
Attachment #344518 - Flags: review?(neil)
Attachment #344518 - Flags: review+

Comment 7

10 years ago
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.
(Assignee)

Comment 8

10 years ago
Pushed changeset 659472bd1018 (with an id on the toolbaritem and no id on the label).
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Assignee)

Comment 9

10 years ago
(and without css changes)

Comment 10

10 years ago
It would be good to have Accel+F focus the search box (dao pointed this out).
You need to log in before you can comment on or make changes to this bug.