717264 - Move Quick Filter Bar toggle button from tab toolbar to mail toolbar

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

The current placement of the QFB toggle button doesn't make a whole lot of sense.  It exists outside of the tab that it belongs to, and is disabled unless that tab is selected.

Blake and I are toying with the idea of removing the Quick Filter Bar button from the tab toolbar, and just having it exist as an optional button in the Customize Toolbar dialog.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

CC'ing some folks who might be interested.  I'm including davida, since it was a discussion with him that precipitated this idea.

Comment 2

[Jim Porter (:squib)]

I think we should keep it, since the button is actually useful on other tabs (potentially). Both Lightning calendar and task tabs have UI that works basically like the QFB, and it just needs to be hooked up to take advantage of the toggle button (the task tab already responds to Ctrl+Shift+K; not sure on the calendar tab).

Comment 3

[Blake Winton (:bwinton) (:☕️)]

I don't know if I understand your comment…  Are you suggesting that we have the QFB toggle button do different things per-tab?

Comment 4

[Jim Porter (:squib)]

Correct. I think the QFB toggle should be a toggle for anything in a tab that works as a filter you can show/hide. Both kinds of Lightning tabs have something like this. The address book tab might also have something like this, though maybe we just want an always-present search box for that.

I know the Lightning devs have expressed interest in doing this before, but I think they ran into an issue with it (which might be fixed by now).

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Philipp:

Did you have some input on this?

-Mike

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Given we have a search feature both in calendar and task tab, I wouldn't object to keeping it. I understand that other tab types may not profit from this though.

If you plan to remove it from the tab toolbar anyway, I'd appreciate if you could add an optional toolbar button to Lighting (both tabs) too.

squib is right though, I think Ctrl+Shift+K is not hooked up on the calendar tab. We should fix that, maybe as part of a different bug.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Jb wants this for 11.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1

Here's my first run at this.

I'm wary of landing stuff on beta, but a swath of local testing seems to indicate that this patch does what it's supposed to do.  I've written a simple test case for custom toolbars as well.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Try builds coming in here:  http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=ff1ee8bb1de8

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v2

Whoops - forgot to include my tests with that last patch.

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Try builds for Patch v2 coming in here:  http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=692962308836

Comment 12

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v3

So I was a bit overconfident when I pushed to try so soon - I just ran all Mozmill tests locally, and I had a failure in the old migration tests for TB 2 to TB 3.

This version of the patch fixes it.  The rest of the failures I got locally were random oranges we sometimes happen to get.

I'm going to hold off requesting review until I see the test results that try spits out.  If it's just the same migration test failure as I got here, then I'll go ahead and r?.  Until then, I'd rather not stuff squib's inbox with more bugmail than he needs.  :D

-Mike

Comment 13

[Thomas D. (:thomas8)]

After this patch, can users customize the mail toolbar to remove the quick filter button altogether? Chances are that many users (like me :) will never use the button because it's such a frequent action that it's really worth remembering the keyboard shortcut...

Comment 14

[Mike Conley (:mconley) (:⚙️)]

Thomas,

Yes! The Quick Filter toggle will be like any of the other toolbar buttons - you can move it to other toolbars, or get rid of it altogether.

-Mike

Comment 15

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 597947 [details] [diff] [review]
Patch v3

Review of attachment 597947 [details] [diff] [review]:
-----------------------------------------------------------------

Aside from the question below, I'm happy with this.  r=me, and I think ui-r=me, just in case you need it…

::: mail/base/modules/mailMigrator.js
@@ -197,0 +197,12 @@
> > +      // In UI version 3, we move the QFB button from the tabbar toolbar to
> > +      // to the mail toolbar.
> > +      if (currentUIVersion < 3) {
> > +        let currentSetResource = this._rdf.GetResource("currentset");
NaN more ...

Wouldn't that replace "a,qfb-show-filter-bar,b" with "a,,b"?  Do we not care, because this._setPersist will take care of it for us?

Comment 16

[Blake Winton (:bwinton) (:☕️)]

The lines of code that was referring to was supposed to be:
    currentSet = currentSet.replace(/(^|,)qfb-show-filter-bar($|,)/,
    "$1$2");
:P

Comment 17

[Mike Conley (:mconley) (:⚙️)]

Blake:

I borrowed this technique from the Firefox version of migrateUI, where they do the same thing (see:  http://mxr.mozilla.org/comm-central/source/mozilla/browser/components/nsBrowserGlue.js#1183)

That, coupled with the fact that the migration seems to work and the test passes, suggests to me that setPersist takes care of this for us.

-Mike

Comment 18

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 597947 [details] [diff] [review]
Patch v3

I believe we want this for TB 11.

Comment 19

[Mike Conley (:mconley) (:⚙️)]

Landed on comm-central as http://hg.mozilla.org/comm-central/rev/78966d586c8a

Comment 20

[Mike Conley (:mconley) (:⚙️)]

Landed on comm-aurora as http://hg.mozilla.org/releases/comm-aurora/rev/80897e9cafe7

Comment 21

[Mike Conley (:mconley) (:⚙️)]

Landed on comm-beta as http://hg.mozilla.org/releases/comm-beta/rev/17b0b1d4af4d