mdbassit / Coloris

A lightweight and elegant JavaScript color picker. Written in vanilla ES6, no dependencies. Accessible.
https://coloris.js.org/examples.html
MIT License
450 stars 58 forks source link

Inline: Mouse Pointer position calculated incorrectly #79

Closed melloware closed 1 year ago

melloware commented 1 year ago

Codepen: https://codepen.io/melloware/pen/PoBgraz

If you have the inline position not at the top of the screen it calculates the Mouse Marker incorrectly. For example click the mouse anywhere in the picker and you will see the Marker jump right to the top.

mdbassit commented 1 year ago

Thank you for reporting this bug. I'll work on a fix ASAP.

mdbassit commented 1 year ago

I pushed a fix. I'll make a new release once you confirm that it fixes your issue.

melloware commented 1 year ago

Its better but not quite right. inline

My inline is in the middle of the page laid out.

mdbassit commented 1 year ago

Can you send me a link to the page you are using to test? Or alternatively make a codepen? I'll look into it tomorrow

melloware commented 1 year ago

let me try and createa code pen my app is not available yet.

melloware commented 1 year ago

OK I think I updated this to show it more closely to the issue?

https://codepen.io/melloware/pen/PoBgraz

mdbassit commented 1 year ago

This is fixed in my version ! Are you using the latest patch I pushed? I didn't make a new release yet, so the dist files aren't updated yet. Could you test it with the source file?

melloware commented 1 year ago

Ok I will try again. If it's still happening i will try and see what is different in my page but when I debugged the Y value was like -766 and adding the other value still didn't make it positive so it was way off.

I will let you know!

melloware commented 1 year ago

OK I debugged my example and after applying your fix I needed to add this to make it work.

    if (container) {
      x -= container.offsetLeft;
      y += container.scrollTop - container.offsetTop;
    }

basically it was not accounting for the container itself position in the document. Have to subtract the offsets of the container. Can you give that a try?

mdbassit commented 1 year ago

That doesn't make sense cause I'm certain I account for the container's (parent option) position. I just replaced the jsdelivr version of the script with mine in the codepen you sent and it fixes the issue. Here is my version:

https://coloris.js.org/temp/coloris.js

Could you try that one and let me know?

mdbassit commented 1 year ago

And here is my version of the pen:

https://codepen.io/mbassit/pen/abjgROO

melloware commented 1 year ago

Yep I don't think its margin exactly that is causing the issue. I decided to print out your current code values and what is happening.

Currently with your code.

    const pointer = getPointerPosition(event);
    console.log("Pointer Position:");
    console.log(pointer);
    console.log("Color Area Dimensions:");
    console.log(colorAreaDims);
    let x = pointer.pageX - colorAreaDims.x;
    let y = pointer.pageY - colorAreaDims.y;

    console.log("Container Offset Left: " + container.offsetLeft);
    console.log("Container Offset Top: " + container.offsetTop);

    if (container) {
      // x -= container.offsetLeft;
      y += container.scrollTop;// - container.offsetTop;
    }

    console.log("Calculated X: " + x + " Y: " + y);

Result: image

You can see it calculates and X and Y outside of the colorAreaDims so you just pin to a corner or edge.

Here is with my fix it calculates appropriately within the colorAreaDims. image

melloware commented 1 year ago

Figured out my issue! It was the way my DOM was loading you calculate on DOM READY the offsets and then I manipulate the DOM so its not in the same spot. I just have to call updatePosition and it fixes it

melloware commented 1 year ago

OK so I know why mine works and yours is not for me as well.

I created a reproducer: https://codepen.io/melloware/pen/OJoLLWO

  1. Click the inline and the mouse marker and color are correct.
  2. Now resize the display to be less than 800px and it makes the div hide with a media query
  3. Now click the inline again you will see the same behavior where it is off.

I believe this is because you are caching the color picker offset at DOMREADY and mine above was calculating it eveyr time.

mdbassit commented 1 year ago

Oh I see, in that case you can solve it by calling updatePosition when the window is resized:

window.addEventListener('resize', event => { Coloris.updatePosition(); });
melloware commented 1 year ago

Thanks! 0.18.0 NPM version has been published.