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] Sortable swapping does not work when forceFallback is enabled and a handle or content is a custom element (web component) with a shadowRoot #2346

Closed
driskull opened this issue Feb 14, 2024 · 5 comments

Comments

@driskull
Copy link
Contributor

driskull commented Feb 14, 2024

Describe the bug

  • When in forceFallback: true, and the handle or list content is a web component with an open shadowRoot items cannot be dragged.
  • In our usage, our handle element is another custom element with shadowRoot and the content within a sortable element is also a custom element.
  • The code problem area is seen in this section:

    Sortable/src/Sortable.js

    Lines 756 to 759 in 07708fa

    while (target && target.shadowRoot) {
    target = target.shadowRoot.elementFromPoint(touchEvt.clientX, touchEvt.clientY);
    if (target === parent) break;
    parent = target;
  • With this line target = target.shadowRoot.elementFromPoint(touchEvt.clientX, touchEvt.clientY);, its returning elements within our handle custom element or content custom element instead of a child of the Sortable instance.
  • This only happens when in forceFallback: true or when used by a mobile device which seems to always go into fallback mode.
  • When using native drag and drop the issue is not seen.

To Reproduce
Steps to reproduce the behavior:

  1. Go to https://codepen.io/driskull/pen/OJqrzVL?editors=1010
  2. Try to drag a handle over another handle and notice it won't swap elements
  3. Drag a handle over a list element textContent and notice it does swap
  4. If there are other custom-elements inside the list-element in place of the text content elements won't swap either.
  5. See sample: https://codepen.io/driskull/pen/xxBmpra?editors=0110

Expected behavior

I would expect sort swapping to work when a handle or content within a list is a custom element.

Information

sortablejs = ^1.15.1
@types/sortablejs = ^1.15.1

Additional context
I have a PR open to fix this but need verification that it is the best approach to fixing the issue.

Reproduction
codesandbox: https://codepen.io/driskull/pen/OJqrzVL?editors=1010 and https://codepen.io/driskull/pen/xxBmpra?editors=0110

Pull Request Fix

#2347

driskull added a commit to driskull/Sortable-shadow-host-fix that referenced this issue Feb 14, 2024
driskull added a commit to Esri/calcite-design-system that referenced this issue Feb 15, 2024
**Related Issue:** #8728

## Summary

- Fixes dragging when on a mobile device.
- Sortablejs uses fallback code instead of native drag and drop on
mobile devices.
- Patches `sortablejs` dependency until the fix for the issue is
installed and released.
SortableJS/Sortable#2346
- Adds and uses the dependency `patch-package` to handle patching the
dependency.
- Applies the changes listed here:
SortableJS/Sortable#2347

---------

Co-authored-by: JC Franco <[email protected]>
Elijbet pushed a commit to Esri/calcite-design-system that referenced this issue Feb 15, 2024
**Related Issue:** #8728

## Summary

- Fixes dragging when on a mobile device.
- Sortablejs uses fallback code instead of native drag and drop on
mobile devices.
- Patches `sortablejs` dependency until the fix for the issue is
installed and released.
SortableJS/Sortable#2346
- Adds and uses the dependency `patch-package` to handle patching the
dependency.
- Applies the changes listed here:
SortableJS/Sortable#2347

---------

Co-authored-by: JC Franco <[email protected]>
@owen-m1
Copy link
Member

owen-m1 commented Mar 22, 2024

Thank you, PR has been merged

@owen-m1 owen-m1 closed this as completed Mar 22, 2024
@driskull
Copy link
Contributor Author

Thank you @owen-m1 🍻

@jcfranco
Copy link

jcfranco commented Apr 5, 2024

Thanks, @driskull and @owen-m1! When is the next SortableJS release planned?

@driskull
Copy link
Contributor Author

@owen-m1 any chance we can get a new release with this fix?

driskull added a commit to Esri/calcite-design-system that referenced this issue Jun 21, 2024
**Related Issue:** #9521

## Summary

- patch sortablejs with the correct update to fix dragging shadow
elements on mobile devices
- SortableJS/Sortable#2347
SortableJS/Sortable#2346
@owen-m1
Copy link
Member

owen-m1 commented Sep 1, 2024

Thank you @driskull it has been released in 1.15.3

newkdr added a commit to newkdr/vscode-gitlens that referenced this issue Dec 12, 2024
![snyk-top-banner](https://redirect.github.com/andygongea/OWASP-Benchmark/assets/818805/c518c423-16fe-447e-b67f-ad5a49b5d123)


<h3>Snyk has created this PR to upgrade sortablejs from 1.15.0 to
1.15.3.</h3>

:information_source: Keep your dependencies up-to-date. This makes it
easier to fix existing vulnerabilities and to more quickly identify and
fix newly disclosed vulnerabilities when they affect your project.

<hr/>


- The recommended version is **3 versions** ahead of your current
version.

- The recommended version was released on **3 months ago**.



<details>
<summary><b>Release notes</b></summary>
<br/>
  <details>
    <summary>Package name: <b>sortablejs</b></summary>
    <ul>
      <li>
<b>1.15.3</b> - <a
href="https://redirect.github.com/SortableJS/Sortable/releases/tag/1.15.3">2024-09-01</a></br><ul>
<li>Expose <code>expando</code> value in
<code>Sortable.utils</code></li>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load
title" data-id="2134996369" data-permission-text="Title is private"
data-url="SortableJS/Sortable#2346"
data-hovercard-type="issue"
data-hovercard-url="/SortableJS/Sortable/issues/2346/hovercard"
href="https://redirect.github.com/SortableJS/Sortable/issues/2346">#2346</a>:
Fix inability to drag in fallback when handle has a shadowRoot</li>
<li>Include src folder in NPM package</li>
</ul>
      </li>
      <li>
<b>1.15.2</b> - <a
href="https://redirect.github.com/SortableJS/Sortable/releases/tag/1.15.2">2024-01-14</a></br><ul>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load
title" data-id="2053450032" data-permission-text="Title is private"
data-url="SortableJS/Sortable#2339"
data-hovercard-type="issue"
data-hovercard-url="/SortableJS/Sortable/issues/2339/hovercard"
href="https://redirect.github.com/SortableJS/Sortable/issues/2339">#2339</a>:
Fixes 1.15.1 regression with not being able to add element into start
&amp; end of list.</li>
</ul>
      </li>
      <li>
<b>1.15.1</b> - <a
href="https://redirect.github.com/SortableJS/Sortable/releases/tag/1.15.1">2023-11-30</a></br><ul>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load
title" data-id="1370365710" data-permission-text="Title is private"
data-url="SortableJS/Sortable#2203"
data-hovercard-type="issue"
data-hovercard-url="/SortableJS/Sortable/issues/2203/hovercard"
href="https://redirect.github.com/SortableJS/Sortable/issues/2203">#2203</a>:
Fix multi drag sort event not firing</li>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load
title" data-id="1656925237" data-permission-text="Title is private"
data-url="SortableJS/Sortable#2263"
data-hovercard-type="pull_request"
data-hovercard-url="/SortableJS/Sortable/pull/2263/hovercard"
href="https://redirect.github.com/SortableJS/Sortable/pull/2263">#2263</a>:
Only call onDrop on destroy if dragged element inside parent
element</li>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load
title" data-id="523964543" data-permission-text="Title is private"
data-url="SortableJS/Sortable#1686"
data-hovercard-type="issue"
data-hovercard-url="/SortableJS/Sortable/issues/1686/hovercard"
href="https://redirect.github.com/SortableJS/Sortable/issues/1686">#1686</a>:
Prevent drag item from jumping to end of list if last element has
smaller width/height</li>
</ul>
      </li>
      <li>
<b>1.15.0</b> - <a
href="https://redirect.github.com/SortableJS/Sortable/releases/tag/1.15.0">2022-03-20</a></br><ul>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load
title" data-id="977152901" data-permission-text="Title is private"
data-url="SortableJS/Sortable#2072"
data-hovercard-type="pull_request"
data-hovercard-url="/SortableJS/Sortable/pull/2072/hovercard"
href="https://redirect.github.com/SortableJS/Sortable/pull/2072">#2072</a>:
Make sure dragged element is inserted after last dragged element</li>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load
title" data-id="1008208683" data-permission-text="Title is private"
data-url="SortableJS/Sortable#2084"
data-hovercard-type="pull_request"
data-hovercard-url="/SortableJS/Sortable/pull/2084/hovercard"
href="https://redirect.github.com/SortableJS/Sortable/pull/2084">#2084</a>:
Added <code>avoidImplicitDeselect</code> option to MultiDrag</li>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load
title" data-id="1030640442" data-permission-text="Title is private"
data-url="SortableJS/Sortable#2093"
data-hovercard-type="issue"
data-hovercard-url="/SortableJS/Sortable/issues/2093/hovercard"
href="https://redirect.github.com/SortableJS/Sortable/issues/2093">#2093</a>:
Remove ID from cloned element</li>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load
title" data-id="1030824960" data-permission-text="Title is private"
data-url="SortableJS/Sortable#2095"
data-hovercard-type="pull_request"
data-hovercard-url="/SortableJS/Sortable/pull/2095/hovercard"
href="https://redirect.github.com/SortableJS/Sortable/pull/2095">#2095</a>:
Remove ignoring click on Chrome for Android when dragging (wasn't
necessary)</li>
</ul>
      </li>
    </ul>
from <a
href="https://redirect.github.com/SortableJS/Sortable/releases">sortablejs
GitHub release notes</a>
  </details>
</details>

---

> [!IMPORTANT]
>
> - Check the changes in this PR to ensure they won't cause issues with
your project.
> - This PR was automatically created by Snyk using the credentials of a
real user.

---

**Note:** _You are seeing this because you or someone else with access
to this repository has authorized Snyk to open upgrade PRs._

**For more information:** <img
src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiIyZjBhYTViMC1jODJiLTRhZjUtODU3Zi0zYWQxOTIxOWZlMDAiLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6IjJmMGFhNWIwLWM4MmItNGFmNS04NTdmLTNhZDE5MjE5ZmUwMCJ9fQ=="
width="0" height="0"/>

> - 🧐 [View latest project
report](https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17?utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr)
> - 📜 [Customise PR
templates](https://docs.snyk.io/scan-using-snyk/pull-requests/snyk-fix-pull-or-merge-requests/customize-pr-templates)
> - 🛠 [Adjust upgrade PR
settings](https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17/settings/integration?utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr)
> - 🔕 [Ignore this dependency or unsubscribe from future upgrade
PRs](https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17/settings/integration?pkg&#x3D;sortablejs&amp;utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr#auto-dep-upgrades)

[//]: #
'snyk:metadata:{"customTemplate":{"variablesUsed":[],"fieldsUsed":[]},"dependencies":[{"name":"sortablejs","from":"1.15.0","to":"1.15.3"}],"env":"prod","hasFixes":false,"isBreakingChange":false,"isMajorUpgrade":false,"issuesToFix":[],"prId":"2f0aa5b0-c82b-4af5-857f-3ad19219fe00","prPublicId":"2f0aa5b0-c82b-4af5-857f-3ad19219fe00","packageManager":"npm","priorityScoreList":[],"projectPublicId":"12a8a5f5-3e19-438c-8280-eb8f4ee06d17","projectUrl":"https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17?utm_source=github&utm_medium=referral&page=upgrade-pr","prType":"upgrade","templateFieldSources":{"branchName":"default","commitMessage":"default","description":"default","title":"default"},"templateVariants":[],"type":"auto","upgrade":[],"upgradeInfo":{"versionsDiff":3,"publishedDate":"2024-09-01T22:55:30.201Z"},"vulns":[]}'
newkdr added a commit to newkdr/vscode-gitlens that referenced this issue Dec 24, 2024
![snyk-top-banner](https://redirect.github.com/andygongea/OWASP-Benchmark/assets/818805/c518c423-16fe-447e-b67f-ad5a49b5d123)


<h3>Snyk has created this PR to upgrade sortablejs from 1.15.3 to
1.15.6.</h3>

:information_source: Keep your dependencies up-to-date. This makes it
easier to fix existing vulnerabilities and to more quickly identify and
fix newly disclosed vulnerabilities when they affect your project.

<hr/>


- The recommended version is **3 versions** ahead of your current
version.

- The recommended version was released **25 days ago**.



<details>
<summary><b>Release notes</b></summary>
<br/>
  <details>
    <summary>Package name: <b>sortablejs</b></summary>
    <ul>
      <li>
<b>1.15.6</b> - <a
href="https://redirect.github.com/SortableJS/Sortable/releases/tag/1.15.6">2024-11-28</a></br><ul>
<li>Restore pervious text clearing selection behaviour on
fallback-enabled sortable lists. Text should be cleared whenever mouse
is down on a sortable item, but text inputs within items should still be
interactable</li>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load
title" data-id="1521940212" data-permission-text="Title is private"
data-url="SortableJS/Sortable#2244"
data-hovercard-type="issue"
data-hovercard-url="/SortableJS/Sortable/issues/2244/hovercard"
href="https://redirect.github.com/SortableJS/Sortable/issues/2244">#2244</a>:
Fix issue where multi-drag selection doesn't work on IOS devices</li>
</ul>
      </li>
      <li>
<b>1.15.5</b> - <a
href="https://redirect.github.com/SortableJS/Sortable/releases/tag/1.15.5">2024-11-27</a></br><ul>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load
title" data-id="2692375294" data-permission-text="Title is private"
data-url="SortableJS/Sortable#2410"
data-hovercard-type="issue"
data-hovercard-url="/SortableJS/Sortable/issues/2410/hovercard"
href="https://redirect.github.com/SortableJS/Sortable/issues/2410">#2410</a>:
Further improvements to handling of text selection during drag, to allow
text inputs to continue to function within sortable items. Removed the
previous <code>nextTick</code> solution which caused text inputs to be
noninteractive, and instead have moved the text selection clearing to
the actual drag start.</li>
</ul>
      </li>
      <li>
<b>1.15.4</b> - <a
href="https://redirect.github.com/SortableJS/Sortable/releases/tag/1.15.4">2024-11-24</a></br><ul>
<li>Fix issue where text selection occurs during dragging when fallback
is enabled</li>
<li>Improved support for pointer events</li>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load
title" data-id="692108553" data-permission-text="Title is private"
data-url="SortableJS/Sortable#1897"
data-hovercard-type="issue"
data-hovercard-url="/SortableJS/Sortable/issues/1897/hovercard"
href="https://redirect.github.com/SortableJS/Sortable/issues/1897">#1897</a>:
[MultiDrag] Prevent multi-select of non-draggable and filtered
items</li>
</ul>
      </li>
      <li>
<b>1.15.3</b> - <a
href="https://redirect.github.com/SortableJS/Sortable/releases/tag/1.15.3">2024-09-01</a></br><ul>
<li>Expose <code>expando</code> value in
<code>Sortable.utils</code></li>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load
title" data-id="2134996369" data-permission-text="Title is private"
data-url="SortableJS/Sortable#2346"
data-hovercard-type="issue"
data-hovercard-url="/SortableJS/Sortable/issues/2346/hovercard"
href="https://redirect.github.com/SortableJS/Sortable/issues/2346">#2346</a>:
Fix inability to drag in fallback when handle has a shadowRoot</li>
<li>Include src folder in NPM package</li>
</ul>
      </li>
    </ul>
from <a
href="https://redirect.github.com/SortableJS/Sortable/releases">sortablejs
GitHub release notes</a>
  </details>
</details>

---

> [!IMPORTANT]
>
> - Check the changes in this PR to ensure they won't cause issues with
your project.
> - This PR was automatically created by Snyk using the credentials of a
real user.

---

**Note:** _You are seeing this because you or someone else with access
to this repository has authorized Snyk to open upgrade PRs._

**For more information:** <img
src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiJhMzBiYmI0ZS1iMmY4LTQ3NmQtYjM2Ny01ZWQ1MDYxODBlMzYiLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6ImEzMGJiYjRlLWIyZjgtNDc2ZC1iMzY3LTVlZDUwNjE4MGUzNiJ9fQ=="
width="0" height="0"/>

> - 🧐 [View latest project
report](https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17?utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr)
> - 📜 [Customise PR
templates](https://docs.snyk.io/scan-using-snyk/pull-requests/snyk-fix-pull-or-merge-requests/customize-pr-templates?utm_source=&utm_content=fix-pr-template)
> - 🛠 [Adjust upgrade PR
settings](https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17/settings/integration?utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr)
> - 🔕 [Ignore this dependency or unsubscribe from future upgrade
PRs](https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17/settings/integration?pkg&#x3D;sortablejs&amp;utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr#auto-dep-upgrades)

[//]: #
'snyk:metadata:{"customTemplate":{"variablesUsed":[],"fieldsUsed":[]},"dependencies":[{"name":"sortablejs","from":"1.15.3","to":"1.15.6"}],"env":"prod","hasFixes":false,"isBreakingChange":false,"isMajorUpgrade":false,"issuesToFix":[],"prId":"a30bbb4e-b2f8-476d-b367-5ed506180e36","prPublicId":"a30bbb4e-b2f8-476d-b367-5ed506180e36","packageManager":"npm","priorityScoreList":[],"projectPublicId":"12a8a5f5-3e19-438c-8280-eb8f4ee06d17","projectUrl":"https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17?utm_source=github&utm_medium=referral&page=upgrade-pr","prType":"upgrade","templateFieldSources":{"branchName":"default","commitMessage":"default","description":"default","title":"default"},"templateVariants":[],"type":"auto","upgrade":[],"upgradeInfo":{"versionsDiff":3,"publishedDate":"2024-11-28T18:21:49.918Z"},"vulns":[]}'
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

3 participants