?
avatar HLeithner
HLeithner
21 Jul 2021

Steps to reproduce the issue

create a category tree:

  • cat 1
    • cat 1.1
  • cat 2
    • cat 2.1

try to sort cat 2 before cat 1 and reload the list.

Expected result

  • cat 2
    • cat 2.1
  • cat 1
    • cat 1.1

Actual result

  • cat 1
    • cat 1.1
  • cat 2
    • cat 2.1

System information (as much as possible)

4.0-rc5-dev today

Additional comments

I think I already reported this already... if someone confirm this bug I would tag it as release blocker

avatar HLeithner HLeithner - open - 21 Jul 2021
avatar joomla-cms-bot joomla-cms-bot - change - 21 Jul 2021
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - labeled - 21 Jul 2021
avatar joomdonation
joomdonation - comment - 21 Jul 2021

Confirmed. Not sure what happened :(. It seems re-ordering children categories in same parent category does not work now, too :(

avatar joomdonation joomdonation - change - 21 Jul 2021
Labels Added: ?
avatar joomdonation joomdonation - labeled - 21 Jul 2021
avatar ChristineWk
ChristineWk - comment - 21 Jul 2021

@HLeithner
a)
screen shot 2021-07-21 at 09 38 19

b) Drag and Drop works:
screen shot 2021-07-21 at 09 38 56

but after Re-load, same again as shown in a)
Therefore confirmed.

Didn't find reported other, at the moment only this: #31480


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34855.
avatar joomdonation
joomdonation - comment - 21 Jul 2021

Somehow, this feature is broken between my PR #34520 and @Fedik PR #34584.

In the original code (and before my PR), we save the order on drop event. But @Fedik changed the logic to use dragend event and it is not working anymore for some reasons

@Fedik Do you remember why you have to use dragend ? In the original code, we always use drop and everything is working fine until that time.

At this stage, if I change the code to use drop event, it is still working OK.

/**
 * @copyright  (C) 2019 Open Source Matters, Inc. <https://www.joomla.org>
 * @license    GNU General Public License version 2 or later; see LICENSE.txt
 */
// The container where the draggable will be enabled
let url;
let direction;
let isNested;
let dragElementIndex;
let dropElementIndex;
let dropElement;
let container = document.querySelector('.js-draggable');
const orderRows = container.querySelectorAll('[name="order[]"]');

if (container) {
    /** The script expects a form with a class js-form
     *  A table with the tbody with a class js-draggable
     *                         with a data-url with the ajax request end point and
     *                         with a data-direction for asc/desc
     */
    url = container.dataset.url;
    direction = container.dataset.direction;
    isNested = container.dataset.nested;
} else if (Joomla.getOptions('draggable-list')) {
    const options = Joomla.getOptions('draggable-list');
    container = document.querySelector(options.id);
    /**
     * This is here to make the transition to new forms easier.
     */

    if (!container.classList.contains('js-draggable')) {
        container.classList.add('js-draggable');
    }

    ({
        url
    } = options);
    ({
        direction
    } = options);
    isNested = options.nested;
}

if (container) {
    // Add data order attribute for initial ordering
    for (let i = 0, l = orderRows.length; l > i; i += 1) {
        orderRows[i].dataset.order = i + 1;
    } // IOS 10 BUG


    document.addEventListener('touchstart', () => {}, false);

    const getOrderData = (rows, inputRows, dragIndex, dropIndex) => {
        let i;
        const result = []; // Element is moved down

        if (dragIndex < dropIndex) {
            rows[dropIndex].setAttribute('value', rows[dropIndex - 1].value); // Move down

            for (i = dragIndex; i < dropIndex; i += 1) {
                if (direction === 'asc') {
                    rows[i].setAttribute('value', parseInt(rows[i].value, 10) - 1);
                } else {
                    rows[i].setAttribute('value', parseInt(rows[i].value, 10) + 1);
                }
            }
        } else {
            // Element is moved up
            rows[dropIndex].setAttribute('value', rows[dropIndex + 1].value);
            rows[dropIndex].value = rows[dropIndex + 1].value;

            for (i = dropIndex + 1; i <= dragIndex; i += 1) {
                if (direction === 'asc') {
                    rows[i].value = parseInt(rows[i].value, 10) + 1;
                } else {
                    rows[i].value = parseInt(rows[i].value, 10) - 1;
                }
            }
        }

        for (i = 0; i < rows.length - 1; i += 1) {
            result.push(`order[]=${encodeURIComponent(rows[i].value)}`);
            result.push(`cid[]=${encodeURIComponent(inputRows[i].value)}`);
        }

        return result;
    };

    const rearrangeChildren = $parent => {
        if (!$parent.dataset.itemId) {
            return;
        }

        const parentId = $parent.dataset.itemId; // Get children list. Each child row should have
        // an attribute data-parents=" 1 2 3" where the number is id of parent

        const $children = container.querySelectorAll(`tr[data-parents~="${parentId}"]`);

        if ($children.length) {
            $parent.after(...$children);
        }
    };

    const saveTheOrder = el => {
        let orderSelector;
        let inputSelector;
        let rowSelector;
        const groupId = el.dataset.draggableGroup;

        if (groupId) {
            rowSelector = `tr[data-draggable-group="${groupId}"]`;
            orderSelector = `[data-draggable-group="${groupId}"] [name="order[]"]`;
            inputSelector = `[data-draggable-group="${groupId}"] [name="cid[]"]`;
        } else {
            rowSelector = 'tr';
            orderSelector = '[name="order[]"]';
            inputSelector = '[name="cid[]"]';
        }

        const rowElements = [].slice.call(container.querySelectorAll(rowSelector));
        const rows = [].slice.call(container.querySelectorAll(orderSelector));
        const inputRows = [].slice.call(container.querySelectorAll(inputSelector));
        dropElementIndex = rowElements.indexOf(el);

        if (url) {
            // Detach task field if exists
            const task = document.querySelector('[name="task"]'); // Detach task field if exists

            if (task) {
                task.setAttribute('name', 'some__Temporary__Name__');
            } // Prepare the options


            const ajaxOptions = {
                url,
                method: 'POST',
                data: getOrderData(rows, inputRows, dragElementIndex, dropElementIndex).join('&'),
                perform: true
            };
            Joomla.request(ajaxOptions); // Re-Append original task field

            if (task) {
                task.setAttribute('name', 'task');
            }
        } // Update positions for a children of the moved item


        rearrangeChildren(el); // Reset data order attribute for initial ordering
    }; // eslint-disable-next-line no-undef


    dragula([container], {
        // Y axis is considered when determining where an element would be dropped
        direction: 'vertical',
        // elements are moved by default, not copied
        copy: false,
        // elements in copy-source containers can be reordered
        // copySortSource: true,
        // spilling will put the element back where it was dragged from, if this is true
        revertOnSpill: true,

        // spilling will `.remove` the element, if this is true
        // removeOnSpill: false,
        accepts(el, target, source, sibling) {
            if (isNested) {
                if (sibling !== null) {
                    return sibling.dataset.draggableGroup && sibling.dataset.draggableGroup === el.dataset.draggableGroup;
                }

                return sibling === null || sibling && sibling.tagName.toLowerCase() === 'tr';
            }

            return sibling === null || sibling && sibling.tagName.toLowerCase() === 'tr';
        },

        mirrorContainer: container
    }).on('drag', el => {
        let rowSelector;
        const groupId = el.dataset.draggableGroup;

        if (groupId) {
            rowSelector = `tr[data-draggable-group="${groupId}"]`;
        } else {
            rowSelector = 'tr';
        }

        const rowElements = [].slice.call(container.querySelectorAll(rowSelector));
        dragElementIndex = rowElements.indexOf(el);
    }).on('drop', el => {
        saveTheOrder(el);
    }).on('dragend', () => {
        const elements = container.querySelectorAll('[name="order[]"]');

        for (let i = 0, l = elements.length; l > i; i += 1) {
            elements[i].dataset.order = i + 1;
        }
    });
}

Could you please look at it again @Fedik ?

PS: Does anyone knows why we need to have code to initialize and reset data-order for each row? I don't think we need it anymore. Ping @dgrammatiko

avatar dgrammatiko
dgrammatiko - comment - 21 Jul 2021

The dragend I think doesn't have the reference for the moved element (it's already dropped, this event is for cleanup work) thus the drop was used in the first place.

avatar dgrammatiko
dgrammatiko - comment - 21 Jul 2021

Does anyone knows why we need to have code to initialize and reset data-order for each row? I don't think we need it anymore. Ping @dgrammatiko

I think the idea was to keep the state reflected in the Dom elements but since we are querying the elements in the respective should be redundant now (I might be wrong here without debugging)

avatar Fedik
Fedik - comment - 21 Jul 2021

hm, strange

Do you remember why you have to use dragend ?

I just moved all in one saveTheOrder method to keep all in one place,

the dropElement is pre-saved:

.on('drop', (el) => {
dropElement = el;
})
.on('dragend', () => {
if (dropElement) {
saveTheOrder(dropElement);
dropElement = null;
}
});

Need to look

avatar joomdonation joomdonation - close - 22 Jul 2021
avatar joomdonation
joomdonation - comment - 22 Jul 2021

I think the idea was to keep the state reflected in the Dom elements but since we are querying the elements in the respective should be redundant now (I might be wrong here without debugging)

@dgrammatiko We might use it in the past (I remember there was a time we set ordering value for each item base on it's current position in the current list, but that was wrong). It is not used now, so I removed these code

@Fedik I changed the call back to drop because it works and that is how it was before :D. We do not need to reset data-order now, so it is still All In One Place :)

PR is #34864 . I'm closing this issue.

avatar joomdonation joomdonation - change - 22 Jul 2021
Status New Closed
Closed_Date 0000-00-00 00:00:00 2021-07-22 09:30:36
Closed_By joomdonation
Labels Added: ?
Removed: ?
avatar richard67 richard67 - change - 22 Jul 2021
Labels Removed: ?
avatar richard67 richard67 - unlabeled - 22 Jul 2021

Add a Comment

Login with GitHub to post a comment