Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix top & left calculation #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix top & left calculation #9

wants to merge 2 commits into from

Conversation

janosorcsik
Copy link

No description provided.

@wmertens
Copy link

Fixes #12

Would this not be better as

        top: "top" in rect ? rect.top : rect.y,
        left: "left" in rect ? rect.left : rect.x,

@janosorcsik
Copy link
Author

In older browsers, the getBoundingClientRect function doesn't return with the x and y.
Every browser fills the top and left properties so the first part of your condition is always true.

@wmertens
Copy link

So if top and left always exist, why polyfill them?

@wmertens
Copy link

in fact, maybe the function should check if any props are missing, and otherwise return the object unchanged.

@janosorcsik
Copy link
Author

I fixed. Thank you @wmertens 👍

@daneroo
Copy link

daneroo commented Aug 10, 2020

I found this as well.
Actually both top and left are inverted.

  const [ref, { x: left, y: top}] = useDimensions()

@janosorcsik
Copy link
Author

@Swizec can you check this PR, please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants