Skip to content

Commit

Permalink
Correctly position cloned sortable element on mobile
Browse files Browse the repository at this point in the history 
Added a helper function to the jQuery UI Sortable call to fix the positioning of the cloned sortable element when dragging on mobile.

Tested on latest Chrome/Safari on iOS 11.3. May need testing on Android devices to confirm the fix.
  • Loading branch information
@brettshumaker
brettshumaker committed May 1, 2018
1 parent df0ffc9 commit bfe91df
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions js/zoninator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ var zoninator = {};
, placeholder : 'ui-state-highlight'
, forcePlaceholderSize: true
//, handle: '.zone-post-handle'
// This helper function addresses #41 (Drag and Drop on Mobile Issue) and corrects the placement of the cloned sortable item on Chrome/Safari iOS
, helper : function(event, ui) {
var clone = $(ui).clone();
clone.css('position','absolute');
return clone.get(0);
}
});
}

Expand Down

5 comments on commit bfe91df

@sboisvert
Copy link
Contributor

Choose a reason for hiding this comment

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

One small piece of feedback, I'd reference the issue in the commit message to make it easier to link back to it. The fix looks good to me.

Do you have any ideas on why this might of changed?

@sboisvert
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably also want to get confirmation on Android, although I guess the initial bug report doesn't mention it being broken there.

@brettshumaker
Copy link
Contributor Author

@brettshumaker brettshumaker commented on bfe91df May 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@sboisvert

I'd reference the issue in the commit message to make it easier to link back to it.

Noted. 👍

Do you have any ideas on why this might of changed?

I originally thought there may have been a new version of touch punch that had a fix. There is a new version, but it doesn't fix this issue. The last commit for jquery-ui-touch-punch was in December of 2014 so I'm guessing it's abandoned unless someone has forked and maintained it. Since the latest jquery-ui-touch-punch didn't solve the issue, I'm would be that WordPress updated the jquery-ui-sortable version and it no longer interacted with jquery-ui-touch-punch the same way. This would need to be investigated, though.

We probably also want to get confirmation on Android

Agreed. I don't have an Android device to check on, but I'll see if I have a free trial left on BrowserStack.

@brettshumaker
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sboisvert - Checked this out on a physical Android device through BrowserStack and Chrome did not show the same behavior as iOS. Also functions correctly with this patch in place.

Do you see any blockers to making a PR with this commit?

@sboisvert
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM, let's PR it!

Please sign in to comment.