Add toolbar to global search results tab, with gloda search box to refine existing faceted search or start a new search

RESOLVED FIXED in Thunderbird 13.0

Status

Thunderbird
Search
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: andreasn, Assigned: mconley)

Tracking

(Blocks: 3 bugs, {regression})

Trunk
Thunderbird 13.0
regression
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird11+ fixed, thunderbird12 fixed)

Details

Attachments

(12 attachments, 12 obsolete attachments)

231.56 KB, image/png
bwinton
: ui-review+
Details
121.65 KB, image/png
Details
102.78 KB, image/png
Details
169.80 KB, image/png
Details
83.87 KB, image/png
Details
35.06 KB, image/png
Details
15.35 KB, patch
andreasn
: review+
andreasn
: ui-review+
Details | Diff | Splinter Review
7.40 KB, image/png
Details
20.63 KB, patch
Details | Diff | Splinter Review
26.30 KB, image/png
Details
26.44 KB, image/png
Details
439 bytes, patch
mconley
: review+
Details | Diff | Splinter Review
(Reporter)

Description

6 years ago
Spinning off #717261
Right now if you fail to find any results for what you're searching for, you just get a error message and needs to go back to the 3-pane tab and do a new search (that will open in a new tab, bug #716058). This becomes a lot of jumping back and forth. It would be great if it was possible to search from the search tab itself.
(Assignee)

Updated

6 years ago
Assignee: nobody → mconley
OS: Linux → All
(Assignee)

Comment 1

6 years ago
Created attachment 589897 [details] [diff] [review]
WIP Patch v1

Here's my first run at this patch so far.  Here are the major changes:

1.  Instead of an iframe as the search tab panel, we open a vbox, and use the same technique that chromeTab/contentTab uses for cloning a dummy node appended to the tabmail element (defined in the specialTabs.xul overlay).
2.  A class .remote-gloda-search is assigned the glodaSearch binding.
3.  The tab monitor defined in searchBar.js determines whether or not we've opened up a glodaFacet tab.  If so, it finds and populates the .remote-gloda-search element with the search query.
4.  The keyboard shortcut for searching (Ctrl-k) is mapped to the .remote-gloda-search element if a glodaFacet tab is currently selected.

asuth:  am I walking into any traps here?  Any suggestions or comments?

Thanks,

-Mike
Attachment #589897 - Flags: feedback?(bugmail)
Comment on attachment 589897 [details] [diff] [review]
WIP Patch v1

Seems like it should work; for some reason I thought the gloda autocomplete stuff was more than a hack than it is.  I must have been thinking about the original exptoolbar gloda prototype...

I would suggest getting rid of GlodaSearchBoxTabMonitor in searchBar.js.  It only existed because the gloda search box lived outside the 3-pane.  Since it apparently now lives inside the 3-pane again, it can be managed entirely in the 3-pane tab logic.  And then the gloda special case you are inserting can just live in the gloda tab too.

