-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Sortable Display Grid not working in 1.3 #1998
Comments
Thanks for the report. That's a pretty substantial bug, indeed. |
I can take a look this evening. My suspect is that it's something very trivial. |
FYI: If you change the CSS on the ul in the demo to {display;flex;flex-flow;row wrap;} it works just fine. Seems to have something to do with the floating perhaps. |
Thanks, @borgboyone! Here's what I've already found: this line in Since there was no requirement in 1.12 to have the parent encompass its children, it'd be good to preserve that. Note that in 1.12 Hopefully you can find a solution out of that info. Thanks! |
I've looked over the issue. Obviously containers that have zero height prevent the block encompassing L427 to kick in since the evaluation of _contactContainers indicates the dragged element is outside of containers. This is true for both 1.12.1 and 1.13.0. However, the logic starting at L247 runs regardless in 1.12.1. The intent was to prevent unnecessary processing of the intersection logic when outside of containers. The zero height for containers wasn't expected as a feasible option. Attempting to get the actual visible client height of such containers is highly problematic in itself (not to mention code cluttering). In order to maintain backward compatibility, my recommendation is to remove L427 and L495 and move L424 and L425 to L496. Regardless, be aware that container specific events will not fire when the dragged element is interacting with zero height containers. Thoughts? |
@borgboyone Thanks for your analysis. I understand the case is a bit specific here but I expect this to be a common problem considering that even the jQuery UI website used a pattern that broke and it did so in one of the few demos it has for the Sortable widget. So maintaining backwards compatibility would be great here. Would you like to submit a PR with a fix? |
@mgol - Thank you, but I'm going to pass. I'm sorry that my contribution caused this issue. |
No problem. Thank you for your input into this issue! |
Note that container specific events will not fire when the dragged element is interacting with zero height containers. Fixes jquerygh-1998
Note that container specific events will not fire when the dragged element is interacting with zero height containers. Fixes jquerygh-1998 Co-Authored-By: A. Wells <borgboyone@users.noreply.github.com>
PR: #2008. @borgboyone I'd appreaciate a quick review from you, at least of the source changes (I also added a test) if you have time. I also listed you as a co-author of the PR since source changes have been suggested by you. I hope you're fine with it. |
jQuery UI 1.13.1 including a fix for this issue has been released: https://blog.jqueryui.com/2022/01/jquery-ui-1-13-1-released/. |
https://jqueryui.com/sortable/#display-grid
No longer drops into place.
The text was updated successfully, but these errors were encountered: