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 IntersectionObserver isIntersecting in Edge #3365

Merged
merged 1 commit into from May 28, 2017

Conversation

nolanlawson
Copy link
Contributor

@nolanlawson nolanlawson commented May 27, 2017

It turns out Edge's implementation of IntersectionObserver is missing the isIntersecting value. Luckily we can calculate the same value from intersectionRatio and it works in both Chrome and Edge (as well as Firefox/Safari, where a polyfill is used).

@sorin-davidoi
Copy link
Contributor

This was my first approach, but I ran into some issues on Chrome. If I recall correctly, scrolling up and down really fast would result in some toots coming into view but still being "collapsed". Don't have my laptop to test right now. On the other hand this was quite early in the implementation, so it may be possible that the issue was something else.

@nolanlawson
Copy link
Contributor Author

@sorin-davidoi Hm, I'm testing in Chrome 58 on a Mac and I cannot reproduce the issue you describe. I tried both two-finger scrolling and sidebar scrolling, scrolling as fast as I could. Did you see this on a mobile device? Or maybe it was just because of an early implementation?

@sorin-davidoi
Copy link
Contributor

It was on desktop on Linux. Will take a look in a few hours.

@nolanlawson
Copy link
Contributor Author

I have a Lubuntu box. I'll give it a shot. What kind of scrolling method? (keyboard/sidebar/mousewheel/etc.)

@sorin-davidoi
Copy link
Contributor

Mousewheel.

@nolanlawson
Copy link
Contributor Author

OK, I tested on Lubuntu using Chrome 58 on https://malfunctioning.technology and I cannot reproduce with a mousewheel or sidebar scrolling, no matter how fast I go. I'm scrolling the Federated Timeline since there are a lot of toots. 😃

@sorin-davidoi
Copy link
Contributor

Cool 👍, sorry for the trouble.

@nolanlawson
Copy link
Contributor Author

No prob, cross-browser is hard; I am paranoid and like to test everything thoroughly too! :)

@nolanlawson
Copy link
Contributor Author

Just tested in desktop Firefox and Safari as well, and this works. I can tell it works because, as I scroll, the number of DOM elements (document.querySelectorAll('*').length) stay about the same or sometimes even go down. :)

@Gargron Gargron merged commit 24d645b into mastodon:master May 28, 2017
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.

None yet

4 participants