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

bug + fix : fallback mode + move in owner container #422

Closed
sp-kilobug opened this issue Jun 10, 2015 · 3 comments
Closed

bug + fix : fallback mode + move in owner container #422

sp-kilobug opened this issue Jun 10, 2015 · 3 comments

Comments

@sp-kilobug
Copy link
Contributor

In fallback mode (no native drag and drop support), a ghost item is appended to the parent container causing the _ghostInBottom() to bug when moving elm in owner container.

This is visible in fallback mode only (set forceFallback to true)
see http://jsbin.com/dolojo/edit?html,css,output

possible fix below

@Addeventure
Copy link

I had a similar problem as you describe, as well having problems moving items between different containers when the items did not have a vertical layout.

There seem to be a bug in _ghostInBottom().

When using touch or when native drag and drop is not supported. In these cases a ghost item is appended to the container.

The problem is that the _ghostInBottom assumes that the lastElementChild is the last draggable item, but it will actually check against the ghost item. Changing the _ghostInBottom to this fixed the problem while still allowing the item to be dragged into another container:

function _ghostInBottom(el, evt) {
        var lastEl = el.lastElementChild,
            rect = lastEl.getBoundingClientRect();

        if (lastEl === ghostEl) {
            return false;
        }

        return (evt.clientY - (rect.top + rect.height) > 5) && lastEl; // min delta
    }

This method has other problems as well which will manifest when trying to do something else than vertical lists when the container is bigger than its contained items.

If the layout is horizontal or like in my example, grid (4xN), the

return (evt.clientY - (rect.top + rect.height) > 5) && lastEl; 

will fail to detect that I am actually hovering the container. It's not an easy to write one that will fit all use-cases. The one that fixed it for me was:

function _ghostInBottom(el, evt) {
                var parentMargin = 16, rect = el.getBoundingClientRect();

                if (evt.clientY > rect.top + parentMargin &&
                        evt.clientX > rect.left + parentMargin &&
                        evt.clientY < rect.bottom - parentMargin &&
                        evt.clientX < rect.right - parentMargin)
                return el.lastElementChild;
    }

This one will have the in-between items margin problem though (which is not an issue in my use case)

Anyways, just wanted to flag for this and propose to expose this method to be configurable :)

@sp-kilobug
Copy link
Contributor Author

Thank you Addeventure, you're right, there is something with the _ghostInBottom related to this bug. Unfortunately, no time to investigate this for the moment

@sp-kilobug sp-kilobug changed the title positioning bug when cursor over parent container bug : positioning when cursor over parent container Jun 18, 2015
@sp-kilobug
Copy link
Contributor Author

based on your idea, I suggest this fix.
If the last element is the ghostEl (fallback mode), it takes the previous element sibling or keep it if no other elements

    function _ghostInBottom(el, evt) {
        var lastEl = el.lastElementChild;
        if (lastEl===ghostEl) {
            lastEl = lastEl.previousElementSibling || ghostEl;
        }
        var rect = lastEl.getBoundingClientRect();
        return (evt.clientY - (rect.top + rect.height) > 5) && lastEl; // min delta
    }

@sp-kilobug sp-kilobug changed the title bug : positioning when cursor over parent container bug + fix : fallback mode + move in owner container Jun 21, 2015
sp-kilobug added a commit to sp-kilobug/Sortable that referenced this issue Jun 26, 2015
sp-kilobug added a commit to sp-kilobug/Sortable that referenced this issue Jun 26, 2015
sp-kilobug added a commit to sp-kilobug/Sortable that referenced this issue Jun 27, 2015
sp-kilobug added a commit to sp-kilobug/Sortable that referenced this issue Jun 27, 2015
SortableJS#422: fix fallback mode moves in owner container
sp-kilobug added a commit to sp-kilobug/Sortable that referenced this issue Jun 27, 2015
sp-kilobug added a commit to sp-kilobug/Sortable that referenced this issue Jun 27, 2015
Revert "SortableJS#422: fix fallback mode moves in owner container"
sp-kilobug added a commit to sp-kilobug/Sortable that referenced this issue Jun 27, 2015
sp-kilobug added a commit to sp-kilobug/Sortable that referenced this issue Jun 27, 2015
sp-kilobug added a commit to sp-kilobug/Sortable that referenced this issue Jun 27, 2015
sp-kilobug added a commit to sp-kilobug/Sortable that referenced this issue Jun 27, 2015
RubaXa added a commit that referenced this issue Sep 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants