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

Projects
None yet
4 participants
@nolanlawson
Collaborator

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

This comment has been minimized.

Show comment
Hide comment
@sorin-davidoi

sorin-davidoi May 27, 2017

Collaborator

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.

Collaborator

sorin-davidoi commented May 27, 2017

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.

@ykzts

ykzts approved these changes May 27, 2017

@nolanlawson

This comment has been minimized.

Show comment
Hide comment
@nolanlawson

nolanlawson May 27, 2017

Collaborator

@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?

Collaborator

nolanlawson commented May 27, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@sorin-davidoi

sorin-davidoi May 27, 2017

Collaborator

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

Collaborator

sorin-davidoi commented May 27, 2017

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

@nolanlawson

This comment has been minimized.

Show comment
Hide comment
@nolanlawson

nolanlawson May 27, 2017

Collaborator

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

Collaborator

nolanlawson commented May 27, 2017

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

@sorin-davidoi

This comment has been minimized.

Show comment
Hide comment
@sorin-davidoi

sorin-davidoi May 27, 2017

Collaborator

Mousewheel.

Collaborator

sorin-davidoi commented May 27, 2017

Mousewheel.

@nolanlawson

This comment has been minimized.

Show comment
Hide comment
@nolanlawson

nolanlawson May 27, 2017

Collaborator

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. 😃

Collaborator

nolanlawson commented May 27, 2017

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

This comment has been minimized.

Show comment
Hide comment
@sorin-davidoi

sorin-davidoi May 27, 2017

Collaborator

Cool 👍, sorry for the trouble.

Collaborator

sorin-davidoi commented May 27, 2017

Cool 👍, sorry for the trouble.

@nolanlawson

This comment has been minimized.

Show comment
Hide comment
@nolanlawson

nolanlawson May 27, 2017

Collaborator

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

Collaborator

nolanlawson commented May 27, 2017

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

@nolanlawson

This comment has been minimized.

Show comment
Hide comment
@nolanlawson

nolanlawson May 27, 2017

Collaborator

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. :)

Collaborator

nolanlawson commented May 27, 2017

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 tootsuite:master May 28, 2017

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
codeclimate no new or fixed issues
Details

gol-cha added a commit to gol-cha/mastodon that referenced this pull request May 29, 2017

orekyuu added a commit to orekyuu/mastodon that referenced this pull request May 31, 2017

YaQ00 added a commit to YaQ00/mastodon that referenced this pull request Sep 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment