Closed
Bug 386600
Opened 17 years ago
Closed 14 years ago
JavaScript autocomplete for review request field
Categories
(Bugzilla :: Attachments & Requests, enhancement)
Bugzilla
Attachments & Requests
Tracking
()
RESOLVED
FIXED
Bugzilla 4.0
People
(Reporter: ted, Assigned: guy.pyrzak)
References
Details
(Whiteboard: [wanted-bmo])
Attachments
(1 file, 6 obsolete files)
4.62 KB,
patch
|
LpSolit
:
review+
|
Details | Diff | Splinter Review |
I searched but didn't find a dupe of this, so I'm filing new. I'm attaching a patch that implements dropdown autocomplete on the review request field. the JS magic is done using jQuery and a jQuery autocomplete implementation that I found online. I'm sure it could be swapped out for pretty much any JS toolkit implementation. I didn't include the jQuery source or the JS autocomplete source in my patch, since they'd just make it harder to read, and I didn't write them anyway. Presumably this could also be hooked up to the CC field, etc, but I didn't get around to that.
Comment 1•17 years ago
|
||
Comment on attachment 270598 [details] [diff] [review] add JS autocomplete to review request fields > # Certain versions are broken, development versions are > # always disallowed. >- blacklist => ['^3\.000[3-6]', '_'], >+ blacklist => ['^3\.000[3-5]', '_'], > }, > name => 'MySQL'}, There are reasons why these MySQL DBI versions are blacklisted. This should not be included in your patch.
Reporter | ||
Comment 2•17 years ago
|
||
(In reply to comment #1) > There are reasons why these MySQL DBI versions are blacklisted. This should not > be included in your patch. Yeah, oops. That was a hack to get things working on Ubuntu. Didn't mean to include that.
Reporter | ||
Comment 3•17 years ago
|
||
Removed that file from the diff.
Attachment #270598 -
Attachment is obsolete: true
Comment 4•17 years ago
|
||
Ted, without the missing JS files, what do you expect us to do with your patch?
Reporter | ||
Comment 5•17 years ago
|
||
Got my bugzilla install back up and running, you can see this in action here: http://bugzilla.rawr.homeip.net/attachment.cgi?id=1&action=edit I loaded a bunch of random usernames into my db, so just try setting r? and typing some characters into the requestee field.
Reporter | ||
Comment 6•17 years ago
|
||
Well, I hacked it together in a few hours, and I just figured bugzilla was a better home for it than sitting in my tree. Here's a complete patch, for reference.
Comment 7•17 years ago
|
||
Comment on attachment 270604 [details] [diff] [review] complete patch >Index: js/jquery-latest.pack.js >+eval(function(p,a,c,k,e,d){e=function(c){return(c<a?"":e(parseInt(c/a)))+((c=c%a)>35 Huh? What's that??
Reporter | ||
Comment 8•17 years ago
|
||
I don't know, I didn't write it, and the JS is obviously compressed, which is why I left it out of my original patch. It's just the jQuery library, downloaded from http://jquery.com/ .
Comment 9•16 years ago
|
||
Fwiw you'd be better off using the minified+gzipped jQuery, not the packed one. It's smaller, and faster on the users' machines as it doesn't have to be unpacked before use.
Assignee | ||
Comment 10•15 years ago
|
||
I've been thinking of doing a few things with the autocomplete. The requestee should be easy along with most of the other person fields. I'd also like to see something that integrates quicksearch into the duplicates field. I'll see if I can revive this along with the other autocomplete ideas I've had.
Reporter | ||
Comment 11•15 years ago
|
||
The patch here is pretty simple, I just took an off-the-shelf jQuery autocomplete plugin and attached it to the review request field. The other interesting part of the patch is queryuser.cgi, which is a CGI I added that takes a q=foo param, and returns one line of "login_name|realname" per user whose login name or real name matches "%foo%".
Comment 12•15 years ago
|
||
I know that this bug has a patch on it and lots of discussion, but it's actually a little too narrow. We want autocomplete for all user-based fields. Also, we don't want to use jQuery, but YUI, which is what is already in Bugzilla and which supports autocomplete.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → DUPLICATE
Reporter | ||
Comment 13•15 years ago
|
||
That's fine. I just hacked it into the review field because that was what annoyed me the most. Also, I don't think any of the YUI stuff existed when I wrote this patch.
Comment 14•14 years ago
|
||
Reopened, as bug 490923 finally didn't implement it.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Updated•14 years ago
|
Status: REOPENED → NEW
Updated•14 years ago
|
Whiteboard: [wanted-bmo]
Assignee | ||
Comment 15•14 years ago
|
||
It would take 1 line of code to implement this on 4.0 or trunk. But the problem is the field will not know if the users that show up are allowed to + the flag, so users will run into that after the save.
Assignee | ||
Comment 16•14 years ago
|
||
Assignee: attach-and-request → guy.pyrzak
Attachment #270599 -
Attachment is obsolete: true
Attachment #270604 -
Attachment is obsolete: true
Attachment #487037 -
Flags: review?(mkanat)
Attachment #487037 -
Flags: review?(LpSolit)
Assignee | ||
Updated•14 years ago
|
Flags: blocking4.0?
Comment 17•14 years ago
|
||
I don't see why it should block 4.0.
Severity: normal → enhancement
Status: NEW → ASSIGNED
Assignee | ||
Comment 18•14 years ago
|
||
well mostly because adding autocomplete is a big feature of 4.0 and not having it for flags is almost a bug than an enhancement.
Comment 19•14 years ago
|
||
Comment on attachment 487037 [details] [diff] [review] 4.0 Version >=== modified file 'template/en/default/flag/list.html.tmpl' > [% ELSE %] >+ [% INCLUDE global/userselect.html.tmpl What's the point to duplicate the code in the ELSE block when it's already in the IF block? Looks like both should be combined.
Attachment #487037 -
Flags: review?(mkanat)
Attachment #487037 -
Flags: review?(LpSolit)
Attachment #487037 -
Flags: review-
Assignee | ||
Comment 20•14 years ago
|
||
you were correct. I was trying to change as little code as possible b/c we're so close to rc1. Here is the simplified version.
Attachment #487037 -
Attachment is obsolete: true
Attachment #487056 -
Flags: review?(mkanat)
Attachment #487056 -
Flags: review?(LpSolit)
Updated•14 years ago
|
Flags: blocking4.0? → blocking4.0+
Updated•14 years ago
|
Target Milestone: --- → Bugzilla 4.0
Comment 21•14 years ago
|
||
Comment on attachment 487056 [details] [diff] [review] v2 The autocomplete parts look fine, but the rest of this is in code that LpSolit owns, so I'll let him review it.
Attachment #487056 -
Flags: review?(mkanat) → review+
Comment 22•14 years ago
|
||
Comment on attachment 487056 [details] [diff] [review] v2 >=== modified file 'template/en/default/flag/list.html.tmpl' >+ [% END %] > [% INCLUDE global/userselect.html.tmpl Please fix the indentation. [% INCLUDE %] should now be vertically aligned with [% END %]. >- emptyok => 1 This removal is incorrect. I can no longer remove the requestee. >+ classes => ["bz_userfield", "requestee"] Is the bz_userfield class needed? Why not adding the new CSS rule to the .requestee class instead? (and why wasn't this class needed till today, when usemenuforusers was turned on?) >+ size => 30 30 is too large, see the current tip UI. This regresses bug 455559, see the patch there. > <span style="white-space: nowrap;"> >- [% IF Param('usemenuforusers') %] > [% INCLUDE global/userselect.html.tmpl Also fix the indentation here, now that [% IF %] is gone. >+ custom_userlist => type.grant_list Don't call type->grant_list when usemenuforusers is turned off. This method is slow, see bug 528230, and is never used when this parameter is off. Note also that this patch only applies cleanly to the 4.0 branch, not to 4.1.
Attachment #487056 -
Flags: review?(LpSolit) → review-
Assignee | ||
Comment 23•14 years ago
|
||
did what LpSolit wanted. I will provide a 4.1 patch once i get an R+
Attachment #487056 -
Attachment is obsolete: true
Attachment #489363 -
Flags: review?(LpSolit)
Comment 24•14 years ago
|
||
(In reply to comment #22) > >+ classes => ["bz_userfield", "requestee"] > > Is the bz_userfield class needed? Why not adding the new CSS rule to the > .requestee class instead? (and why wasn't this class needed till today, when > usemenuforusers was turned on?) Could I have an answer to this question, please? > >+ size => 30 > > 30 is too large, see the current tip UI. This regresses bug 455559, see the > patch there. This comment hasn't been addressed. Consider this a r-.
Assignee | ||
Comment 25•14 years ago
|
||
the answer to both of your questions are "they are not needed". They should have beed deleted. My apologies. Assuming those strings are deleted, anything else?
Assignee | ||
Comment 26•14 years ago
|
||
i don't know what happened with that previous patch. i meant to delete both of the references.
Attachment #489363 -
Attachment is obsolete: true
Attachment #491593 -
Flags: review?(LpSolit)
Attachment #489363 -
Flags: review?(LpSolit)
Comment 27•14 years ago
|
||
Comment on attachment 491593 [details] [diff] [review] V4 r=LpSolit
Attachment #491593 -
Flags: review?(LpSolit) → review+
Updated•14 years ago
|
Flags: approval4.0+
Flags: approval+
Comment 28•14 years ago
|
||
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/ modified template/en/default/attachment/create.html.tmpl modified template/en/default/attachment/edit.html.tmpl modified template/en/default/flag/list.html.tmpl Committed revision 7607. Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/ modified template/en/default/attachment/create.html.tmpl modified template/en/default/attachment/edit.html.tmpl modified template/en/default/flag/list.html.tmpl Committed revision 7486.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago → 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•