Position Calculation is wrong when body has css transform bug (2) about ctxmenu HOT 19 CLOSED

If there is no simple fix, a way to display the menu at a specific position X,Y would also be desirable.

from ctxmenu.

I've already investigated a little bit, a fix shouldn't be too complicated. Basically the culprit is that the values returned by Element.getBoundingClientRect() are calculated after the transform has taken place. This means I'll have to switch to offsetWidth and other respective values, as these don't take the transform into effect.

Hopefully this will actually simplify the implementation a bit as I can then probably delete the fix for the first issue of this kind (#31), however Element.getBoundingClientRect() is used in multiple places, so some rework might be required...

from ctxmenu.

Thanks for your response and your efforts!

from ctxmenu.

You're welcome!
You can track the progress on the position branch. I've started to extract all positioning code into it's own file. Also, I noticed that the context menu can be outside the viewport, when the body is larger than the viewport. Probably it is easier to rewrite the positioning logic entirely, as it has grown into quite a mess and there are several changes required for the fixes.
Currently the code on the branch isn't working any better, but I'll let you know once I have done some fixes.
I'd appreciate your help validating the changes, once I feel they are ready... You seem to have a talent for finding peculiar bugs 😂

from ctxmenu.

a way to display the menu at a specific position X,Y would also be desirable.

good point, will definitely add this as a convenience method.

from ctxmenu.

This sounds like a plan! I will occasionally pop in to check the progress and will help with testing once it's ready.

from ctxmenu.

I've just uploaded another change and also pushed the compiled js files so you can download them from the position branch.
I've tried different transforms: scale > 1, scale < 1, scale and transform and all the former while zooming into the page and scrolling and I couldn't get the menu to render outside the viewport. To me the implementation seems very solid, but maybe you can break it 😅
Surprisingly to me, even 3D transforms work sufficiently well in my opinion, although not perfect in edge cases (pun intended 😄)

Thanks for reporting, I enjoy a good challenge and this certainly has been one so far!

from ctxmenu.

I've created PR #40 and will merge it if you don't find more bugs

from ctxmenu.

Something seems wrong. The menu appears at the very bottom of this page. Right-click the "test" button.

from ctxmenu.

could you please use the non-minified version so I can debug the page and see what's going on?

from ctxmenu.

nevermind, I found the issue, getScale() looks at the ratio between body size and viewport size, which obviously will only work when the body's width and height is 100% - I am used to that, as I always build my webpages like that, but in your example it is only 5% of the viewport's height and thus the y position is magnified 20 times...
Comparing the body's bounding box versus its offset size should work, I'll upload a change i a few minutes

from ctxmenu.

I found another issue, will upload a fix shortly

from ctxmenu.

I have just pushed some more fixes, I am pretty confident that it works as intended now 😄

from ctxmenu.

I just did some quick tests and at first glance all looks good.
Just a minor thing that I've noticed: The menu appears 2px too far to the right when it appears at the rightmost edge.
I'm not sure if this only happens when there is no space on the right and it is therefore positioned to the far right.

from ctxmenu.

Interesting...
I am pretty sure that's because of the DOM box model, in my demo and tests I have

* {
  box-sizing: border-box;
} 

which is also a quite common reset pattern, which you don't have on your page, so I guess the border is added to the final size, causing the overflow...

Try adding it to your page, to fix it.

I didn't want to add it to the css that comes with ctxmenu to avoid overwriting anything, but I might add it to the ctxmenu class and all children...

from ctxmenu.

I just pushed a fix for the box sizing issue, too.

from ctxmenu.

Excellent, I'll test it a bit later today but think it's all good now.

from ctxmenu.

Thanks, it's perfect now.

from ctxmenu.

Glad to hear that 🙂

from ctxmenu.