Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly instantiate select2 on newly created django inlines #249

Closed
jurrian opened this issue Nov 16, 2023 · 6 comments
Closed

Correctly instantiate select2 on newly created django inlines #249

jurrian opened this issue Nov 16, 2023 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@jurrian
Copy link

jurrian commented Nov 16, 2023

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: applegrew#65, applegrew#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.

@jurrian jurrian added the bug Something isn't working label Nov 16, 2023
@codingjoe
Copy link
Owner

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
Copy link
Author

jurrian commented Nov 27, 2023

@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
Copy link
Owner

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
Copy link
Author

jurrian commented Nov 28, 2023

@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
Copy link
Owner

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

@jurrian
Copy link
Author

jurrian commented Jan 2, 2024

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.

@jurrian jurrian closed this as not planned Won't fix, can't repro, duplicate, stale Jan 2, 2024
mardukbp added a commit to mardukbp/django-select2 that referenced this issue Sep 11, 2024
codingjoe added a commit that referenced this issue Dec 10, 2024
As described in #249 and #297 django-select2 does not currently work
with selects added dynamically in the Django admin.

The solution was implemented in #249 and all credit should go to Jurrian
Tromp. I only added a small correction:

```diff
- jqRow.find('.select2-container').remove()
+ jqRow.find('.django-select2').parent().find('.select2-container').remove()
```

to only remove `.select2-container` when it is a sibling of
`.django-select2`. Otherwise the wrong `.select2-container` might get
deleted.

---------

Co-authored-by: Johannes Maron <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants