fix(scroll): prevent wrong offset when the scroll is on the html element #2499
Conversation
src/dd-draggable.ts
Outdated
| const transformParentRect = transformParent.getBoundingClientRect(); | ||
| // We need to be careful here as the html element actually also includes scroll | ||
| // so in this case we always need to ignore it | ||
| const transformParentRect = transformParent === document.documentElement ? { top: 0, left: 0 } : transformParent.getBoundingClientRect(); |
There was a problem hiding this comment.
isn't this assuming here that the entire doc is scrollable, but it could be any parent div in between. I thought we had code to detect scrolled parent and take that into content... I would look back at the original code that handled any level fine (just didn't scale)
There was a problem hiding this comment.
yes actually you are right the issue is bigger than that. I will fix it 😄
|
Okey I tested pretty much everything and I couldn't find issue. It should be good 🤞 |
|
thanks for this fix but #2498 #2497 remains and I have to revert all your changes for now as 9.3 is worst, until you can prove your entire new code path is better than what it replaced. would have been much better to add scale to existing code instead of hunting/debugging all the issues you have now created. Update: I have reverted in #2504 which you would need to pick again if you insist on your new code path and can prove it is better than adding scale to the old code path. |
gridstack#2263) * fix(drag): place the element dragged always below the mouse * fix: review fixes * fix: fix data transfer demo * fix: review fixes * fix: fix position according to first transform parent * review fixes * review fixes * review fixes * fix: fix the refacto --------- Co-authored-by: Vincent Molinié <vincent.m@forestadmin.com>
VincentMolinie
force-pushed
the
fix/scroll-on-html-element
branch
from
ad024a9 to
cc005fc
Compare
|
This one is fixed too: #2498 but like I told you for the other one. I don't see how it is related. It seems more related to a PR that is 3 weeks old, not mine |
|
going with #2515 instead of your re-write (what I had asked all along). |
Description
Having scroll on the
htmlelement was increasing the offset because that element behave a bit differently, meaning that the scroll was included in the position of the html element.So we had basically position like
But if it's the
htmlelement in our case we were always expecting0Checklist
yarn test)Issues related
Fix #2491