The attached patch enhances the autocomplete list in serveral ways:

  • It improves the code quality of the autocomplete.js and moves all JS objects to the Drupal.autocomplete namespace.
  • The cursor is now moved to the end of the textfield in Safari when a user selects an autocomplete item.
  • It allows modules to provide more than just one string for an autocomplete entry, allowing “rich” items. For example, you can use bold formatting, links and even images (e.g. avatars when autocompleting users). This allows modules to give more context when selecting an item. E.g. when a node should be selected, it’s often hard to tell from the title of the node if it’s the correct one. Additional information are often useful.
    Also, there is a new theme function theme_autocomplete_item that allows themes to modify the structure of an autocomplete item.
  • There is a new field info in the autocomplete JSON object which can contain general information about the autocompletion. In the patch it is for example used to display the user who many items in addition to the 10 displayed were found. This is a freeform HTML field that is not selectable (as opposed to regular items).
CommentFileSizeAuthor
#10 autocomplete_3_1.patch20.06 KBkkaefer
autocomplete_3_0.patch18.92 KBkkaefer
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

It allows modules to provide more than just one string for an autocomplete entry

Isn't this possible already ?
the autocomplete widget for CCK nodereference fields can currently be set to return stylable Views-generated HTML.

nedjo’s picture

This looks good, thanks for giving autocomplete some needed attention. I've given the code only a quick initial review. The namespace introduction is a good simple improvement. I like the move to pass autocomplete data in settings rather than the current awkward encoding in a form field.

There are various other issues with the autocomplete implementation. Likely they should be left for separate patches. I'll mention them, though, in case you're inspired to tackle one or more along with these improvements.

  1. Hard-coding in _element_info(). Here, like many other places (notably our form element theme functions), we've rather crudely hacked js behaviours into core. The yardstick should be, can this same approach be applied by contrib modules (where we don't have the option of hacking core)? If not, then we're not doing it correctly. In this case, we should look at registering a #process callback to the textfield form element in system_elements() to handle the processing.
  2. It would be nice to see the whole popup with its key events factored out into e.g. popup.js, (like we have progress.js, not a behavior but a helper object) as it could be used beyond autocomplete.
  3. In the Activeedit module in Javascript Tools I've implemented a behaviour where we respond to an autocomplete return with no results by offering to create a new record (loading the form via AJAX and rendering it in place). To implement this, I had to override the whole Drupal.ACDB.prototype.search function. It would be nice to leave an opening for contrib behaviours to respond to a 'no matches' return.

A comment and question or two:

  • Probably we should have exit(); at the end of drupal_autcomplete().
  • Setting this.popup = to a jquery result set is an interesting approach. The subsequent this.popup[0].owner seemed awkward the first time I looked at it, but I see the power of returning a jquery reference rather than a direct object reference.
  • $(this.input.form).submit(Drupal.autocomplete.submit); Why submit the autocomplete when the form is being submitted?
Dries’s picture

Does core take advantages of any of the new features? Could it take advantage of them? It's always nice to set good examples.

Update: looks like the user and taxonomy code takes advantage of this. Care to share a screenshot? It would save many of us time to setup such a large list of taxonomy terms. :-)

The taxonomy and user module changes needs more work. It's not a good idea to use the pager, and then to read out internal pager variables by setting them global. That's a hack, IMO.

This patch has great potential, but needs a little bit more work. ;)

kkaefer’s picture

Isn’t this possible already ?
the autocomplete widget for CCK nodereference fields can currently be set to return stylable Views-generated HTML.

Yes, this is possible at the moment, but not officially supported. The implementation for this is not really bullet-proof at the moment. Currently, it’s done using .html('<div>'+ matches[key] +'</div>') with matches[key] being the “string” returned from autocomplete. However, I could imagine that there are some weird combinations of HTML tags that confuse jQuery and break the autocompletion. Therefore, I changed that to the “correcter” way .html(matches.matches[key]). Using HTML in the autocomplete string is now recommended and “officially allowed”.

I like the move to pass autocomplete data in settings rather than the current awkward encoding in a form field.

Yeah, forgot to mention that. I think killes once had some issues with differing IDs in the hidden form field and the actual autocomplete field.

nedjo’s first point is so true: Currently, we add the autocomplete JS files in a theme function instead of adding them programmatically somewhere during the build process. The second point is also a logical step. Drupal.autocomplete.field is completely independent of Drupal.autocomplete.handler and can be used separately. In fact, I’ve even used to do non-AJAX autocompletion.

Probably we should have exit(); at the end of drupal_autcomplete().

We had exit; calls there before, but I removed them on purpose. Just exiting in the middle of a script makes it impossible for other modules do perform actions in hook_exit(). And if everything is correctly implemented (like it is atm), everything works fine and we don’t get JavaScript errors. If we do, we made something wrong (e.g. not checking for the Content-type when outputting something). So this can be understood as a kind of “indicator” for the exitting system working correctly.

$(this.input.form).submit(Drupal.autocomplete.submit); Why submit the autocomplete when the form is being submitted?

You misunderstood something here. There is no submit handler called in this line. However, a new submit handler (the function Drupal.autocomplete.submit) is added as submit handler to the current form. This submit function (which has been present before) checks if the autocomplete popup is open. So when the user hits enter for the first time, the popup closed instead of the form being submitted right away.

The taxonomy and user module changes needs more work. It’s not a good idea to use the pager, and then to read out internal pager variables by setting them global. That’s a hack, IMO.

Yes, it could be considered a hack and could be refactored. Any suggestions?

Steven’s picture

Status: Needs review » Needs work

It has always been possible to use HTML in the autocomplete matches. There was nothing hackish or unofficial about this, and this is why check_plain() was needed for autocomplete matches. I don't see what the presence of a wrapper div would do to change that. It just ensures the items stack vertically and automatically have a block-level enclosing. PS: <code> tags on Drupal.org escape for you.

While I applaud moving towards clean settings rather than hidden form items, and cleaning up the JS code, I'm not sure about some of the other bits though:

  • form_autocomplete() is a weird function. It adds the JS headers, but not the CSS class... so it is useless for re-use. Either rename it and document it as an internal helper, or just move it back in. It's only two lines after all.
  • theme('autocomplete_item') seems a very useless function. It doesn't give the themer much useful freedom, as it hardcodes the "primary / secondary / description" format and gives you no control over what information actually appears. There is very little you can do here that you can't do with CSS already. We should only have themables for each specific autocomplete type IMO.

    I also think we should do more useful things with these in core, for example by displaying avatars in user results and displaying the number of nodes that have a certain tag in brackets (and grayed out) behind the tag name.

  • The classes 'primary' and 'secondary' are used elsewhere in Drupal for menus and in many themes, and should be avoided. They don't seem appropriate here either.
  • Putting links in the autocomplete seems weird and dangerous. For one thing, entire list item lights up when you hover over it, including for the links. To make it worse, clicking such a link will lead you away from the form, leading to possible data loss. I think the information in the autocomplete item should be enough to make a decision.
  • The styling appears weird, as we don't use bold for the form item contents by default. I'd say use a normal font weight for main, and gray out the secondary attributes. I'd also avoid floating the secondary attribute right, as this puts too much distance between it and the main label.
  • The cursor style was removed, giving me an i-beam on Safari. This is inappropriate.
smk-ka’s picture

This looks very interesting, as I'm currently dealing with dynamically altered autocomplete fields. By using a global storage (Drupal.autocomplete.handlers) it'd be finally possible to alter the autocomplete uri on the fly, ie. extend it with a dynamic value from another form field.

The only question that arises for me is: could the code by changed to store the handlers by id instead of uri? That is, change Drupal.autocomplete.attach() to

    if (!handlers[id]) {
      handlers[id] = new Drupal.autocomplete.handler(url);
    }

instead of assigning to handlers[uri]? I'm primarily asking if this would have any negative consequences. The use should be obvious: the autocomplete uri could be much easier updated, because I can access the storage by a static id instead of an ever changing uri.

smk-ka’s picture

Couldn't we even go beyond the above said and make the two central handlers dealing with the autocomplete results – Drupal.autocomplete.field.found() and Drupal.autocomplete.field.select() – user-definable? Placing the handlers in the very same Drupal.settings object and falling back to the default if nothing has been overridden would allow a 3rd party module to reuse 90+% of the existing code. It would just have to provide said functions and set them via a new forms api element #autocomplete_handler, for example. Of course, refactoring common stuff out of Drupal.autocomplete.field.found() would make such "add-ons" even shorter, probably leaving only the block commented // Prepare matches in.

Gurpartap Singh’s picture

Any updates?

Stefan Nagtegaal’s picture

Please update or re-roll the patch..

I'll give this a proper review when the code is updated or the patch is rerolled.
(While i'm not a jQuery guru, I can not test the patch right now)

kkaefer’s picture

Status: Needs work » Needs review
FileSize
20.06 KB
  • Bugfix: When the user clicked on a autocomplete suggestion, the autocomplete field was not focused afterwards.

@Steven:

  • Added the cursor style back in. I didn’t get an I-Beam before, so I removed it, but obviously this can depend on the configuration.
  • No central theme_autocomplete_item() anymore.
  • Merged form_autocomplete() back into every single autocomplete function.
  • Taxonomy terms now have their description shown next to them (small & grey).
  • No links in autocomplete lists.

@smk-ka: The reason we use url here is that we can have multiple autocomplete fields with the same url per page. Now, if you type “ab” in one field, the results are cached. If you type “ab” in the other field with the same AJAX callback but another ID, it requeries the server instead of obtaining the results from the cache. Do you have a use case where you want to change autocomplete callbacks? Wouldn’t it be easier to negiocate that on the server side?

By the way, it is already possible to override every single autocomplete function: Just create a new object and overwrite the function (yes, JavaScript is that flexible!):

var x = new Drupal.autocomplete.field($('#' + id)[0], handlers[url]);
x.search = function(searchString) {
  // Do your stuff here
  this.owner.found(...);
}

There are still no avatars in user autocompletion as I think that the popup could grow too big (10 * 85 pixels = 850 pixels at least). Maybe we can look into resizing them, but afaik, we don’t have small avatars in core as of now.

Now, the title is not bold anymore in its entirety. Instead, the parts of the text the user entered is highlighted. If the user enters “ue”, the last two characters of “Blue” are now bold.

Also, it is now possible to search for any parts of user names instead of just from the beginning. Searching for “kaefer” will now also return “kkaefer”. with all but the first character highlighted.

catch’s picture

Status: Needs review » Needs work

No longer applies. This is usability so leaving in D6.

Fool2’s picture

I'd just like to add on and say that this is a necessary update to autocomplete functionality.

There are countless situations like http://drupal.org/node/52568 where you would want the autocomplete display value (even in the textfield) to be different than the returned value from the form.

This will happen anytime there is a non-unique display item (like node title or a user's name) and a non-displayable unique ID (like uid or nid). It presents information that is useless to the user. While it may be acceptable for entering such data, going back and editing a field like that means you just see a meaningless number.

I will be trying this code but it probably still needs work...

The user avatar problem can be solved with imagecache. We can create a module for enhanced user autocomplete with a dependency or optional extension if imagecache is enabled.

Pancho’s picture

Version: 6.x-dev » 7.x-dev

Moving feature requests to D7 queue.

attiks’s picture

Just bumping, because we had a discussion at FosDem about autocomplete in core, what people are looking for is something like this
http://jquery.bassistance.de/autocomplete/ ans since we're using jquery 1.2.3, I guess it can't be that hard to do :p

For the moment I don't have the time to do this myself, but ...

Cheers,
Peter

Wim Leers’s picture

Subscribing.

Fool2’s picture

I'm gonna bring this up at MIT Codesprint after Drupalcon. I guess we'll see if we do it!

molly_n’s picture

subscribing...

criznach’s picture

Subscribing...

I had conversations with several people about this in Boston. I've been working on an add-on module for a Drupal 5x client that adds the unique key behavior described above. It's not currently a one-size-fits-all solution. In addition, I've done some client-focused autocomplete work for taxonomy that routes through a theme function. In Boston we updated my 5x module to a proof of concept for D6 but it needs a JS overhaul.

So I think all of these things are possible. The problem I anticipate with the unique key situation is usability. What happens for screen readers or JS-disabled situations? I propose gathering the possible responses with a user-supplied function that returns a structured array. Then upon validation error (indicating that JS may not have been enabled) we display a select list of the possible matches. Some modules like nodequeue append the unique ID to the displayed string like:

Node Title [nid0234]

This works to a point, but no human being should be expected to conform to this format.

Quicksketch suggested gathering the autocomplete parameters using the JS settings array. I think this is a great idea. He also encouraged me to work on the HEAD branch rather than a contrib module for D6.

I'd also like to see a client-side API for clearing the autocomplete cache or manually adding items. This is something that I worked around in my 5x solution.

bcn’s picture

sub'ing

adub’s picture

subscribing, and second #14

Robin Monks’s picture

Subscribing

yonailo’s picture

subscribing.

robertgarrigos’s picture

subscribing

mlncn’s picture

Science Collaboration Framework could use this for both an enhanced nodereference and custom autocomplete fields. So... uh... subscribing until i have more to offer.

benjamin, Agaric Design Collective

mgifford’s picture

Issue tags: +Accessibility

There are nice auto complete options in both google & yahoo. Important to have this function available to screen readers.

mohammed76’s picture

hi. @mgifford, how exactly is this related to screen readers and accessibility? thanks.

mgifford’s picture

Look at #18. But also feedback from a blind user who was surprised to hear that there were autocomplete features because his modern screen reader didn't display it.

Mike

Aren Cambre’s picture

Related improvement that should probably be folded into this: #490092: Autocomplete should have everything selected when clicked

GuillaumeDuveau’s picture

I think it's important to allow to show anything to the user, while passing something else to the module. Otherwise we always have to process the entry.

The node selection is a good example, like in CCK Nodereference. Since the node title is not unique, we need to specify also the NID :
- for the user, but knowing the difference of NIDs is not very usable
- for Drupal, otherwise only the first result value will be displayed

Resolving that issue would suppose :
- to have the NID in the returned JSON, but hidden from the user
- when a result is clicked on, to display the node title in the Nodereference Autocomplete field, but it should not be the value, it is the NID which should be the value (is this even possible ??? maybe with Javascript ? modifying the display of the value but not really the value itself ?)
- same problem when editing a node with an already filled-in value in the autocomplete field.

Consider this as a big "Subscribe", work on core is still beyond my skills :)

marcoBauli’s picture

second #14: the autocomplete script at http://jquery.bassistance.de/autocomplete/ works also when pasting text in the textfield.

Actually if you try to copy/paste content in an autocomplete field in Drupal, makes autocomplete fail and give out all values.

Copy/Paste functionality would be nice in Drupal too.

mgifford’s picture

Yes, patch in comment #10 is pretty busted now.

How did you apply the auto complete script in #14 or are you just proposing it?

Was just, just updated it seems -->> 22-Aug-2009

Dane Powell’s picture

I have noticed something very annoying when using Keyword Autocomplete, which uses autocomplete.js to auto-complete a search box with popular search strings. Basically, a user has to hit enter/return TWICE to submit the search form if the popup window is open, even if they have strictly typed their search term and not selected anything in the popup. I understand that we don't want to accidentally submit the form if people are selecting items in the popup- but instead of blocking the form submit on the enter/return keystroke whenever the popup is open, can we just block the submit if an item in the popup menu is actually selected?

mgifford’s picture

@marcoBauli I'm not sure that the software license or ownership of the code is clear here - http://jquery.bassistance.de/autocomplete/

This looks like it's not as actively developed, but more official - http://plugins.jquery.com/project/jq-autocomplete

There's this one http://dyve.net/jquery/?autocomplete & variations http://www.pengoworks.com/workshop/jquery/autocomplete.htm

Looks interesting, but not Apache license & not GPL - http://code.google.com/p/jqac/

Is there a reason for a drupal specific auto-complete form? have any of the ones above been tested for accessibility? Others?

mgifford’s picture

Swapping out jquery code isn't going to be an option at this stage. However for D7, perhaps adding some Aria live regions could help & cause minimal disruption.

Everett Zufelt’s picture

Version: 7.x-dev » 8.x-dev

If anyone wants to tackle this before 7.x RC I'd be happy to provide some support. But, I really think that this is a problem best dealt with upstream by jQuery UI folk. AFAIK, does not cripple functionality.

mgifford’s picture

Can this be brought into D7 using the Accessible Helper Module?

Everett Zufelt’s picture

Issue tags: -Accessibility
chien_fu’s picture

So is there still a plan to patch the D6 version? This functionality would still be a huge benefit.

yched’s picture

@chien_fu : nope. Work happens on the D8 branch now. D7 and D6 can receive bugfixes. D7 *might* receive a couple non-API-breaking, but this won't go in core D6. Nothing prevents a better autocomplete in contrib, though.

mdupont’s picture

Marked #80565: enhanced autocomplete widget as a duplicate of the issue.

mdupont’s picture

Marked #75156: FreeTagging breaks down under this condition as a duplicate of this issue.

klonos’s picture

...curious to see where this will go.

Everett Zufelt’s picture

I'd love to get a feature list at #675446: Use jQuery UI Autocomplete of required / wanted / not required features for autocomplete so that I can find an accessible jQuery based solution.

I am very strongly in favor of us using a 3rd party UI component here and not a home grown solution.

mgifford’s picture

Why maintain code we don't have to?

nod_’s picture

Some good idea, how about rerolling the patch and checking what still apply?

nod_’s picture

Status: Needs work » Closed (duplicate)

Seems like the only thing left for this patch is the themable autocomplete result. That's possible with jQuery UI. closing.

nod_’s picture

Version: 8.x-dev » 7.x-dev
Status: Closed (duplicate) » Needs work

Sorry, that's still a valid feature request for D7. need reroll.

j0rd’s picture

Need this myself right now for custom widget I'm making. Drupal core auto-complete is too useless to be reused. Additionally not being able to store a different value than what's shown also makes it horrible from a UX standpoint.

mgifford’s picture

Issue summary: View changes
Issue tags: +autocomplete

I think this should probably be marked as won't fix in Core for D7. Maybe there's a module that can take this on.

droplet’s picture

Status: Needs work » Closed (won't fix)