codingjoe / django-select2

This is a Django integration for Select2
https://django-select2.rtfd.io
MIT License
172 stars 52 forks source link

Correctly instantiate select2 on newly created django inlines #249

Closed jurrian closed 10 months ago

jurrian commented 1 year ago

Describe the bug When a new inline is added using the "Add another ..." button, the Select2 elements on that inline are not working. Something similar has been proposed in an old PR, I've adapted it to the new situation.

If it's appreciated I can create a PR with the change I made.

Other possible related issues: https://github.com/applegrew/django-select2/issues/65, https://github.com/applegrew/django-select2/issues/32

Exception & Traceback Clicking will provide no logs or trace.

Code Snippet I've added this code to the django_select2/django_select2.js file in order to fix it:

$(function () {
    $('.django-select2').djangoSelect2()

    // This part fixes new inlines not having the correct select2 widgets
    function handleFormsetAdded(row, formsetName) {
      // Converting to the "normal jQuery"
      var jqRow = django.jQuery(row)

      // Because select2 was already instantiated on the empty form, we need to remove it, destroy the instance,
      // and re-instantiate it.
      jqRow.find('.select2-container').remove()
      jqRow.find('.django-select2').djangoSelect2('destroy');
      jqRow.find('.django-select2').djangoSelect2()
    }

    // See: https://docs.djangoproject.com/en/dev/ref/contrib/admin/javascript/#supporting-versions-of-django-older-than-4-1
    django.jQuery(document).on('formset:added', function (event, $row, formsetName) {
      if (event.detail && event.detail.formsetName) {
          // Django >= 4.1
          handleFormsetAdded(event.target, event.detail.formsetName)
      } else {
          // Django < 4.1, use $row and formsetName
          handleFormsetAdded($row.get(0), formsetName)
      }
    })
    // End of fix
  })

To Reproduce

  1. Go to a Django page with inlines, the inlines should contain a select with a Select2Widget (not heavy).
  2. Click on "Add another" to add a new inline.
  3. Click on the select widget.
  4. Although there are options, clicking will do nothing.

Expected behavior Normally the dropdown with options appears.

Screenshots image Clicking on the select will do nothing.

codingjoe commented 1 year ago

Hi @jurrian,

Thank you for reaching out. Would you care to elaborate a bit more on some of my questions? Did this recently break, as in change in the latest Django version? At this point, I would discourage the usage of Django-Select2 in Django's admin, with only a few exceptions. Did you try the autocomplete fields? Why aren't they a viable option for your problem?

I am looking forward to hearing back from you.

Cheers! Joe

jurrian commented 12 months ago

@codingjoe I think it has not worked for any Django version, judging on the other issues I mentioned which were opened in 2013. I guess not many ppl are Django-Select2 this way, I don't like the autocomplete fields that are using an endpoint cause it is an extra call, it also requires additional configuration. Since the html <select> already contains all the information and the records are usually <100, the data is not so big. So just having using the Select2Widget on all ForeignKey and choices fields was very convenient to me. I just added this on a base admin:

formfield_overrides = {ForeignKey: {'widget': Select2Widget}}

That worked perfectly out of the box, except for those inlines for which this easy javascript fix works well.

codingjoe commented 12 months ago

Hi @jurrian,

True. We also didn't have official admin support since recently. Would you care to create a patch for this? Preferably in a separate JS file, to keep the frontend payload and complicity small.

Thanks! Joe

jurrian commented 12 months ago

@codingjoe, I can make a PR for this. I can put it in a separate JS file, but I don't see a way to detect if it will be on an admin page or not. So either:

  1. The new JS file is always loaded, in which case it would not make much of a difference to having it in the same file.
  2. There needs to be one or more new mixin classes where the new file will be added to the Media. However, yet another mixin might make it more confusing, especially since the added value for the new mixin is minimal.

Another thought: would adding 20 lines / 940 chars of code to the existing JS file be observable at all in terms of page load?

codingjoe commented 11 months ago

@jurrian there is a fairly newly added admin mixin. The file should only be port of the media.js on that mixin.

jurrian commented 10 months ago

I would be fine with making a PR for the code that I provided in one single file. But to be honest, I am not willing to invest much more time in adding it to a mixin and testing that.

If someone wants to spend time in that, more than welcome. Otherwise the problem is known and fix can be found here for purpose of reference.