Closed
Bug 190892
Opened 22 years ago
Closed 21 years ago
Radio button for "run this query" appears needlessly if you don't have saved queries
Categories
(Bugzilla :: Query/Bug List, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.18
People
(Reporter: justdave, Assigned: justdave)
References
Details
Attachments
(1 file, 2 obsolete files)
1.70 KB,
patch
|
justdave
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 2•21 years ago
|
||
Assignee | ||
Updated•21 years ago
|
Attachment #118265 -
Flags: review?(gerv)
Comment 3•21 years ago
|
||
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
Attachment #118265 -
Flags: review?(gerv) → review+
Assignee | ||
Comment 4•21 years ago
|
||
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.
Assignee | ||
Comment 5•21 years ago
|
||
Not sure what I was thinking on the [%-, took the - out and it didn't product blank lines. :) this one has the indenting.
Attachment #118265 -
Attachment is obsolete: true
Assignee | ||
Comment 6•21 years ago
|
||
removing the stupid indent I missed that made the diff look screwy :)
Attachment #118271 -
Attachment is obsolete: true
Assignee | ||
Comment 7•21 years ago
|
||
Comment on attachment 118272 [details] [diff] [review] Patch v3 carrying forward r=gerv
Attachment #118272 -
Flags: review+
Assignee | ||
Comment 8•21 years ago
|
||
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
Status: NEW → RESOLVED
Closed: 21 years ago
Flags: approval+
Resolution: --- → FIXED
Assignee | ||
Updated•21 years ago
|
Target Milestone: --- → Bugzilla 2.18
Updated•12 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•