radiac / django-tagulous

Fabulous Tagging for Django
http://radiac.net/projects/django-tagulous/
Other
336 stars 66 forks source link

Strange behavior while editing existing tags #148

Closed BoPeng closed 1 month ago

BoPeng commented 2 years ago

Using the example project, add S1 and S2 to a Person, then edit the Person as follows:

image

Remove two tags

image

Add s1 back,

image

Then both s1 and s2` are added

image

bernhardmiller commented 2 years ago

I think i this is a problem with the select2 adapter. https://github.com/radiac/django-tagulous/blob/90c6ee5ac54ef1127f2edcfac10c5ef3ab09e255/tagulous/static/tagulous/adaptor/select2-4.js#L254-L268 This looks to me like it is designed for AJAX select2 sources (see https://select2.org/programmatic-control/add-select-clear-items#preselecting-options-in-an-remotely-sourced-ajax-select2).

So when the options are provided via the data-tag-list option, tagulous ends up adding the selected tags again to the list of options (e.g. if you have "1" and "2" as possible options and "1" is selected, you will end up with an action list like ["1" (unselected), "2" (unselected), "1" (selected)]. This produces all kinds of weird behavior in select2.

The following code could be a fix; but I don't understand all the select2 and tagulous use cases enough to be sure it does not break anything else...

        if (selectedTags.length > 0) {
            if (url) {
                var selectedData = [];
                for (var i = 0; i < selectedTags.length; i++) {

                    var option = new Option(selectedTags[i], selectedTags[0], true, true);
                    $selectCtl.append(option).trigger('change');
                    selectedData.push({id: selectedTags[i], text: selectedTags[i]});
                }

                $selectCtl.trigger({
                    type: "select2:select",
                    params: {
                        data: selectedData,
                    }
                });
            } else {
                $selectCtl.val(selectedTags);
                $selectCtl.trigger('change');
            }
        }

@radiac : Please let me know if you think this fix is appropriate, then I can create a pull request.

twang817 commented 2 years ago

I think i this is a problem with the select2 adapter.

Just wanted to follow up that this solved all sorts of issues with select2 for me. Stepping through the code in a debugger, it seems that the following code adds all the options to the select2 control already:

https://github.com/radiac/django-tagulous/blob/90c6ee5ac54ef1127f2edcfac10c5ef3ab09e255/tagulous/static/tagulous/adaptor/select2-4.js#L205-L208

I'm not sure why it then continues to add selected values again -- but it does produce duplicate values in the select2 list. The change above solves that.

I found that to get the clear button working, I had to make one more minor change:

        $.extend(args, {
            // Select2 options
            tokenizer: tokenizer,
            tags: true,

            // Things defined by field/tag options, which can't be overridden
            multiple: !isSingle,
            quotedTags: true,
            allowClear: !options.required,
            maximumSelectionLength: isSingle ? null : options.max_count || null,
            spaceDelimiter: options.space_delimiter !== false,
            placeholder: options.placeholder || '',
        });

Above, I added an empty placeholder, which allows the clear button to remove all the tags.