Closed
Bug 719363
Opened 13 years ago
Closed 13 years ago
add an "instant search" tab to query.cgi
Categories
(bugzilla.mozilla.org :: User Interface, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: glob, Assigned: glob)
Details
Attachments
(3 files)
9.75 KB,
patch
|
dkl
:
review-
|
Details | Diff | Splinter Review |
10.59 KB,
patch
|
dkl
:
review+
|
Details | Diff | Splinter Review |
13.26 KB,
patch
|
Details | Diff | Splinter Review |
several people have commented that they use the possible duplicates function on enter_bug to quickly find bugs.
it won't take a lot of work to add a "quick search" tab to query.cgi, which simply displays possible-duplicate results for the selected product. we should use the implementation from the guided bug entry as a basis as it also searches related products (eg. searching in the firefox product will also show matches from the core product).
this patch implements "instant search".
this is very much a slightly edited copy of the guided's possible-dupes code.
Summary: add a "quick search" tab to query.cgi → add an "instant search" tab to query.cgi
Updated•13 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
Comment 2•13 years ago
|
||
Comment on attachment 589833 [details] [diff] [review]
patch v1
Review of attachment 589833 [details] [diff] [review]:
-----------------------------------------------------------------
Comments:
1. Why duplicate this much JS code instead of reusing what was already in js/bug.js?
2. Why not make this an extension instead? Should be technically possible.
Issues:
1. Doesnt store 'instant' in the cookie for query.cgi so when you come back later it defaults back to specific. (See Bugzilla::Search::IsValidQueryType)
::: js/instant-search.js
@@ +18,5 @@
> + }
> + Dom.get('content').focus();
> +});
> +
> +var instantSearch = {
Use YAHOO.bugzilla namespace as much as possible.
var bz = YAHOO.bugzilla;
bz.instantSearch = {
etc.
@@ +138,5 @@
> + try {
> + // run the search
> + Dom.removeClass(instantSearch.elList, 'hidden');
> +
> + instantSearch.dataTable.showTableMessage(
nit: weird indention.
@@ +151,5 @@
> + id: ++instantSearch.counter,
> + params: {
> + product: instantSearch.getProduct(),
> + summary: query,
> + limit: 12,
Should we show more than 12?
::: template/en/default/search/search-instant.html.tmpl
@@ +14,5 @@
> + 'js/instant-search.js', ]
> + yui = [ 'datatable', 'container' ]
> +%]
> +
> +[% default.product.0 = 'Firefox' IF default.product.0 == '' %]
Loading product=Bugzilla in the URL does not select that value in the drop down. It basically defaults to the first product in the list.
This issue goes away if I change the above to:
[% default.product.push('Firefox') IF !default.product.size %]
@@ +17,5 @@
> +
> +[% default.product.0 = 'Firefox' IF default.product.0 == '' %]
> +
> +<script>
> +instantSearch.setLabels( {
I do not see that the descriptions show up for me anywhere. If I hover over the column labels, there is
not a tooltip or similar. Where do these descriptions show up?
@@ +45,5 @@
> + <optgroup label="[% c.name FILTER html %]">
> + [% FOREACH p = user.get_selectable_products(c.id) %]
> + [% IF p.components.size %]
> + <option value="[% p.name FILTER html %]"
> + [% " selected" IF lsearch(default.product, p.name) != -1 %]>
Bonus: Add title attribute to each option with the products summary so that it is displayed on mouseover or maybe show the description to the right
similar to enter_bug.cgi. Not a show-stopper as specific search doesn't do it either but it would be nice addition.
Attachment #589833 -
Flags: review?(dkl) → review-
thanks for the quick review dkl :)
> 1. Why duplicate this much JS code instead of reusing what was already in
> js/bug.js?
the code in js/bug.js doesn't do exactly what we need to do (no cross-product matching, different columns, etc), and it wasn't designed to be easily extended. once you replace the required methods, there isn't much left.
> 2. Why not make this an extension instead? Should be technically possible.
that's what i initially thought too, however there aren't any hooks, so i followed the lead taken when the google search tab was added. i don't think adding hooks for this is worth the effort required.
> > + instantSearch.dataTable.showTableMessage(
>
> nit: weird indention.
TIL 005whitespace.t doesn't check js files for tabs :)
> > + limit: 12,
>
> Should we show more than 12?
sure, changed to 20. i don't want to show too many bugs here.
> > +instantSearch.setLabels( {
>
> I do not see that the descriptions show up for me anywhere. If I hover over
> the column labels, there is not a tooltip or similar. Where do these
> descriptions show up?
this code sets the column header text: "Bug ID", "Summary", "Component", "Status".
Attachment #590119 -
Flags: review?(dkl)
Comment 4•13 years ago
|
||
>> 2. Why not make this an extension instead? Should be technically possible.
>
> that's what i initially thought too, however there aren't any hooks, so i
> followed the lead taken when the google search tab was added. i don't think
> adding hooks for this is worth the effort required.
The reason I didn't do the google search as an extension at the time as I couldn't come up with an easy way to override Bugzilla::Search::IsValidQueryType to make it store the format cookie for query.cgi. I am attaching a proof of concept for InstantSearch as an extension by putting a hook there. Works fine now and I could probably convince upstream to take the hook as well. With this hook the Google search could be moved to an extension.
But we can not do this and do it the current way if you desire.
>> > +instantSearch.setLabels( {
>>
>> I do not see that the descriptions show up for me anywhere. If I hover over
>> the column labels, there is not a tooltip or similar. Where do these
>> descriptions show up?
>
> this code sets the column header text: "Bug ID", "Summary", "Component",
> "Status".
Ah, brain fart. I was thinking the longer description such as when you hover over a field name in show_bug.cgi. Yes I see now that it is just the column header description.
dkl
Comment 5•13 years ago
|
||
Comment on attachment 590119 [details] [diff] [review]
patch v2
Review of attachment 590119 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good and works for me. If we decide not to do an extension after all, then r=dkl.
Attachment #590119 -
Flags: review?(dkl) → review+
thanks dkl; i've committed the non-extension version.
Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bmo/4.0/
modified Bugzilla/Search.pm
added js/instant-search.js
added template/en/default/search/search-instant.html.tmpl
modified template/en/default/search/tabs.html.tmpl
Committed revision 8038.
Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bmo/4.2/
modified Bugzilla/Search.pm
added js/instant-search.js
added template/en/default/search/search-instant.html.tmpl
modified template/en/default/search/tabs.html.tmpl
Committed revision 8029.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 7•12 years ago
|
||
When is this going to turn up in a Bugzilla release (or as a downloadable extension)? Seems to be only available on b.m.o at the moment, as far as I can see...
Comment 8•12 years ago
|
||
(In reply to Mark Clements from comment #7)
> When is this going to turn up in a Bugzilla release (or as a downloadable
> extension)? Seems to be only available on b.m.o at the moment, as far as I
> can see...
very much a BMO customization and not sure if we will ever get to push upstream unless someone else wants to take it. So for now, best to just get it from our bzr repo:
http://bzr.mozilla.org/bmo/4.2/view/head:/template/en/default/search/search-instant.html.tmpl
Also requires some code from the extensions/GuidedBugEntry extension to work.
dkl
You need to log in
before you can comment on or make changes to this bug.
Description
•