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

fix(list): fix mobile device dragging with nested lists #9573

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

driskull
Copy link
Member

@driskull driskull commented Jun 13, 2024

Related Issue: #9521

Summary

  • Makes sure that connectSortableComponent isn't called during an active drag of a sortable item.
    • Previously, mutation observers were calling connectSortableComponent during a drag
    • This caused the existing implementation to be destroyed and a new one created which left the user in a frozen state.
    • This only occurred on mobile devices because mobile devices do not use native drag and drop with Sortable.js
  • The fix makes sure that connectSortableComponent and disconnectSortableComponent do nothing during an active drag.

@github-actions github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Jun 13, 2024
@driskull driskull marked this pull request as ready for review June 13, 2024 04:12
@driskull driskull requested a review from a team as a code owner June 13, 2024 04:12
@driskull driskull requested a review from jcfranco June 13, 2024 04:12
@driskull driskull added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Jun 13, 2024
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM!

Would current tests cover these changes or is there mobile-specific behavior in SortableJS that can't be tested? Possibly related, are we waiting for #8297 to improve/add coverage for this?

@jcfranco jcfranco changed the title fix(list): fixes mobile device dragging with nested lists fix(list): fix mobile device dragging with nested lists Jun 14, 2024
@driskull
Copy link
Member Author

Would current tests cover these changes or is there mobile-specific behavior in SortableJS that can't be tested? Possibly related, are we waiting for #8297 to improve/add coverage for this?

Yeah we can't currently test it because we need mobile testing of sortablejs. We would need to do some sort of browser identification spoofing alongside the automated test.

@driskull driskull merged commit 6f466aa into dev Jun 17, 2024
12 checks passed
@driskull driskull deleted the dris0000/drag-fix-sortable branch June 17, 2024 18:27
@github-actions github-actions bot added this to the 2024-06-25 - Jun Release milestone Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug reports for broken functionality. Issues should include a reproduction of the bug. pr ready for visual snapshots Adding this label will run visual snapshot testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants