Closed Bug 883482 Opened 6 years ago Closed 6 years ago

Change the checkbox to search only messages saved locally to a more descriptive menulist

Categories

(SeaMonkey :: Search, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.22

People

(Reporter: xfox.mozilla, Assigned: rsx11m.pub)

Details

Attachments

(4 files, 4 obsolete files)

In the Advanced Search window (from the Mail & Newsgroups module, Tools -> Search Messages…) there is a checkbox labelled "Search local system".
First time I saw that checkbox I had to check the SeaMonkey integrated help to understand what exactly it was supposed to do, that is "search only messages from newsgroups or IMAP accounts that have been saved locally".
To improve the user interface intelligibility I propose to change the label of that checkbox to something like "Search local system only" or "Search only local system" (I'm not sure which one is the most correct form).
This makes sense, and there is sufficient space in that dialog window to extend the label a bit. I'm not entirely sure about the grammatically correct form either but would add an "on" as in "Search on the local system only" to make it a "real" sentence. I'll come up with a patch.
Assignee: nobody → rsx11m.pub
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Summary: Change the label of the checkbox to search only messages saved locally to "Search local system only" → Change the label of the checkbox to search only messages saved locally to "Search on the local system only"
Attached patch Proposed patch (obsolete) — Splinter Review
An alternative label would have been "Perform search on the local system only" but that seems to be pushing it a bit too much.

I have expanded the help content a bit to also explain what happens when that box is *not* checked and made it clear that local searches of the message body depend on synchronization being enabled.

Drive-by fix: White-space adjustments for saveAsVFButton.* in SearchDialog.dtd.
Attachment #763112 - Flags: ui-review?(neil)
Attachment #763112 - Flags: review?(iann_bugzilla)
(In reply to Andrea Govoni from comment #0)
> In the Advanced Search window (from the Mail & Newsgroups module, Tools ->
> Search Messages…) there is a checkbox labelled "Search local system".
> First time I saw that checkbox I had to check the SeaMonkey integrated help
> to understand what exactly it was supposed to do, that is "search only
> messages from newsgroups or IMAP accounts that have been saved locally".

That's not exactly true; you don't have to save messages to be able to search locally for common headers (which is why I didn't call it Search Offline).

> To improve the user interface intelligibility I propose to change the label
> of that checkbox to something like "Search local system only" or "Search
> only local system" (I'm not sure which one is the most correct form).

Adding the word "only" is also potentially misleading as it suggests that the search normally occurs both locally and remotely (rather than either/or).
As it appears the search indeed is either strictly on the server only (when the box is not checked) or strictly locally only (no communication when it's checked), which is what I was trying to emphasize in the help updates.

Any other suggestion for the label to make it clearer? Getting back to my other idea, "Perform search on the local system" should be unambiguous and doesn't have any only/also context problem.
(In reply to neil@parkwaycc.co.uk from comment #3)
> > First time I saw that checkbox I had to check the SeaMonkey integrated help
> > to understand what exactly it was supposed to do, that is "search only
> > messages from newsgroups or IMAP accounts that have been saved locally".
> That's not exactly true; you don't have to save messages to be able to search
> locally for common headers (which is why I didn't call it Search Offline).

That's what the current help text says, but while we are here I can certainly modify this as well. In addition, you could have synchronized the last 30 days only, thus your ability for message-body search is limited to that duration. Headers indeed shouldn't be affected as they are stored locally anyway.

Thus, "save" should be "synchronized" for sure, but I'm uncertain how much more information should be provided without overloading this section with details.
(In reply to rsx11m from comment #4)
> As it appears the search indeed is either strictly on the server only (when
> the box is not checked) or strictly locally only (no communication when it's
> checked), which is what I was trying to emphasize in the help updates.

I agree that the help needs an update.

> Any other suggestion for the label to make it clearer? Getting back to my
> other idea, "Perform search on the local system" should be unambiguous and
> doesn't have any only/also context problem.

Yes, that sounds reasonable. It might even let you find a better access key!
Attached patch Proposed patch (v2) (obsolete) — Splinter Review
Changes made to the label as suggested. For the Help content, I've moved the sentence on message-body search and synchronization up into the main item #4 description and made clear that the major headers are always available for a local search. I've left the "only" in that context but offset it by an "all" later when describing remote search with the box not being checked.

Also added POP accounts to the note as an example when the box is disabled.

As for the title of the bug report, I'll adjust it once we have settled on the final label for this checkbox. ;-)
Attachment #763112 - Attachment is obsolete: true
Attachment #763112 - Flags: ui-review?(neil)
Attachment #763112 - Flags: review?(iann_bugzilla)
Attachment #763250 - Flags: ui-review?(neil)
Attachment #763250 - Flags: review?(iann_bugzilla)
Comment on attachment 763250 [details] [diff] [review]
Proposed patch (v2)

Yes, I like this version.
Attachment #763250 - Flags: ui-review?(neil) → ui-review+
(In reply to neil@parkwaycc.co.uk from comment #3)
> Adding the word "only" is also potentially misleading as it suggests that
> the search normally occurs both locally and remotely (rather than either/or).

I'm fine with the "Perform search on the local system" label (should the bug title be updated accordingly?) and the expanded help section but, from an UI design POV, it now seems wrong to me to use a checkbox to convey the actual advanced search behavior.
Yes, the updated help will explain it very clearly anyway, but IMHO the right UI element to communicate to the user that the search occurs either locally or remotely is a radio button, maybe approximately with the following design:

                       O local system
Perform search on the
                       O server

Please tell what do you think about this, if it's good for you I'll go ahead and file a new bug.
Attached image Alternative design
(In reply to Andrea Govoni from comment #9)
> (should the bug title be updated accordingly?)

As I said in comment #7, I'd update it once the final design and text is clear.

> right UI element to communicate to the user that the search occurs either
> locally or remotely is a radio button, maybe approximately with the
> following design:
> 
>                        O local system
> Perform search on the
>                        O server

I wouldn't like to have another row occupied, and mixing horizontal ("all"/"any") and vertical radiogroups may not be the best layout. Also, forming a natural sentence with multiple branches is frequently a problem for internationalization.

Thus, as a feasible alternative to the checkbox, I'd suggest to replace it with a horizontal radiogroup and full (but shortened due to limited space) labels for each option. This is a XUL mockup on Windows 7 (Mac OSX needs more width, thus I'm reluctant to add redundant "the" and "Perform" to both labels).

Neil, what do you think?
Attachment #763509 - Flags: feedback?(neil)
Comment on attachment 763509 [details]
Alternative design

Sure, that would work. (Would a menulist be too cumbersome?)
Attachment #763509 - Flags: feedback?(neil) → feedback+
Ok, I'll try the menulist (which would allow us to retain the "Perform" labels but would loose the accesskeys) and keep the radiogroup as the fallback.
(correction: the accesskey for the label should focus the menulist.)
(In reply to rsx11m from comment #10)
> As I said in comment #7, I'd update it once the final design and text is
> clear.

Sorry, I missed that line.

> Thus, as a feasible alternative to the checkbox, I'd suggest to replace it
> with a horizontal radiogroup and full (but shortened due to limited space)
> labels for each option. This is a XUL mockup on Windows 7 (Mac OSX needs
> more width, thus I'm reluctant to add redundant "the" and "Perform" to both
> labels).

Why not move the "Search subfolders" checkbox beside the drop-down list to select the folder to search?
The current layout has somewhat of a hierarchy:

 - 1st row: Select account or folder to search
   - 2nd row: Options on the search location
     - 3rd row: Options on what to search for

Intuitively "Search subfolders" fits where it is right now, but it would also fit into the 1st row narrowing down the folders to search. Thus, either should work.
Attached image Menulist options
(In reply to Andrea Govoni from comment #14)
> Why not move the "Search subfolders" checkbox beside the drop-down list to
> select the folder to search?

I agree, it looks better with the menulists being at the beginning of the window than the combination checkbox/menulist in one row.
Attached patch Proposed patch (v3) (obsolete) — Splinter Review
Implementation as menulist and the subfolders checkbox moved into 1st row. This also allowed the introductory label to be a bit more specific, I've made it now "search operation" rather than just "search" to be unambiguous what's happening.

Includes the necessary updates to Help content.
Attachment #763250 - Attachment is obsolete: true
Attachment #763250 - Flags: review?(iann_bugzilla)
Attachment #763837 - Flags: ui-review?(neil)
Attachment #763837 - Flags: review?(iann_bugzilla)
Attached image Screenshot (v3)
Note that I've changed the flex attribute for the spacer, thus giving the account/folder menulist a bit more space. It got frequently truncated.
Summary: Change the label of the checkbox to search only messages saved locally to "Search on the local system only" → Change the checkbox to search only messages saved locally to a more descriptive menulist
Version: SeaMonkey 2.18 Branch → Trunk
(In reply to rsx11m from comment #18)
> Note that I've changed the flex attribute for the spacer, thus giving the
> account/folder menulist a bit more space. It got frequently truncated.

I was worried about that; the menulist is variable width, but we want to limit it to available space (so it can't be inflexible) but we don't want it unnecessarily consume space (thus the spacer).
The spacer is still there, it's just smaller now. Do you have any concerns that the layout may break with that change?
Comment on attachment 763837 [details] [diff] [review]
Proposed patch (v3)

>+++ b/suite/locales/en-US/chrome/mailnews/SearchDialog.dtd

>+<!ENTITY searchOnHeading.label       "Perform search operations on:">
>+<!ENTITY searchOnHeading.accesskey   "P">
>+<!ENTITY searchOnRemote.label        "the remote server">
>+<!ENTITY searchOnLocal.label         "the local system">
I rather have "the" in the searchOnHeading.label entity, so "Perform search operations on the:" and the help pages reflect that.

r=me with that addressed/rebutted.
Attachment #763837 - Flags: review?(iann_bugzilla) → review+
Attached patch Proposed patch (v4) (obsolete) — Splinter Review
(In reply to Ian Neal from comment #21)
> I rather have "the" in the searchOnHeading.label entity, so "Perform search
> operations on the:" and the help pages reflect that.

I figured that "the remote server" and "the local system" are grammatical units, thus separating the article to the introductory label with the colon inbetween didn't look right (and feels especially odd when reading the help text.

Thus, as a compromise, I've got rid of the articles in the menuitem labels and capitalized the first word. This corresponds to what we have elsewhere.

(In reply to neil@parkwaycc.co.uk from comment #19)
> I was worried about that; the menulist is variable width, but we want to
> limit it to available space (so it can't be inflexible) but we don't want it
> unnecessarily consume space (thus the spacer).

I've changed the spacer to get a bit wider again, now with a flex="3" compared to a flex="10" before. This looks ok when increasing the width of the dialog window.

If you check on Linux, width sizes may be larger there than on Windows and Mac, given what we've seen in the pref panes (but Windows needs the most height).
Attachment #763837 - Attachment is obsolete: true
Attachment #763837 - Flags: ui-review?(neil)
Attachment #763968 - Flags: ui-review?(neil)
Attachment #763968 - Flags: review+
Comment on attachment 763968 [details] [diff] [review]
Proposed patch (v4)

There is one minor nit I have with this which I have only just noticed.

Because their contents are variable, all of the existing menulists are given a default width of 12em somewhere (sorry I forgot to remember where) so that they don't resize when their contents changes.

However this also affects your new menulist, which means that should some locale decide that they need a string longer than 12em then it won't fit. (The problem is likely to be worse on the Mac because the UI font is bold by default.)
Attachment #763968 - Flags: ui-review?(neil) → ui-review+
Those are defined in searchDialog.css as global menulist { } rules. Thus, I could define a separate class for this specific menulist which has a "width: auto;" override.

Mac seems to set it to 18em rather than 12em.
(In reply to rsx11m from comment #24)
> Those are defined in searchDialog.css as global menulist { } rules. Thus, I
> could define a separate class for this specific menulist which has a "width:
> auto;" override.

Or using the id would work. (Are we expecting to have a second menulist?)
Ok, I went with the identifier which also has the benefit that the XUL code won't need any changes. In addition to the width, I also had to exclude the rule for -moz-padding-end from the menuitem rules for #menuSearchLocalSystem. It was ok in classic without that change, but the label was cut off in modern.

Rather than adding more rules, I've just added :not(#menuSearchLocalSystem) modifiers to the existing rules. Tested with classic and modern but not mac, given that I don't have that platform around.
Attachment #763968 - Attachment is obsolete: true
Attachment #771689 - Flags: ui-review?(neil)
Attachment #771689 - Flags: review+
Comment on attachment 771689 [details] [diff] [review]
Proposed patch (v5)

Seems to work well, thanks.
Attachment #771689 - Flags: ui-review?(neil) → ui-review+
Thanks Neil. I don't know what the current status of comm-central is, tbpl still looks broken and I see other checkin-needed patches have been pending for a week.

Callek, can this land on trunk, or what's the current procedure?
Flags: needinfo?(bugspam.Callek)
I see regular patches landing for SeaMonkey again, thus it seems to be safe to request checkin for comm-central at this time.
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/5528186da5a0
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.22
Flags: needinfo?(bugspam.Callek)
You need to log in before you can comment on or make changes to this bug.