Help button in Search Addresses dialog lacks Help icon

RESOLVED FIXED in seamonkey2.1a1

Status

defect
RESOLVED FIXED
10 years ago
4 years ago

People

(Reporter: mnyromyr, Assigned: iann_bugzilla)

Tracking

(Blocks 1 bug)

Trunk
seamonkey2.1a1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 8 obsolete attachments)

The (custom) Help button in the Search Messages dialog lacks the Help icon used by standard dialog Help buttons. Furthermore, the Help button definition should come from helpMessengerOverlay.xul as well, so that ABSearchDialog.xul does not need to know about the OpenHelp function.
Search Addresses dialog, of course.
Summary: Help button in Search Messages dialog lacks Help icon → Help button in Search Addresses dialog lacks Help icon
OS: Linux → All
Hardware: x86 → All
This patch changes the SearchDialog.xul from being a <window> to being a <dialog> with standard help button.
Assignee: nobody → iann_bugzilla
Status: NEW → ASSIGNED
Attachment #400043 - Flags: ui-review?(stefanh)
Attachment #400043 - Flags: superreview?(neil)
Attachment #400043 - Flags: review?(mnyromyr)
Posted image Screenshot-Search Messages (obsolete) —
Comment on attachment 400043 [details] [diff] [review]
Change Search Messages/Addresses to dialog patch v0.1

As per bug 515227. Oh, and if you really want the help button to be in the overlay, then give the container an id and overlay the container to add it.
Attachment #400043 - Flags: superreview?(neil) → superreview-
You should put the help button in the lower right corner of the dialog instead.
Attachment #400043 - Flags: ui-review?(stefanh)
Attachment #400043 - Flags: review?(mnyromyr)
Changes since v0.1:
* searchDialog.css now imports global dialog.css for default theme.
* help button is now moved to where help button is usually found on a dialog
(dlgtype="help" seems to give icon="help" automagically).
* added class="dialog-button" to help button.
Attachment #400043 - Attachment is obsolete: true
Attachment #400267 - Flags: ui-review?(stefanh)
Attachment #400267 - Flags: superreview?(neil)
Attachment #400267 - Flags: review?(mnyromyr)
Attachment #400045 - Attachment is obsolete: true
Attachment #400046 - Attachment is obsolete: true
Is it possible to put the Help button in the right corner?
(In reply to comment #10)
> Is it possible to put the Help button in the right corner?

Yes, but would have to put some #ifdef's in for *nix vs the rest.
(In reply to comment #0)
> the Help button definition should come from helpMessengerOverlay.xul as well,
> so that ABSearchDialog.xul does not need to know about the OpenHelp function.
helpMessengerOverlay.xul was only created to add buttons to shared dialogs...

(In reply to comment #10)
> Is it possible to put the Help button in the right corner?
It would certainly make more sense. It looks terrible in the left corner.
Comment on attachment 400267 [details] [diff] [review]
Change Search Messages/Addresses to dialog without overlay patch v0.1a

>-<window id="searchAddressBookWindow"
>+<dialog id="searchAddressBookWindow"
I still don't see the point of the window/dialog conversion.
Comment on attachment 400267 [details] [diff] [review]
Change Search Messages/Addresses to dialog without overlay patch v0.1a

What's the reason for The Great Accesskey Renaming?

Basically, the lone Help button down left is rather pointless. Just move one line up and to the outer right there.

Furthermore, the top address book dropdown should flex horizontally.
Attachment #400267 - Flags: review?(mnyromyr) → review-
Attachment #400267 - Flags: ui-review?(stefanh)
Attachment #400267 - Flags: superreview?(neil)
Changes since v0.1b:
* Moved to right and inline with rest of the buttons in both dialogs.

Help now takes H accesskey (as is the default on most other dialogs/windows) hence the accesskey shuffle.
Attachment #400267 - Attachment is obsolete: true
Attachment #401966 - Flags: review?(mnyromyr)
Attachment #400270 - Attachment is obsolete: true
Attachment #400271 - Attachment is obsolete: true
Comment on attachment 401966 [details] [diff] [review]
Change Search Messages/Addresses to dialog help on right patch v0.1b

Basically okay, just some nits:

>+++ b/suite/locales/en-US/chrome/mailnews/SearchDialog.dtd
> <!ENTITY openButton.label            "Open">
> <!ENTITY openButton.accesskey        "n">

I think this Open should get the O...

>+<dialog id="searchAddressBookWindow"
>         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
>         xmlns:nc="http://home.netscape.com/NC-rdf#"
>         onload="searchOnLoad();"
>         onunload="onSearchStop(); searchOnUnload();"
>+        buttons="help"
>+        ondialoghelp="return openHelp('mail_advanced_ab_search');"

Command handler attributes should go last.

>           <label value="&abSearchHeading.label;" accesskey="&abSearchHeading.accesskey;" control="abPopup"/>
>           <menulist id="abPopup" oncommand="SelectDirectory(this.value);">
>             <menupopup id="abPopup-menupopup" class="addrbooksPopup"/>
>           </menulist>
>           <spacer flex="10"/>
>           <button id="search-button" oncommand="onSearchButton(event);" default="true"/>

Linux Classic at least has a problem with styling menulists. Although there should be enough space here, the menuitem "Personal Address Book" gets truncated to "Personal Addre..." in the menulist and to "Personal Address..." in the popup. How'd you feel about making the top menulist in all three dialogs (Message Search, AB Search, Filters) flex?

>+        <button dlgtype="help" class="dialog-button"/>

As with the filters dialog, the Help button draws an unwanted "margin-top: 6px" rule here. :-/

>+<dialog id="searchMailWindow"
>         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
>         xmlns:nc="http://home.netscape.com/NC-rdf#"
>         onload="searchOnLoad();"
>         onunload="onSearchStop(); searchOnUnload();"
>+        buttons="help"
>+        ondialoghelp="return openHelp('search_messages');"

Command handler attributes should go last.
Attachment #401966 - Flags: review?(mnyromyr) → review-
Changes since v0.1b:
* Removed 6px margin from the top of help button in classic.
* Changed order of attributes on dialog box.

As discussed changes to size of menuitem will happen in bug 515967.
Attachment #401966 - Attachment is obsolete: true
Attachment #403035 - Flags: review?(mnyromyr)
Attachment #403035 - Flags: review?(mnyromyr) → review+
Attachment #403035 - Flags: superreview?(neil)
Testing this patch made me notice that there's a bug in searchDialog.css (both themes) whereby for some reason the padding on menuitems is overridden. This unfortunately isn't the whole story for address book search, because it suffers from the additional indignity that the width of menulists is overridden, when in fact only the message folders menulist should have its width set.

When I added the "Search local system" checkbox to SearchDialog.xul it was convenient that I could add it to what was a blank cell in the "grid". I'm not sure that I would have put it there had the Help button been at the bottom.

Now that these are dialogs, should we remove the Ctrl+W key to close them?
(In reply to comment #20)
> Testing this patch made me notice that there's a bug in searchDialog.css (both
> themes) whereby for some reason the padding on menuitems is overridden. This
> unfortunately isn't the whole story for address book search, because it suffers
> from the additional indignity that the width of menulists is overridden, when
> in fact only the message folders menulist should have its width set.
See my comment#19
> 
> When I added the "Search local system" checkbox to SearchDialog.xul it was
> convenient that I could add it to what was a blank cell in the "grid". I'm not
> sure that I would have put it there had the Help button been at the bottom.
So should it go in parallel to the other checkbox?
> 
> Now that these are dialogs, should we remove the Ctrl+W key to close them?
Yes, probably.
(In reply to comment #21)
> See my comment#19
Sorry, I didn't realise the significance.

> So should it go in parallel to the other checkbox?
Yes, probably ;-)
Attachment #403035 - Flags: superreview?(neil)
Changes since v0.1c:
* Move checkboxes in Search Messages to be in parallel.
* Remove Ctrl-W from new dialog boxes.
Attachment #403035 - Attachment is obsolete: true
Attachment #405366 - Flags: superreview?(neil)
Attachment #405366 - Flags: review+
Regarding the removal of Accel+W: Is this just because it's a dialog? If that's the case, is there anyone that will think it's a dialog? I mean, I have never seen a dialog with a status bar...
Attachment #405366 - Flags: superreview?(neil) → superreview+
Comment on attachment 405366 [details] [diff] [review]
Remove Close Window key patch v0.1d [Checkin: comment 27]

I'll ask for a= as even there are locale changes they are just accesskeys and the one string that is removed, even if localizers don't remove it it should make no difference.
Attachment #405366 - Flags: approval-seamonkey2.0?
Comment on attachment 405366 [details] [diff] [review]
Remove Close Window key patch v0.1d [Checkin: comment 27]

I don't like the locale changes for 2.0 final, esp. not after RC1 has been cut. The accesskeys would possibly be OK, but the removed strings mean all dashboard entries turning orange, which is not something I'd like to have in final release preparations.
We can probably look into taking that for 2.0.1 with the necessary thunder in the L10n group/list, but please not before 2.0 proper.
Attachment #405366 - Flags: approval-seamonkey2.0? → approval-seamonkey2.0-
Comment on attachment 405366 [details] [diff] [review]
Remove Close Window key patch v0.1d [Checkin: comment 27]

http://hg.mozilla.org/comm-central/rev/cefd439cd3c2
Attachment #405366 - Attachment description: Remove Close Window key patch v0.1d → Remove Close Window key patch v0.1d [Checkin: comment 27]
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1a1
The switch to a dialog made here caused bug 543160.
Depends on: 543160
(In reply to comment #24)
> Regarding the removal of Accel+W: Is this just because it's a dialog? If
> that's the case, is there anyone that will think it's a dialog? I mean, I
> have never seen a dialog with a status bar...

I don't really get the logic behind that either; the whole reasoning that was put into words here comes down to "because it's a dialog [<dialog>], Ctrl+W should not close it", but I don't understand where that's coming from. Please enlighten us and leave your comments over at bug 672155.
Are we talking about the lifebelt symbol in Help button left from "Help" text?
I still do not see that  with EN-US Seamonkey 2.33.1 (German Language pack)  Gecko/20100101 Build 20150321194901 (Default Theme) on German WIN7 64bit.
Flags: needinfo?(iann_bugzilla)
Hm, I never saw that lifebelt in any help button.
I remember it in the past, but I think it depends on the OS (and OS theme).
Flags: needinfo?(iann_bugzilla)
Yeah, it's an OS icon I think, and as such depends on the desktop theme.
See attachment 8602370 [details] for a prefpane example on Linux with KDE4.
You need to log in before you can comment on or make changes to this bug.