Problem with moving inline admin elements

jrief / django-admin-sortable2

Generic drag-and-drop ordering for objects in the Django admin interface

https://django-admin-sortable2.readthedocs.io/en/latest/

Other

774 stars 180 forks source link

Problem with moving inline admin elements #275

Open rybaczewa opened 3 years ago

rybaczewa commented 3 years ago

Using SortableInlineAdminMixin enables dragging on inline admin elements. Elements don't reorder correctly on drag tough. Depending on the part of the 'drag' element that I click, behavior changes. Element starts dragging, but can't place it behind last or before first. Seems like JS issue with calculating height/position that I wasn't able to identify in the source code. Looks like this: Peek 2021-05-13 16-32

As you can see, Match Photo 6 can't be moved to last from first position. Then Match Photo 7 can be (clicked bottom part of the element this time). Then again Photo 6 to last doesn't work (top part of the element).

I am using Django==3.2.2 and django-admin-sortable2==1.0

rybaczewa commented 3 years ago

OK, it looks like it's 11 year old bug in jQuery-UI: https://bugs.jqueryui.com/ticket/5772

Would change to other library (like SortableJS) be welcome, or it's to much of a change?

marksweb commented 3 years ago

@jrief What are you thoughts on switching to another library to avoid this bug?

jrief commented 3 years ago

And the best feature of SortableJS...

No jQuery required (but there is support)

I'm very favourable in changing that library to SortableJS. Many thanks for making that proposal, I was unaware of that library.

jrief commented 2 years ago

I started to migrate towards SortableJS. The list view is working already. Please try the demo on this working branch: https://github.com/jrief/django-admin-sortable2/tree/Sortable.js

jrief commented 2 years ago

TabularInline and StackedInline now also work with Sortable.js. By refactoring the code, I was able to reuse the internal Django templates, so there no more need to overwrite them. This means that the dragging area moved into the title.

Before releasing a new version of this library, I have to adopt the unit tests. I will use Playwright and replace the tests against end2end-tests. because that is what really matters.

To speed up development, I would be really grateful, if you could checkout the branch https://github.com/jrief/django-admin-sortable2/tree/Sortable.js and test the provided example.

jrief commented 2 years ago

@rybaczewa Please peer review pull request #301

It adds the features you were asking for.

rybaczewa commented 2 years ago

Missed the notification - will try to do it this weekend. Thanks for the pull request!

rybaczewa commented 2 years ago

Ok, I've checked out testapp app on the new branch, few notes:

Example app is missing django in requirements.txt, it would be nice if it installed everything needed on a fresh start
Dragged element scale is a bit off on Firefox (Chromium is close to 1:1, FF is tiny compared to the element)
Admin list element sort seems to work fine.
Inline admin mixin: unless I'm missing something, currently there's no way to see that inline element are draggable. No drag element, no cursor indicator. At first I thought it wasn't working, after few tries I realised it was the element's header.