Problem/Motivation

When selecting an option from the auto-complete list the form does not auto-submit even though the option is set in config and the appropriate class is on the element.

Proposed resolution

The javascript refers to Drupal.jsAC (which has been replaced by Drupal.autocomplete, which is a jQueryUI auto-complete element) and Drupal.settings (which has been replaced by drupalSettings). search_api_autocomplete.js needs to be changed to take into account these changes.

Remaining tasks

Update javascript
Review

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#7 2825669-7.patch1.29 KBjofitz
#7 interdiff_4-7.txt7.34 KBjofitz
#4 2825669-4.patch7.63 KBjofitz
#4 interdiff_2-4.txt570 bytesjofitz
#2 2825669-2.patch7.47 KBjofitz

Comments

Jo Fitzgerald created an issue. See original summary.

jofitz’s picture

jofitz
he/him
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 2825669-2.patch7.47 KB

Most of the javascript is no longer necessary because it is covered by the jQueryUI auto-complete.

This patch uses similar methods to the previous javascript to auto-submit the form if the auto-submit class is present.

dawehner’s picture

dawehner
Primary language German
commented

Nice simplification!

  1. +++ b/js/search_api_autocomplete.js
@@ -5,195 +5,24 @@
+  Drupal.autocomplete.options.select = function (event, ui) {
+    oldSelect.call(this, event, ui);
+    if ($(this).hasClass('auto_submit')) {
+      var selector = getSetting(this, 'selector', ':submit');
+      $(selector, this.form).trigger('click');
+    }
+  };

    It would be nice to have a one line summary of what this line is doing. 

  2. +++ b/js/search_api_autocomplete.js
@@ -5,195 +5,24 @@
+    if (typeof search == 'undefined'
+      || typeof drupalSettings.search_api_autocomplete == 'undefined'
+      || typeof drupalSettings.search_api_autocomplete[search] == 'undefined'
+      || typeof drupalSettings.search_api_autocomplete[search][setting] == 'undefined') {
+      return defaultValue;

    Could you use $.extend() maybe?

jofitz’s picture

jofitz
he/him
commented
StatusFileSize
new interdiff_2-4.txt570 bytes
new 2825669-4.patch7.63 KB

Add a line of explanation.

@dawehner How do you suggest $.extend() could be used?

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs review » Needs work

Would be great if this now worked better in D8 and we don't need all those workarounds anymore. So, thanks for proposing this, and also fixing auto-submit!

However, you also remove some other code, implementing functionality specific to this module: linked results (which will take you to a new page, not just select a suggestion), custom autocomplete script paths, customized delays, and maybe others. I can't imagine D8's autocomplete already provides all of these, and even if it does, we'd have to integrate with it properly, I guess. (Or is this already done?)
So, unless this will still be working, please don't just remove all other code as well.

Also, I think we should still have the check whether Drupal.autocomplete is even set before doing anything else, as a safety precaution. Yes, our script should never be included without Core's autocomplete.js, but I'd say it's better to be on the safe side. (Or are there examples in Core that assume that this is always working correctly? I guess with the Libraries system, this is now really a lot less likely to go wrong.)

dawehner’s picture

dawehner
Primary language German
commented

I guess with the Libraries system, this is now really a lot less likely to go wrong.)

Yeah, the library system really ensures that this cannot go wrong, and well, if this goes wrong, there is more broken on your site, for sure.

jofitz’s picture

jofitz
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff_4-7.txt7.34 KB
new 2825669-7.patch1.29 KB

@drunken monkey, perhaps you are right, I may have got a little delete-happy so I have re-instated what I removed. None of it will ever run because it is surrounded by a condition that will be FALSE in D8, but it can serve as a reminder for other tasks that need to be completed.

ressa’s picture

ressa
he/him
commented

I just tested the patch, and it does indeed activate the auto-submit function, thanks @Jo Fitzgerald!

#2825703: Changing settings don't take effect is still an issue, though - values in "Minimum length of keywords for autocompletion" and "Delay in ms" is ignored.

ressa’s picture

ressa
he/him
commented
Status: Needs review » Reviewed & tested by the community

Auto-submit works as expected after applying the patch.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Reviewed & tested by the community » Fixed

Well, I guess it's an improvement over what we currently have.
Added a @todo and committed.
Thanks again!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.