Last Comment Bug 190892 - Radio button for "run this query" appears needlessly if you don't have saved queries
: Radio button for "run this query" appears needlessly if you don't have saved ...
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Query/Bug List (show other bugs)
: 2.17.3
: All All
: -- normal (vote)
: Bugzilla 2.18
Assigned To: Dave Miller [:justdave] (justdave@bugzilla.org)
: default-qa
Mentors:
Depends on:
Blocks: bz-zippy
  Show dependency treegraph
 
Reported: 2003-01-27 18:36 PST by Dave Miller [:justdave] (justdave@bugzilla.org)
Modified: 2012-12-18 20:46 PST (History)
2 users (show)
justdave: approval+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch (1.24 KB, patch)
2003-03-23 20:50 PST, Dave Miller [:justdave] (justdave@bugzilla.org)
gerv: review+
Details | Diff | Splinter Review
Patch v2 (1.74 KB, patch)
2003-03-23 23:49 PST, Dave Miller [:justdave] (justdave@bugzilla.org)
no flags Details | Diff | Splinter Review
Patch v3 (1.70 KB, patch)
2003-03-23 23:53 PST, Dave Miller [:justdave] (justdave@bugzilla.org)
justdave: review+
Details | Diff | Splinter Review

Description Dave Miller [:justdave] (justdave@bugzilla.org) 2003-01-27 18:36:07 PST
If you don't have any saved queries, the radio button for "Load/Run/Forget this
remembered query" does not appear.  However, the "Run this query" radio button
is still there, and it looks silly by itself.

If there are no remembered queries, the first radio button should be a hidden
form field and not a visible radio button.
Comment 1 Dave Miller [:justdave] (justdave@bugzilla.org) 2003-03-23 20:49:16 PST
-> mine
Comment 2 Dave Miller [:justdave] (justdave@bugzilla.org) 2003-03-23 20:50:12 PST
Created attachment 118265 [details] [diff] [review]
Patch
Comment 3 Gervase Markham [:gerv] 2003-03-23 23:35:55 PST
Comment on attachment 118265 [details] [diff] [review]
Patch

>Index: template/en/default/search/knob.html.tmpl
>===================================================================
>+    [%- IF namedqueries.size > 0 %]

All these [%- minuses are unnecessary; it may seem like a nit, but we should
only use them when we need them, because otherwise a) people spend time trying
to work out why they are there, and b) it messes up the formatting of the
generated HTML.

In the vast majority of cases, POST_CHOMP plus a sensible indented formatting
scheme produces good HTML.

>         <input type="checkbox" id="remember" name="remember" value="1" 
>                onclick="remCheckboxChanged()">
>+        [%- IF namedqueries.size > 0 %]
>         <label for="remember">and remember it</label>
>+        [%- ELSE %]
>+        <label for="remember">Remember this search</label>
>+        <input type="hidden" id="cmdtype-doit"
>+               name="cmdtype" value="doit">
>+        [%- END %]

I'm pretty sure that the inclusion of doit shouldn't be conditional. We always
send this.
Nit: please indent inside the new tags.

r=gerv if you fix these two issues.

Gerv
Comment 4 Dave Miller [:justdave] (justdave@bugzilla.org) 2003-03-23 23:43:26 PST
The inclusion of "doit" isn't conditional.  How it's included is.  If there are
named queries available, it's a radio button.  If there aren't, it's a hidden input.

As for the [%-, we don't have PRE_CHOMP on, and I got unnecessary blank lines in
the HTML if I didn't use those.

My lack of indenting was to make the resulting HTML be indented properly, but I
suppose that doesn't matter much and it'll make the template more readable with
the additional indent... :)  so I'll fix that.
Comment 5 Dave Miller [:justdave] (justdave@bugzilla.org) 2003-03-23 23:49:32 PST
Created attachment 118271 [details] [diff] [review]
Patch v2

Not sure what I was thinking on the [%-, took the - out and it didn't product
blank lines. :)  this one has the indenting.
Comment 6 Dave Miller [:justdave] (justdave@bugzilla.org) 2003-03-23 23:53:13 PST
Created attachment 118272 [details] [diff] [review]
Patch v3

removing the stupid indent I missed that made the diff look screwy :)
Comment 7 Dave Miller [:justdave] (justdave@bugzilla.org) 2003-03-23 23:58:30 PST
Comment on attachment 118272 [details] [diff] [review]
Patch v3

carrying forward r=gerv
Comment 8 Dave Miller [:justdave] (justdave@bugzilla.org) 2003-03-24 00:00:51 PST
Checking in template/en/default/search/knob.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/search/knob.html.tmpl,v 
<--  knob.html.tmpl
new revision: 1.9; previous revision: 1.8
done

Note You need to log in before you can comment on or make changes to this bug.