Last Comment Bug 575956 - Customize Address Book toolbars
: Customize Address Book toolbars
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Address Book & Contacts (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: seamonkey2.1a3
Assigned To: Ian Neal
:
Mentors:
Depends on: 882178
Blocks: 576403 576402
  Show dependency treegraph
 
Reported: 2010-06-30 04:50 PDT by Ian Neal
Modified: 2013-06-13 08:10 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
WIP patch (11.89 KB, patch)
2010-06-30 11:40 PDT, Ian Neal
no flags Details | Diff | Review
Customizable toolbar patch v0.1 (20.62 KB, patch)
2010-06-30 15:21 PDT, Ian Neal
philip.chee: review-
Details | Diff | Review
Customizable toolbar patch v0.1a (27.93 KB, patch)
2010-07-01 16:58 PDT, Ian Neal
philip.chee: review+
Details | Diff | Review
Customizable toolbar with removable patch v0.1b [Checkin: Comment 13] (28.22 KB, patch)
2010-07-12 16:44 PDT, Ian Neal
philip.chee: review+
neil: superreview+
Details | Diff | Review

Description Ian Neal 2010-06-30 04:50:09 PDT
Other parts of SM have customizable toolbars so AB should too.
Comment 1 Ian Neal 2010-06-30 11:40:24 PDT
Created attachment 455194 [details] [diff] [review]
WIP patch

This is very much WIP
Still need to move default buttons out of the palette, do CSS work and find out why the customization doesn't work
Comment 2 Ian Neal 2010-06-30 15:21:12 PDT
Created attachment 455282 [details] [diff] [review]
Customizable toolbar patch v0.1

This patch:
* Makes toolbar in the address book customizable

I've tried having the default icons outside of the toolbarpalette but you just end up with icons you cannot move from the toolbar.
Comment 3 Ian Neal 2010-06-30 15:22:17 PDT
(In reply to comment #2)
> Created an attachment (id=455282) [details]
> Customizable toolbar patch v0.1
> 
> This patch:
> * Makes toolbar in the address book customizable
> 
> I've tried having the default icons outside of the toolbarpalette but you just
> end up with icons you cannot move from the toolbar.

Oh I also changed the id for delete to make it unique.
Comment 4 Philip Chee 2010-07-01 03:10:44 PDT
Comment on attachment 455282 [details] [diff] [review]
Customizable toolbar patch v0.1

Bug 575956 - Customize Address Book toolbars

> I've tried having the default icons outside of the toolbarpalette but you just
> end up with icons you cannot move from the toolbar.

Strange. That's what Minefield does. I wonder what we/they are doing differently.

> Oh I also changed the id for delete to make it unique.
Unique in what sense? FYI Thunderbird uses "button-abdelete"

> +  <toolbar type="menubar"
> +           id="ab-toolbar-menubar2"

Thunderbird uses: "addrbook-toolbar-menubar2" Too verbose, but I suppose Mnyromyr will insist on following Thunderbird slavishly.

> +      <menubar id="ab-menubar">

Thunderbird uses: "mail-menubar" Too generic I fear. Perhaps Thunderbird should change instead of us?

> +  <toolbar class="chromeclass-toolbar toolbar-primary"
> +           id="abToolbar"

Thunderbird->"ab-bar2". Sigh.

> +    <toolbaritem id="searchBox"

Thunderbird uses "search-container". *We* use "search-container" in mailnews (IIRC it was your patch too). Note must change the .js as well. However it is unclear why Thunderbird increased the timeout to 800ms though.

> +    <toolbaritem id="throbber-box" align="center">

In small icon mode (e.g. move it to the menubar) the background should be removed e.g. { background: none; }. Bonus do it for the main mailnews window as well.

Unfortunately you will have to remove the list-style-image from .toolbarbutton-1

> .toolbarbutton-1 {
>   list-style-image: url("chrome://messenger/skin/addressbook/icons/addressbookicons.png");
> }

And apply it to each individual button. See the Thunderbird /skin/addressbook.css for example.

> -#button-delete {
> +#button-delete-item {

   list-style-image: url("chrome://messenger/skin/addressbook/icons/addressbookicons.png");

>    -moz-image-region: rect(0 29px 29px 0);
>  }

Please file bugs in classic and in modern for adding small toolbar icons to the Addressbook. For classic a suitable image resizing tool on the existing large icons would do. Modern would need completely new images.
Comment 5 Ian Neal 2010-07-01 16:58:06 PDT
Created attachment 455595 [details] [diff] [review]
Customizable toolbar patch v0.1a

Changes since v0.1:
* Renamed id "ab-toolbar-menubar2" to "addrbook-toolbar-menubar2"
* Renamed id "abToolbar" to "ab-bar2"
* throbber-box for small icon toolbars now has no background on modern
* Removed the list-style-image from .toolbarbutton-1
* Added list-style-image, where relevant, to the individual buttons

Tried having one button still left in the palette and that was the only button that could be moved, all the rest were locked in place.

Left "ab-menubar" and "searchBox" ids alone so that they are unique in SM.
Comment 6 Philip Chee 2010-07-04 08:59:33 PDT
Comment on attachment 455595 [details] [diff] [review]
Customizable toolbar patch v0.1a

Looks good.
Comment 7 Philip Chee 2010-07-09 10:33:09 PDT
>> I've tried having the default icons outside of the toolbarpalette but you just
>> end up with icons you cannot move from the toolbar.
> 
> Strange. That's what Minefield does. I wonder what we/they are doing
> differently.

A closer reading of Bug 354048 indicates that we need to add |removable="true"| to those items that are already on a toolbar. Items in the palette automatically gain this attribute when moved to a toolbar.
Comment 8 Ian Neal 2010-07-12 16:44:06 PDT
Created attachment 456965 [details] [diff] [review]
Customizable toolbar with removable patch v0.1b [Checkin: Comment 13]

Changes since v0.1a:
* Make use of |removable="true"|
Comment 9 Philip Chee 2010-07-16 08:40:21 PDT
Comment on attachment 456965 [details] [diff] [review]
Customizable toolbar with removable patch v0.1b [Checkin: Comment 13]

Looks good. r=me
Comment 10 Ian Neal 2010-07-22 04:57:48 PDT
(In reply to comment #9)
> Comment on attachment 456965 [details] [diff] [review]
> Customizable toolbar with removable patch v0.1b
> 
> Looks good. r=me

Just as a note I have one additional local change of <popup> to <menupopup>
Comment 11 neil@parkwaycc.co.uk 2010-07-22 08:43:01 PDT
(In reply to comment #4)
> > +  <toolbar class="chromeclass-toolbar toolbar-primary"
> > +           id="abToolbar"
> Thunderbird->"ab-bar2". Sigh.
[Double sigh. Who was responsible for that id?]
Comment 12 neil@parkwaycc.co.uk 2010-07-22 08:53:38 PDT
Comment on attachment 456965 [details] [diff] [review]
Customizable toolbar with removable patch v0.1b [Checkin: Comment 13]

>+toolbar:not([iconsize="small"]) #throbber-box {
We only want the override to apply when the toolbar has a line, which is the specific case where the throbber is in a default primary non-navigator toolbar. So you either need to use .toolbar-primary:not([labelalign="end"]):not(etc.) or you need to override the background to transparent in each of the cases where toolbar.css overrides the line plus either way you also need to override the background to transparent in navigator.css which turns the line off anyway.
Comment 13 Ian Neal 2010-07-25 16:43:22 PDT
Comment on attachment 456965 [details] [diff] [review]
Customizable toolbar with removable patch v0.1b [Checkin: Comment 13]

http://hg.mozilla.org/comm-central/rev/6368a42a985a
Pushed with popup->menupopup and css rule change.

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