colorjoe

colorjoe copied to clipboard

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

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

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

@mrloop Great. Thanks. :+1:

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

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 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?

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?

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

@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

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

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

Cool. Thanks a lot!

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

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

@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.

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

@gsouquet No probs. Thanks for the effort.