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

Handle zero-area and display: none cases for v2 release branch #103

Merged
merged 1 commit into from
Jan 8, 2019

Conversation

asakusuma
Copy link
Contributor

Attempts to satisfy the spec: https://www.w3.org/TR/intersection-observer/#update-intersection-observations-algo

isIntersecting, non-zero area, and display:nonen are all related, so fixing in one swoop.

Fixes:
#93
#73

Related issues:
w3c/IntersectionObserver#69
w3c/IntersectionObserver#222

Attempts to satsify the spec: https://www.w3.org/TR/intersection-observer/#update-intersection-observations-algo

isIntersecting, non-zero area, and display:nonen are all related, so fixing in one swoop.

Fixes:
#93
#73

Related issues:
w3c/IntersectionObserver#69
w3c/IntersectionObserver#222
@asakusuma
Copy link
Contributor Author

v2 equivalent of #102

@lynchbomb
Copy link
Member

lgtm

@lynchbomb lynchbomb merged commit fba08c7 into v2-release Jan 8, 2019
@guidobouman
Copy link

This is not compliant with Steps 7 & 8:

7. Let isIntersecting be true if targetRect and rootBounds intersect or are edge-adjacent, even if the intersection has zero area (because rootBounds or targetRect have zero area); otherwise, let isIntersecting be false.

8. If targetArea is non-zero, let intersectionRatio be intersectionArea divided by targetArea.
Otherwise, let intersectionRatio be 1 if isIntersecting is true, or 0 if isIntersecting is false.```

Particularly: support for zero space elements is removed.

@lynchbomb lynchbomb deleted the v2-isIntersectingTests branch July 18, 2019 23:38
@asakusuma
Copy link
Contributor Author

@guidobouman thanks for calling out the non-compliance.

@seanjohnson08 can you take a look at this non-compliance issue?

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