daybrush / moveable

Moveable! Draggable! Resizable! Scalable! Rotatable! Warpable! Pinchable! Groupable! Snappable!
https://daybrush.com/moveable/
MIT License
10.08k stars 618 forks source link

Subpixel values are rounded causing misalignment #164

Open dvoytenko opened 4 years ago

dvoytenko commented 4 years ago

Environments

Description

The offsetLeft/offsetTop unfortunately round the actual offsets. Hence, a style="left: 17.85px" becomes offsetLeft == 18. This occasionally causes visible misalignment. This is a bit even more difficult given that sometimes getBoundingClientRect() is used for some operations, which is fractional, like here.

This could be addressed by using getBoundingClientRect() everywhere. However, getBCR is not always equivalent to offsetLeft/Top. The difference between these two is not very common though.

daybrush commented 4 years ago

@dvoytenko

Thank you for reporting bug.

I'll check it.

daybrush commented 4 years ago

@dvoytenko

There is one question: Is the situation where the target uses position: fixed?

dvoytenko commented 4 years ago

@daybrush In that particular case it was a position: absolute target.

daybrush commented 4 years ago

@dvoytenko

https://github.com/daybrush/moveable/blob/master/packages/react-moveable/src/react-moveable/ables/Resizable.ts#L269

Oh right. It is not related to the isFixed code. Related to Resizable code. This issue has come a lot. Mostly using left, top.

You should use transform translate instead of left and top to ensure that you do not shake correctly when resizing.

dvoytenko commented 4 years ago

@daybrush, not sure that's the case here. I'm pretty certain that this is due to the this code:

let offsetLeft = (el as HTMLElement).offsetLeft;
let offsetTop = (el as HTMLElement).offsetTop;

I mean no matter what happens - these always return integers.

daybrush commented 4 years ago

@dvoytenko Oh yes. If you use left, top, that code will be the problem. There is no replacement for offsetTop and offsetLeft. So use transform: translate.

dvoytenko commented 4 years ago

This appears the issue with Chrome rounding the height/width. I filed https://bugs.chromium.org/p/chromium/issues/detail?id=1049374

EffakT commented 3 years ago

I noticed that this rounding bug is now marked as WontFix, as the issue occurs on Firefox and Safari as well.

daybrush commented 3 years ago

@EffakT

Not yet. It's not precise, but there's a way to do it most similarly.

bytasv commented 1 year ago

hey @daybrush I just bumped into this issue as well 🤔 I'm wondering - would it be possible to use getBoundingClientRect to derive left and top values? If you take parent and subtract child rect values, then you get same values as for offsetLeft/Top - that should take care of the issue with misalignment

anxpara commented 1 year ago

hey @daybrush I just bumped into this issue as well 🤔 I'm wondering - would it be possible to use getBoundingClientRect to derive left and top values? If you take parent and subtract child rect values, then you get same values as for offsetLeft/Top - that should take care of the issue with misalignment

Do transforms affect getBoundingClientRect? I have the same issue with Wayfinder, and I've been avoiding gBCR so far due to performance concerns, but if it's the only way to achieve precision then i might bite the bullet for now.

One of my concerns is if gBCR takes transforms into account, because i require the offset from direct parent ignoring transforms

Sure would be nice if preciseOffsetLeft and preciseOffsetTop existed

anxpara commented 1 year ago

looked into gBCR() more and it does calculate the bounds after any transforms are applied. so not useful for Wayfinder, but perhaps still useful for y'all depending on exactly what you're looking for

daybrush commented 1 year ago

@bytasv @anxpara

gBCR is screen standard getRect of moveable is based on container.

It can make a difference. The transform affects gBCR. Rotate and scale change the width and height of gBCR.

bytasv commented 1 year ago

@daybrush do you see any way how the misalignment issue could be solved? either by tweaks to the library or maybe it's possible to override it using API right now?

I.e. the simplest use case that I can come up with:

  1. Create 10px container
  2. Add 1px vertical child element
  3. Using flexbox center that element
  4. Use Resizeable on that child element so you can change its height
  5. You will see that moveable's indicator is misaligned
daybrush commented 1 year ago

@bytasv

Can you provide a codepen or code sandbox demo?

bytasv commented 1 year ago

@daybrush sorry, should have done it before, here it is https://codesandbox.io/s/moveable-alignement-b5ynwi (I added transform so it's a bit more visible

daybrush commented 1 year ago

@bytasv @dvoytenko @anxpara @EffakT

react-moveable@0.45.0-beta.2 is released.

And use useAccuratePosition prop (to true)

https://codesandbox.io/s/moveable-alignement-forked-h7ogfw?file=/src/index.js

bytasv commented 1 year ago

@daybrush that works perfectly, thank you!

daybrush commented 1 year ago

@bytasv @dvoytenko @anxpara @EffakT

moveable's new version is released.

use useAccuratePosition prop (to true)

bytasv commented 1 year ago

Hey @daybrush I'm not sure if you have a way to test it out but seems that when useAccuratePosition is used with multiple object selection the selector box breaks and is displayed in a random place. I don't have codesandbox for investigation yet if you need one, lemme know I'll try to recreate, but maybe you will know what's the issue without one. Thank you

https://user-images.githubusercontent.com/437214/211390737-ee685fbb-8a8e-4572-bebe-8f3a5bea83c6.mp4