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

Adds x and y to IntersectionObserverEntry #1222

Merged
merged 1 commit into from
Aug 10, 2022
Merged

Adds x and y to IntersectionObserverEntry #1222

merged 1 commit into from
Aug 10, 2022

Conversation

mhassan1
Copy link
Collaborator

This PR adds x and y to all instances of IntersectionObserverEntry in the IntersectionObserver polyfill.

Resolves #116.

@mhassan1 mhassan1 requested a review from a team as a code owner August 10, 2022 16:13
@origamiserviceuser origamiserviceuser added this to Backlog in Origami ✨ Aug 10, 2022
@github-actions github-actions bot added the library Relates to an Origami library label Aug 10, 2022
@JakeChampion
Copy link
Owner

JakeChampion commented Aug 10, 2022

/ok-to-test sha=15a5742

https://github.com/Financial-Times/polyfill-library/actions/runs/2834150336

@romainmenke
Copy link
Collaborator

https://developer.mozilla.org/en-US/docs/Web/API/DOMRectReadOnly

DOMRectReadOnly.top Read only
Returns the top coordinate value of the DOMRect (usually the same as y.)

When is it not the same?

https://drafts.fxtf.org/geometry/#DOMRect

The y attribute, on getting, it must return the y coordinate value. For the DOMRect interface, setting the y attribute must set the y coordinate to the new value.

The top attribute, on getting, must return the NaN-safe minimum of the y coordinate and the sum of the y coordinate and the height dimension.

Is it correct that y can be NaN-unsafe and top can not?

@mhassan1
Copy link
Collaborator Author

y is different from top when height is negative; for example,

{
  y: 10
  top: 9
  height: -1
}

In the case of this polyfill, it doesn't seem like height will ever be negative, so they will always be the same.

I'm not sure if any of the top values in this polyfill could ever be NaN, but if they could, what would we use for y, since y is derived from top?

@romainmenke
Copy link
Collaborator

I agree that the difference won't matter in the context of this polyfill.
Thank you for helping me understand it better :)

Copy link
Collaborator

@romainmenke romainmenke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you 🎉

@romainmenke romainmenke merged commit 0a37f6e into JakeChampion:master Aug 10, 2022
Origami ✨ automation moved this from Backlog to Done Aug 10, 2022
@mhassan1 mhassan1 deleted the client-rect-x-y branch June 30, 2023 12:40
@robertboulton robertboulton removed this from Done in Origami ✨ Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
library Relates to an Origami library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IntersectionObserver GetBoundingRect missing properties
3 participants