I think that will make more sense, especially because right now its onTabSwitched is going to be persisting incorrect information for gloda tabs.  (And the persistence is not needed because the UI widget already carries all the state we need, the goal having been to not lose the user's edits to the search box when they change tabs.)
Attachment #589897 - Flags: feedback?(bugmail) → feedback+
Blocks: 717261, 644169
Hardware: x86_64 → All
Summary: make it possible to make a new search from the search tab → Add search box to refine existing search or make a new search from global search results tab
Summary: Add search box to refine existing search or make a new search from global search results tab → Add gloda search box to refine existing search or make a new search from global search results tab
Blocks: 725507
Adjusting summary to be more clear about the good things we are doing here, and make this more retrievable
Summary: Add gloda search box to refine existing search or make a new search from global search results tab → Add toolbar to global search results tab, with gloda search box to refine existing faceted search or start a new search
Blocks: 716058
tracking-thunderbird11: --- → ?
(In reply to Thomas D. from bug 717261, comment #15)
Ftr, imo, the benefits of having a toolbar for global search results (this bug)
can very well co-exist with bug 717261.

> The resulting UI for
> the faceted search results tab would be very similar to the FF UI on a
> Google results page:
> - have an ever-present global search box in the upper right corner of the
> screen (both in FF and TB, bug 717261)
> - allow in-place refining of search terms in a central search box above
> search results (both in FF-google-results, and in TB, this bug 719008), which 
> at the same time acts as a headline for search results.
See Also: → bug 717261
Mike, could you attach a screenshot with current wip patch applied?
This bug fixes the regression that currently, as a result of tabs on top (bug 644169), we can no longer refine existing global search results.

The minimal fix for this regression would be something like bug 717261, or, ideally, do both bug 717261 and this one, as explained in comment 4.
Keywords: regression
(Assignee)

Comment 7

6 years ago
I've been pulled off of this to work on BigFiles.

This is an *excellent* bug for a new contributor, and I'm willing to mentor.
Assignee: mconley → nobody
(Assignee)

Updated

6 years ago
Assignee: nobody → mconley
(Assignee)

Comment 8

6 years ago
(In reply to Andrew Sutherland (:asuth) from comment #2)
> Comment on attachment 589897 [details] [diff] [review]
> WIP Patch v1
> 
> Seems like it should work; for some reason I thought the gloda autocomplete
> stuff was more than a hack than it is.  I must have been thinking about the
> original exptoolbar gloda prototype...
> 
> I would suggest getting rid of GlodaSearchBoxTabMonitor in searchBar.js.  It
> only existed because the gloda search box lived outside the 3-pane.  Since
> it apparently now lives inside the 3-pane again, it can be managed entirely
> in the 3-pane tab logic.  And then the gloda special case you are inserting
> can just live in the gloda tab too.
> 
> I think that will make more sense, especially because right now its
> onTabSwitched is going to be persisting incorrect information for gloda
> tabs.  (And the persistence is not needed because the UI widget already
> carries all the state we need, the goal having been to not lose the user's
> edits to the search box when they change tabs.)

Hm - we may want to stick with the GlodaSearchBoxTabMonitor actually... the search input *can* exist within the 3-pane tab, but it can also exist outside of it (in the tabbar-toolbar for example).
(Assignee)

Comment 9

6 years ago
Created attachment 598344 [details] [diff] [review]
Patch 1

Un-bitrotted.
Attachment #589897 - Attachment is obsolete: true
(Assignee)

Comment 10

6 years ago
Created attachment 598351 [details]
Wireframe for toolbar layout

So my latest patch simply inserts a thin toolbar and a text input that kicks off Gloda searches in the same tab.

It works, but visually, it's a little strange to just have the text input up there.  I've attached what I'm proposing.

Or is there a better solution?
Attachment #598351 - Flags: ui-review?(bwinton)
(Assignee)

Comment 11

6 years ago
Created attachment 598352 [details]
Wireframe for toolbar layout

Just fixing a small detail - but otherwise the same.
Attachment #598351 - Attachment is obsolete: true
Attachment #598351 - Flags: ui-review?(bwinton)
Attachment #598352 - Flags: ui-review?(bwinton)
(In reply to Mike Conley (:mconley) from comment #10)
> Created attachment 598351 [details]
> Wireframe for toolbar layout
> So my latest patch simply inserts a thin toolbar and a text input that kicks
> off Gloda searches in the same tab.
> It works, but visually, it's a little strange to just have the text input up
> there.  I've attached what I'm proposing.
> Or is there a better solution?

Some ideas:

(In reply to Thomas D. from comment #14)
> (In reply to Bryan Clark [:clarkbw] from comment #12)
> > (In reply to Jim Porter (:squib) from comment #11)
> > > Yeah, I've suggested before that the gloda tab have a toolbar with a gloda
> > > search box and some useful buttons (e.g. "View as List"). Doing that would
> > > also probably let us get rid of some of the content in the tab (e.g. the
> > > "Search <query>" heading), which would let us clean the UI up a bit, I think.
> > 
> > This sounds like a pretty good idea to me
> 
> +1!!!
> 
> With tabs on top, adding a customizable toolbar to the global search results
> tab, as suggested by Jim, could really help to streamline & improve the UI.
> To begin with, some interesting elements for that toolbar:
> 
> * Gloda global search box, to refine existing search, or start new search: 
> bug 719008, bug 596212, bug 716058, and new bug needed for Ctrl+global
> search from search results to open in a new tab
> * "View as list": something like bug 518336
> * Dateline toggle: to this day, this poor creature does not even have a
> tooltip to tell what it does (bug 516797), so making that a proper toggle
> button on the toolbar for showing/hiding the dateline would certainly help...
(In reply to Thomas D. from comment #12)
> (In reply to Mike Conley (:mconley) from comment #10)
> Some ideas:
> 
> (In reply to Thomas D. from comment #14)

The ideas quote in comment 12 was taken from Bug 717261 Comment 14.

Comment 14

6 years ago
Created attachment 598581 [details]
Mockup

Here's what I was thinking for UI. This gives us a fair bit more space for results (the important bit), since things are moved into the toolbar, and the "Search <query>" heading is gone. With the query in the tab title and the text box, we really don't need it in the page, too.
(In reply to Jim Porter (:squib) from comment #14)
> Created attachment 598581 [details]
> the "Search <query>" heading is gone. With the query in the tab title and
> the text box, we really don't need it in the page, too.

That looks great to me, definitely a step into the right direction.
However, due to the unfinished nature of Global search, removing the query caption from the search results page will cause some new problems:

STR

1) Global Search, type in "Blake", choose "Blake Winton (bwinton(at)mozilla.com)" *from Autocomplete*
2) Global Search, type in "bwinton(at)mozilla.com", press Enter

Actual Results

after 1) Global search results tab title is "Search", and results page headline is "Search involving bwinton(at)mozilla.com"; 36 msgs found

after 2) Global search results tab title is "bwinton(at)mozilla.com", and results page headline is "Search bwinton(at)mozilla.com"; 213 msgs found

Now this is where we run into trouble if we just remove the page headline, and it's not easy to get this right. Some caveats:

a) For cases of 1) (involves...), we cannot just leave "bwinton(at)mozilla.com" in the search box because we didn't do a fulltext search for that string, we don't have a syntax for indicating "involves" in the search box, and we definitely need to distinguish between the two, because they yield completely different result sets.
So lest we find a syntax which actually works, we cannot leave anything in the search box, so it must be blank as is current behaviour.
Maybe in the tab title, we could have "Involving bwinton(at)mozilla.com", but even for small numbers of tabs, that tab title will be truncated due to its length (and it might be worse in other locales).
So practically, there would be no fully visible caption to inform the user about the current query -> bad!

b) for cases of 2)(fulltext), we do the correct thing which is just putting "bwinton(at)mozilla.com" into both tab title and search box. No problem here.

More considerations about removing the page header with information on the current query:

c) Currently, global search for "tourist attraction" (without quotes) will do fulltext search for messages containing tourist AND attraction (and as a matter of fact, at least for EN, we also find tourist AND attracted). So it's enough for the mail to have variants of both words anywhere in the source, and the actual string "tourist attraction" is not required. The result for that is different from searching "tourist attraction" (with quotes), where we correctly take it as one string, so there are less results (and it's even correctly documented in (1) on SuMoMo). Furthermore, you can combine the two and search for [new "tourist attraction"] (without brackets), and we'll create a correct results headline from that: Search *new* AND *tourist attraction*. In the tab title, we just show what the user entered, including the quotes.

If we remove the headline, we'll remove any explicit explanation as to how we parsed the search terms. But maybe in this case, it's minor problem because users know from Google etc. how this works, and at least it can be reconstructed from the exact search terms with quotes in search box and tab title. Case of a) above is much worse.

