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

hoverClass does not work if sortable item contains inner elements #504

Closed
chuen1118 opened this issue May 31, 2019 · 1 comment · Fixed by #730
Closed

hoverClass does not work if sortable item contains inner elements #504

chuen1118 opened this issue May 31, 2019 · 1 comment · Fixed by #730
Labels
bug help wanted info needed More info needs to be provided to discuss/solve the issue PR needed This issue requires a PR to be fixed. If you have time to help, please send a PR. test case needed To validate this issue a jsfiddle or codepen is needed.

Comments

@chuen1118
Copy link

I'm opening this issue because:

I used wrappers to group some elements, and wanted to re-order the wrappers.
css of hoverClass set in config would not show up since most of the time my cursor is on a child element.

It is probably result of this line,

if (item !== event.target) {

the event.target from a child element will not match item

supporting information:

  • I use sortable version: v0.9.16
  • I tested in the following browser (incl. versions): Google Chrome (74.0.3729.169)
  • Windows, OS X/macOS, or Linux?: Windows

How can the issue be reproduce the problem?

Wrap the list content text of the first example provided in a <p>, item no longer changes to maroon when hovered.

<ul class="ml4 js-sortable sortable list flex flex-column list-reset">
    <li class="mb1 navy bg-yellow" style="position: relative; z-index: 10"><p class="mb0 p1">Item 1</p></li>
    <li class="mb1 navy bg-yellow" style="position: relative; z-index: 10"><p class="mb0 p1">Item 2</p></li>
    <li class="mb1 navy bg-yellow" style="position: relative; z-index: 10"><p class="mb0 p1">Item 3</p></li>
    <li class="mb1 navy bg-yellow" style="position: relative; z-index: 10"><p class="mb0 p1">Item 4</p></li>
    <li class="mb1 navy bg-yellow" style="position: relative; z-index: 10"><p class="mb0 p1">Item 5</p></li>
    <li class="mb1 navy bg-yellow" style="position: relative; z-index: 10"><p class="mb0 p1">Item 6</p></li>
</ul>

As a quick fix I modified the html5sortable.js, adding a class to all sortable-list

// property to define as sortable
sortableElement.isSortable = true;
sortableElement.classList.add("isSortable");

then check if the event.target a descendant of item and if they share the same parent sortable-list

if (!item.contains(event.target) || item.closest(".isSortable") !== event.target.closest(".isSortable")) {
    (_a = item.classList).remove.apply(_a, hoverClasses_1);
}

Hope this would be helpful, not sure if this will cause any conflicts.

@lukasoppermann
Copy link
Owner

Hey, if you can add this as a PR with a test and make sure it only runs when a hoverClass is set, I am very happy to merge it. 👍

@lukasoppermann lukasoppermann added bug help wanted info needed More info needs to be provided to discuss/solve the issue PR needed This issue requires a PR to be fixed. If you have time to help, please send a PR. test case needed To validate this issue a jsfiddle or codepen is needed. labels Jul 4, 2020
lukasoppermann pushed a commit that referenced this issue May 19, 2021
In case when sortable elements has child's,
hover classes don't append correctly.

Fixes: #504
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help wanted info needed More info needs to be provided to discuss/solve the issue PR needed This issue requires a PR to be fixed. If you have time to help, please send a PR. test case needed To validate this issue a jsfiddle or codepen is needed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants