The module assumes the use of Drupal's core autocomplete widget. The active_tags.js file has references to "#autocomplete" div ids and "input" form elements. These items may not be present in replacement widgets.

The javascript code needs to be more generic by identifying the autocomplete field and using it as a reference throughout.

An updated patch will be posted shortly.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ron_s created an issue. See original summary.

ron_s’s picture

Status: Active » Needs review
FileSize
778 bytes

Attached is a patch for review.

internal’s picture

Issue tags: +bootstrap theme

We also need this for enter key.

diff --git a/sites/all/modules/active_tags/active_tags.js b/sites/all/modules/ac
index ef7e215..0766c42 100644
--- a/sites/all/modules/active_tags/active_tags.js
+++ b/sites/all/modules/active_tags/active_tags.js
@@ -33,7 +33,7 @@ activeTags.checkEnter = function (event) {
$('#autocomplete').each(function () {
this.owner.hidePopup();
});
- $(this).parent().find('.at-add-btn').click();
+ $('.at-add-btn').click();
event.preventDefault();
return false;
}

internal’s picture

Overall, the function calls like $(this).parent().find in js are not safe in different themes. By now we just encounter then fix.

ron_s’s picture

I don't believe your suggestion in #3 will work correctly on forms that have multiple Active Tags.

ron_s’s picture

I disagree with your statement in #4. There is no problem with $(this).parent().find. The issue has to do with #autocomplete vs. .dropdown. That is what's causing the problem.

internal’s picture

That's fine. #3 is not a general patch. But I have to use it for the enter key for me. #2 works.

ron_s’s picture

@internal, I'm going to create a clean patch. You can use what you have in #3 if you like, but it will eventually need to be replaced.

internal’s picture

Thanks, @ron_s. I'm also trying to improve it. Here's something about the enter key and wrong hidePopup.
https://www.drupal.org/node/309088

ron_s’s picture

Status: Needs review » Needs work

I have a working solution, but needs to be tested more. Setting this as "needs work" until I can post it.

ron_s’s picture

ron_s’s picture

Title: Use of input in activeTagsAdd behavior causes issues with themes rendering buttons » Support non-core autocomplete widgets
Category: Bug report » Feature request
Issue summary: View changes
Status: Needs work » Active
ron_s’s picture

Status: Active » Needs review
FileSize
2.62 KB

Please review the attached patch.

I'd also recommend reviewing the patch here: https://www.drupal.org/node/2685299, since this is necessary to support multiple Active Tags-enabled fields on a form.

internal’s picture

Tested #13. There is a bug with and without the patch.
Steps:
Type 'a' in autocomplete text field.
If you have 'apple' or 'bad' items in dropdown list, use arrow key to select apple, press enter key.
Bug: The 'a' is added as a new tag first, next enter key adds 'apple' tag.

It seems the first enter key should not be sent to the text field but the dropdown list.

ron_s’s picture

Something must have happened with the patch, because this was working correctly before. I'll have to take a look at it.

ron_s’s picture

FileSize
2.84 KB

New version of the patch. When using the keyboard, the .closest function would not work as coded because it is one level deeper.

This update definitely works, although I do have some concern because it relies on the .form-wrapper class. If a theme strips this class, it's not going to work... however I'm not sure how else the field ID can be accurately captured for all use cases without using it. I've added a "@todo" in the code in case this comes up in the future.

Also there is a new version of this patch that you should update: https://www.drupal.org/node/2685299

internal’s picture

Status: Needs review » Reviewed & tested by the community

Patch #16 works great, thank you very much! Pretty cool module again. Remember to apply that patch first from https://www.drupal.org/node/2626802 if your theme requires jQuery 1.9+.

ron_s’s picture

Don't you mean patch #16? Patch #13 was the old one you said didn't work.

internal’s picture

Edited. Too happy to watch;)