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

Disable ViewportMetrics unless MouseEvent lacks support for pageX/pageY #6129

Merged
merged 1 commit into from
Feb 26, 2016
Merged

Disable ViewportMetrics unless MouseEvent lacks support for pageX/pageY #6129

merged 1 commit into from
Feb 26, 2016

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Feb 26, 2016

This rebases #2271 by @syranide which fixes #1300.

The only change in the rebase is that the file moved.
I haven’t changed the logic.

@spicyj cleaned up the callsites internally so this should be good.

gaearon added a commit that referenced this pull request Feb 26, 2016
Disable ViewportMetrics unless MouseEvent lacks support for pageX/pageY
@gaearon gaearon merged commit 1e5bb2e into facebook:master Feb 26, 2016
@gaearon gaearon deleted the syranide-lessvpm branch February 26, 2016 20:56
@syranide
Copy link
Contributor

👍

gaearon added a commit that referenced this pull request Feb 26, 2016
@gaearon
Copy link
Collaborator Author

gaearon commented Feb 26, 2016

Oops, I merged before it linted.
Fixed the lint in 4c80436.

raineroviir pushed a commit to raineroviir/react that referenced this pull request Mar 5, 2016
raineroviir pushed a commit to raineroviir/react that referenced this pull request Mar 5, 2016
@sophiebits sophiebits added this to the 15-next milestone Aug 27, 2016
@sophiebits
Copy link
Collaborator

@zpao Can we pick this into the next patch? @axemclion asked me about it today.

@zpao zpao modified the milestones: 15.3.2, 15-next Sep 8, 2016
@zpao
Copy link
Member

zpao commented Sep 8, 2016

@spicyj This shipped in 15.0.0 (in the RCs even)… what issue are you guys seeing?

@sophiebits
Copy link
Collaborator

Oh, my mistake. I thought I verified it hadn't been picked but I just checked again and it does look okay. @axemclion Can you let us know if you're still seeing a scroll listener added?

@aweary
Copy link
Contributor

aweary commented Sep 8, 2016

@spicyj I just tested this in Chrome. monitorScrollValue is never called and there's no scroll listener attached that I can see.

@axemclion
Copy link

axemclion commented Sep 8, 2016

@spicyj I am using React 15.3.1 and for some reason, I was still seeing the scroll event handler being added. The stack trace shows that it is from here. Looks like this was because I was using JSDOM in my renderer and the change from this condition was not being me.

So yes, this is fixed and we no longer have a scroll handler.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

React tracks scroll position internally and forces synchronous layout
6 participants