colorjoe icon indicating copy to clipboard operation
colorjoe copied to clipboard

findPos incorrect offset when used with perfectScrollbar

Open mrloop opened this issue 10 years ago • 17 comments
trafficstars

Replacing findPos function with following fixes issue

//https://stackoverflow.com/questions/442404/retrieve-the-position-x-y-of-an-html-element
function findPos(e) {
  var bodyRect = document.body.getBoundingClientRect(),
  elemRect = e.getBoundingClientRect(),
  x = elemRect.left - bodyRect.left,
  y = elemRect.top - bodyRect.top;
  return {x: x, y: y};
}

unfinished demo

mrloop avatar Sep 03 '15 15:09 mrloop

Great! Do you want to do a PR or should I patch it?

bebraw avatar Sep 03 '15 15:09 bebraw

@bebraw I can check it in IE10 and latest chrome and firefox from application code. Will submit a PR when checked, cheers.

mrloop avatar Sep 03 '15 16:09 mrloop

@mrloop Great. Thanks. :+1:

bebraw avatar Sep 03 '15 16:09 bebraw

@mrloop Any luck? I don't mind patching it if you are busy.

bebraw avatar Oct 10 '15 06:10 bebraw

Sorry, lots of deadlines, on todo list, please patch if you want or I'll get around to it eventually

Sent from my mobile device

On 10 Oct 2015, at 07:38, Juho Vepsäläinen [email protected] wrote:

@mrloop Any luck? I don't mind patching it if you are busy.

— Reply to this email directly or view it on GitHub.

mrloop avatar Oct 10 '15 06:10 mrloop

@mrloop No problems. I tried the function you suggested at dragjs. Unfortunately it fails for the first demonstration case (the leftmost one). The calculation doesn't work when you have overflow like that. Maybe we need to combine your solution with offsetParent based one somehow?

bebraw avatar Nov 30 '15 15:11 bebraw

I have found a solution to make colorjoe work inside scrollable elements.

function findPos(e) {
  var bodyRect = document.body.getBoundingClientRect(),
  elemRect = e.getBoundingClientRect(),
  x = elemRect.left - bodyRect.left,
  y = elemRect.top - bodyRect.top;
  var offsetX = scrollableParent != null ? scrollableParent.scrollLeft : 0;
  var offsetY = scrollableParent != null ? scrollableParent.scrollTop : 0;
  return {x: x + offsetX, y: y + offsetY};
}

This does the tricks and works quite well. My only concern before doing a PR for that, is that I'm not sure how to pass the reference to that scrollableParent when I init the plugin.

Any ideas, recommendations?

germain-gg avatar Feb 18 '16 12:02 germain-gg

@gsouquet That's cool. Where does that scrollableParent exactly come from?

bebraw avatar Feb 19 '16 06:02 bebraw

@bebraw So that's pretty much my problem, sorry if I wasn't very clear in my first message. So scrollableParent is a reference to the closest ancestor that is set with overflow:auto or overflow:scroll.

The problem is that we need to specify somewhere that reference (perhaps on the plugin initialisation).

At the moment I have added an extra parameter on the colorjoe constructor, but I am not sure that this is the way you want to handle thing. That's why I commented first before doing a PR. Just to know what approach you would prefer for passing a reference to scrollableParent when initialising the plugin.

Thanks

germain-gg avatar Feb 19 '16 07:02 germain-gg

I don't have a strong opinion on this. Plugin initialization sounds fair enough as then we don't have to touch the core.

bebraw avatar Feb 19 '16 08:02 bebraw

Cheers, i'll write a PR and update the doc during the week-end.

germain-gg avatar Feb 19 '16 09:02 germain-gg

Cool. Thanks a lot!

bebraw avatar Feb 19 '16 09:02 bebraw

Sorry I didn't come back to you on this yet, but I found some use cases that are still buggy with the fix I did. So i'm trying to find a fix that would work in every scenario but I'm quite struggling to find it and I don't have a lot of time.

More to come soon hopefully

germain-gg avatar Mar 05 '16 13:03 germain-gg

No hurry. Thanks for the update. :+1:

bebraw avatar Mar 05 '16 13:03 bebraw

@gsouquet Any update? I would love to see a proper fix. 👍

We did certain IE9+ related fix a while ago so that might have changed the situation somehow.

bebraw avatar Aug 15 '16 08:08 bebraw

Sorry, forgot to come back to you, well I spend quite a lot of time on this, and when I found a fix it was either not covering every single use case or not working well cross-browser. Kind of tricky this one

germain-gg avatar Aug 15 '16 10:08 germain-gg

@gsouquet No probs. Thanks for the effort.

bebraw avatar Aug 15 '16 15:08 bebraw