Conclusions:

Because of a), I think we cannot remove search results headline, but we could make it smaller, more like smallprint information or a tooltip. Even if we keep the current format and size of headline, we can still save some vertical space with Jim's toolbar if we cut down the way too generous vertical offsets, including the offset on the right which is about 1cm too wide, just wasting my screen real estate. Furthermore, we'll still have the benefit that the toolbar removes a lot of visual clutter from the search results page, and it's a first step towards converting the completely alien design of global search back into something that blends in better with the rest of Thunderbird's UI.
(In reply to Thomas D. from comment #15)
> (and it's even correctly documented in (1) on SuMoMo). Furthermore, you can

Global search documentation:

(1) https://support.mozillamessaging.com/en-US/kb/global-search
The gloda search UI precursor 'exptoolbar' promoted semantic objects into colored bubbles, which was a growing UI trend at the time and even moreso now: http://www.visophyte.org/blog/?p=112 and http://www.visophyte.org/blog/?p=117

A bubble display that also transformed fulltext terms and showed a more verbose-when-inactive display might be a workable option.  For example 'new "tourist attraction"' could be displayed as '("new") and ("tourist attraction")' in the bar when not focused.  If you click in the bar, the bubbles for the text would disappear and so would the and/or hints.

Although doing the bubble UI would itself be a medium-effort undertaking in itself, it could easily increase tension between "altering the search to find what I want" and "filtering the results to find what I want", especially if it caused the altering the search option to become more expressive/powerful.  This would be undesirable because the performance characteristics are such that it is likely far preferable to filter an existing search than issue a new search.  The exception is that an initial search that was far too generic or otherwise hit our LIMIT on results would probably want to re-issue the search.  (It is, of course, possible to cause the search box to trigger filtering where possible, potentially even refiltering as the user types, but that would be a large-effort undertaking.)
Comment on attachment 598352 [details]
Wireframe for toolbar layout

I mostly like this, and squib's mockup below.

The one thing I would change for consistency with other toolbars is to put the gloda search box on the right instead of the left…

ui-r=me with that, though.

Thanks,
Blake.
Attachment #598352 - Flags: ui-review?(bwinton) → ui-review+
(Assignee)

Comment 19

6 years ago
Jim:

That's an awesome looking mock-up!

However, I'm hesitant to do such a sweeping change when this alteration is supposed to be *released* in only 3 weeks.  Ideally, I'd like to change as little as possible to avoid introducing more bugs so late into the cycle.

So, if it's alright with you, I think we should scale back the patch for this such that we simply put a search input into the toolbar.  The rest can come later.

Does this sound reasonable?

-Mike
(Assignee)

Comment 20

6 years ago
Created attachment 599169 [details] [diff] [review]
WIP Patch v2

This patch puts us in a "functional" state - now we need to make this look pretty.
Attachment #598344 - Attachment is obsolete: true
tracking-thunderbird11: ? → +

Comment 21

6 years ago
Well, my mockup has string changes, so it couldn't get in before 13.0 anyway (unless we wanted to annoy localizers). I'm ok with something basic for now.
(Reporter)

Comment 22

6 years ago
Created attachment 599288 [details] [diff] [review]
Aero part of prettify

Ok, here is the first part of the prettyfication. Needs some testing first to make sure it will work with Personas and all themes. OSX and Linux coming up!
(Reporter)

Comment 23

6 years ago
Created attachment 599304 [details] [diff] [review]
Windows + Mac styling

Like previous patch, + the pinstripe parts.
Attachment #599288 - Attachment is obsolete: true
Comment on attachment 599169 [details] [diff] [review]
WIP Patch v2

Okay, the code seems fine to me, so next time I'll just review Andreas's additions.  If you wanted to get asuth to take a look at it too, that might be a good idea…
Attachment #599169 - Flags: review+
(Reporter)

Comment 25

6 years ago
Created attachment 599310 [details] [diff] [review]
windows, mac and linux patch

And this part takes care of Linux too.
Attachment #599304 - Attachment is obsolete: true
(Reporter)

Comment 26

6 years ago
Created attachment 599320 [details] [diff] [review]
windows, mac and linux patch (v2)

Windows, Mac and Linux style patch (to be applied on top of Mike's patch)
Fixed slight bitrot.
(Reporter)

Updated

6 years ago
Attachment #599310 - Attachment is obsolete: true
(Assignee)

Comment 27

6 years ago
Created attachment 599332 [details] [diff] [review]
Patch v1

Aggregating my patch and Andreas' patch into a single patch.  I've also made the search input in the search tab be 1/3rd of the width of the toolbar, with a minimum width of 10em.
Attachment #599169 - Attachment is obsolete: true
Attachment #599320 - Attachment is obsolete: true
Attachment #599332 - Flags: review?(bugmail)
(Assignee)

Comment 28

6 years ago
Created attachment 599337 [details]
Ubuntu - Gloda search tab with toolbar
(Assignee)

Comment 29

6 years ago
Created attachment 599338 [details]
Windows Aero - Gloda search tab with toolbar
(Assignee)

Comment 30

6 years ago
Created attachment 599340 [details]
Windows Classic - Gloda search tab with toolbar
Comment on attachment 599332 [details] [diff] [review]
Patch v1

Sure fine fine.
Attachment #599332 - Flags: review?(bugmail) → review+
(Assignee)

Comment 32

6 years ago
Created attachment 599356 [details] [diff] [review]
Patch v2 (carrying over r+ from asuth)

This patch adds some padding to the height of the .remote-gloda-search container to make the toolbar equal in height to mail-bar3.

gnomestripe and qute-aero are taken care of.  Andreas will be taking care of pinstripe and qute-classic.
Attachment #599332 - Attachment is obsolete: true
(Reporter)

Comment 33

6 years ago
Created attachment 599381 [details] [diff] [review]
Patch v2 + pinstripe

Same as previous patch, but correct height for pinstripe search toolbar.
Attachment #599381 - Flags: review+
(Reporter)

Comment 34

6 years ago
Created attachment 599388 [details]
above patch in action
(Reporter)

Comment 35

6 years ago
Created attachment 599425 [details] [diff] [review]
Amalgamated patch (r+'d by asuth, ui-r+'d by bwinton)

Patch v2 with all themes issues fixed.
Attachment #599381 - Attachment is obsolete: true
Attachment #599425 - Flags: ui-review+
Attachment #599425 - Flags: review+
(Reporter)

Comment 36

6 years ago
Created attachment 599427 [details]
screenshot of patch running under xp mode
(Assignee)

Comment 37

6 years ago
Comment on attachment 599356 [details] [diff] [review]
Patch v2 (carrying over r+ from asuth)

Marking this as obsolete, since Andreas' patch contains it.
Attachment #599356 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Attachment #599425 - Attachment description: and classic (under xp) included too → Amalgamated patch (r+'d by asuth, ui-r+'d by bwinton)
(Assignee)

Comment 38

6 years ago
Committed to comm-central as http://hg.mozilla.org/comm-central/rev/fd98bca81bec
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 13.0
(Assignee)

Updated

6 years ago
Attachment #599425 - Flags: approval-comm-aurora?
(Assignee)

Comment 39

6 years ago
Created attachment 599448 [details] [diff] [review]
Backport for comm-beta

Backport of attachment 599276 [details] [diff] [review] [diff] [review] for comm-beta.  Carrying forward r+/ui-r+.
(Assignee)

Comment 40

6 years ago
Comment on attachment 599448 [details] [diff] [review]
Backport for comm-beta

This backport should be committed to comm-beta.
Attachment #599448 - Flags: approval-comm-beta?
(Assignee)

Comment 41

6 years ago
Created attachment 599459 [details]
Windows XP - Fat toolbar

Andreas:

I've noticed that there's a style problem in Windows XP for the Gloda tab search toolbar - it seems a bit too tall.

See attached.

-Mike
(Assignee)

Comment 42

6 years ago
Created attachment 599462 [details]
Windows XP - Thinned toolbar

Andreas:

I fiddled with the CSS until I got it to be the same size as the default mail-bar3.  Patch is my next attachment.

-Mike
(Assignee)

Comment 43

6 years ago
Created attachment 599463 [details] [diff] [review]
Style fix for Windows XP

We'll want to apply this on top of attachment 599448 [details] [diff] [review] on beta, and then on top of attachment 599425 [details] [diff] [review] (already committed) for Aurora and Nightly.
Attachment #599463 - Flags: review?(nisses.mail)
Attachment #599425 - Flags: approval-comm-aurora? → approval-comm-aurora+
Attachment #599448 - Flags: approval-comm-beta? → approval-comm-beta+
Pushed the main patches to aurora and beta:

http://hg.mozilla.org/releases/comm-aurora/rev/2464602e9a8f
http://hg.mozilla.org/releases/comm-beta/rev/b23b75bf76e8

Attachment 599463 [details] [diff] is still outstanding.
status-thunderbird11: --- → fixed
status-thunderbird12: --- → fixed
Attachment #599463 - Flags: approval-comm-beta?
Attachment #599463 - Flags: approval-comm-aurora?
(Reporter)

Comment 45

6 years ago
As mentioned on IRC, when I tried with a fresh profile, Thunderbird had a fat toolbar due to large icons being the default. It seems it's still jumps 2px though, so writing a patch for that.
(Reporter)

Comment 46

6 years ago
Created attachment 599638 [details] [diff] [review]
style fix for XP (v2)

This should give us a search bar that matches the toolbar height we get with a fresh profile.
Attachment #599463 - Attachment is obsolete: true
Attachment #599463 - Flags: review?(nisses.mail)
Attachment #599463 - Flags: approval-comm-beta?
Attachment #599463 - Flags: approval-comm-aurora?
Attachment #599638 - Flags: review?(mconley)
(Assignee)

Comment 47

6 years ago
Comment on attachment 599638 [details] [diff] [review]
style fix for XP (v2)

Looks good.
Attachment #599638 - Flags: review?(mconley)
Attachment #599638 - Flags: review+
Attachment #599638 - Flags: approval-comm-beta?
Attachment #599638 - Flags: approval-comm-aurora?
Attachment #599638 - Flags: approval-comm-beta?
Attachment #599638 - Flags: approval-comm-beta+
Attachment #599638 - Flags: approval-comm-aurora?
Attachment #599638 - Flags: approval-comm-aurora+
(Assignee)

Comment 48

6 years ago
Attachment 599638 [details] [diff] was checked in to:

comm-central as http://hg.mozilla.org/comm-central/rev/acc5c71ae5a3
comm-aurora as http://hg.mozilla.org/releases/comm-aurora/rev/c06f0acf8326
comm-beta as http://hg.mozilla.org/releases/comm-beta/rev/c3b9327d2a21
You need to log in before you can comment on or make changes to this bug.