-
Notifications
You must be signed in to change notification settings - Fork 986
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
IE 11 fixes for react-sortable-hoc #248
Conversation
… animationNode functions as fixes for IE 11
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @humiston, thanks for taking the time to submit this!
I think there might be an oversight in the code though, I don't think your proposed solution would work the same as the existing code.
Would you mind updating your PR to address this feedback? 😄
src/SortableContainer/index.js
Outdated
translate.y -= (window.scrollY - this.initialWindowScroll.top); | ||
translate.x -= (window.scrollX - this.initialWindowScroll.left); | ||
translate.y -= (window.scrollY - this.initialWindowScroll.top) || window.pageYOffset; | ||
translate.x -= (window.scrollX - this.initialWindowScroll.left) || window.pageXOffset; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks to me like those two statements are not equivalent.
The first one will take into account the diff between the current scrollY
and scrollX
values vs the initial scroll value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated this pr with the correct assignment of our initial scroll values so our diff is accurate when using IE 11.
…rmine the diff between the initial and current scroll values
All of the browsers that support scrollX/Y already support pageXOffset and pageYOffset
Just published |
@clauderic @humiston I'm running storybook with this fix, and I still see the issue with IE11. Both in my code and in your Drag Handle storybook example. Items aren't draggable at all. |
It's because IE11 doesn't support Array.From() which is being used in the code. I just ran into this myself. Edit: Using a polyfill fixed it, but then you're using a polyfill 😬 |
Added window.pageYOffset and window.pageXOffset to updatePosition and animateNode to allow proper drag and drop animation for Internet Explorer 11