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

Add passive scroll #631

Merged

Conversation

jooj123
Copy link
Contributor

@jooj123 jooj123 commented Aug 15, 2018

Added passive scroll event (which is more performant).

See: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener (passive in options)

Needed the feature detection for browsers that dont support it

Note: hard to test this as the demos for google-map-react are all busted due to incorrect google maps key

@itsmichaeldiego
Copy link
Member

@jooj123 Is this related to #625 ?

@jooj123
Copy link
Contributor Author

jooj123 commented Aug 16, 2018

@itsmichaeldiego this is the only place in this lib where it needs a passive event - so it should get rid of the warnings in the console and things should be a bit smoother (so yes its related to #625)

Come to think of it, i might as well add this to the other 4 event listeners too:

window.addEventListener('resize', this._onWindowResize);

@itsmichaeldiego
Copy link
Member

@jooj123 Sounds good, maybe we can create a new method call addPassiveEventListener so we avoid DRY, what do you think?

@jooj123
Copy link
Contributor Author

jooj123 commented Aug 17, 2018

@itsmichaeldiego updated. Not sure how i can unit test this as its browser specific functionality
I cant even validate it working correctly without the demo working incorrect due to google maps key

@itsmichaeldiego
Copy link
Member

@jooj123 That is fine man! Thanks for this! Could you provide something like a video or whatever so I can see this is working? I really want to merge this but I've no time to test it :(

@itsmichaeldiego
Copy link
Member

@jooj123 Bump!

@jooj123
Copy link
Contributor Author

jooj123 commented Sep 14, 2018

@itsmichaeldiego sorry havent had a chance to test this one - will do sometime over the weekend or next week

It would be much easier if the internal demo worked in this repo :(

@itsmichaeldiego
Copy link
Member

@jooj123 What do you mean that the internal demo worked? We're working on new examples at https://github.com/google-map-react/google-map-react-examples

@itsmichaeldiego itsmichaeldiego merged commit 40c8f67 into google-map-react:master Sep 20, 2018
itsmichaeldiego added a commit that referenced this pull request Sep 21, 2018
itsmichaeldiego added a commit that referenced this pull request Sep 21, 2018
* Revert "Bump version to 1.0.7 (#644)"

This reverts commit 800092a.

* Revert "Add passive scroll (#631)"

This reverts commit 40c8f67.

* Revert "Use React 16 portal to render map overlay (#643)"

This reverts commit b121bb6.
@jooj123 jooj123 deleted the patch/add-passive-scroll branch October 8, 2018 11:20
DonovanDeSmedt added a commit to DonovanDeSmedt/google-map-react that referenced this pull request Nov 21, 2018
* 'master' of github.com:google-map-react/google-map-react:
  Bump to 1.1.1 (google-map-react#680)
  Revert "Added feature: update heat map on data change + fix linting" (google-map-react#679)
  Bump version to 1.1.0 (google-map-react#671)
  Added feature: update heat map on data change + fix linting (google-map-react#593)
  Pass map instance to onDrag handler (google-map-react#656)
  add math abs to avoid negative values when calculating zoom (google-map-react#655)
  Bump version to 1.0.9 (google-map-react#651)
  Custom div style options (google-map-react#634)
  Bump version to 1.0.8 (google-map-react#646)
  Revert 643 fix/map context (google-map-react#645)
  Bump version to 1.0.7 (google-map-react#644)
  Add passive scroll (google-map-react#631)
  Use React 16 portal to render map overlay (google-map-react#643)
  Fix old examples links and add one to new examples (google-map-react#633)
  Bump version to 1.0.6 (google-map-react#621)
  Add prop `onTilesLoaded` (google-map-react#615)
  Fix typo, and call fromContainerPixelToLatLng() as you would expect. (google-map-react#620)
  Update API.md (google-map-react#611)
  Upgrade version to 1.0.5 (google-map-react#607)
  Remove marker jiggle. (google-map-react#603)
@lock
Copy link

lock bot commented Dec 1, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants