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.

Sign up for GitHub

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

Jump to bottom

Lock / retain specified center on resize #308

Closed
wants to merge 5 commits into from
Closed

Lock / retain specified center on resize #308

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f45626c
lockCenter prop added
Jan 23, 2017
ce2264c
Merge pull request #1 from istarkov/master
Marcel-G Jan 23, 2017
01086bc
Resolve conflict
Marcel-G Jun 23, 2017
0a39b6c
Merge branch 'istarkov-master'
Marcel-G Jun 23, 2017
c0688ff
Resolve conflicts
Marcel-G Jun 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/google_map.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ export default class GoogleMap extends Component {
draggable: PropTypes.bool,
style: PropTypes.any,
resetBoundsOnResize: PropTypes.bool,
lockCenter: PropTypes.bool,
layerTypes: PropTypes.arrayOf(PropTypes.string), // ['TransitLayer', 'TrafficLayer']
};

Expand Down Expand Up @@ -268,7 +269,7 @@ export default class GoogleMap extends Component {
0,
this
);
if (this.props.resetBoundsOnResize) {
if (this.props.resetBoundsOnResize || this.props.lockCenter) {
const that = this;
detectElementResize.addResizeListener(mapDom, that._mapDomResizeCallback);
}
Expand Down Expand Up @@ -398,7 +399,7 @@ export default class GoogleMap extends Component {
window.removeEventListener('keydown', this._onKeyDownCapture);
mapDom.removeEventListener('mousedown', this._onMapMouseDownNative, true);
window.removeEventListener('mouseup', this._onChildMouseUp, false);
if (this.props.resetBoundsOnResize) {
if (this.props.resetBoundsOnResize || this.props.lockCenter) {
detectElementResize.removeResizeListener(
mapDom,
this._mapDomResizeCallback
Expand Down Expand Up @@ -453,9 +454,10 @@ export default class GoogleMap extends Component {
_mapDomResizeCallback = () => {
this.resetSizeOnIdle_ = true;
if (this.maps_) {
const originalCenter = this.map_.getCenter();
const originalCenter = this.props.center || this.props.defaultCenter;
const currentCenter = this.map_.getCenter();
this.maps_.event.trigger(this.map_, 'resize');
this.map_.setCenter(originalCenter);
this.map_.setCenter(this.props.lockCenter ? originalCenter : currentCenter);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Marcel-G This PR is looking good! But it is throwing an error in this line on Travis CI:

460:27 error Follow `prettier` formatting (expected '\n' but found 't') prettier/prettier

Would you mind checking on that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@itsmichaeldiego, thanks for stepping up as a new maintainer! I've already upgraded to the latest version, thanks to the fix in #452 (comment). However, I need to get this fix landed as well before I can actually take advantage of the fullscreen functionality.

The CI error can be fixed by running yarn lint --fix, or applying the following patch:

diff --git a/src/google_map.js b/src/google_map.js
index aba1ea5..e4a8b8c 100644
--- a/src/google_map.js
+++ b/src/google_map.js
@@ -457,7 +457,9 @@ export default class GoogleMap extends Component {
       const originalCenter = this.props.center || this.props.defaultCenter;
       const currentCenter = this.map_.getCenter();
       this.maps_.event.trigger(this.map_, 'resize');
-      this.map_.setCenter(this.props.lockCenter ? originalCenter : currentCenter);
+      this.map_.setCenter(
+        this.props.lockCenter ? originalCenter : currentCenter
+      );
     }
   };

Would you like me to submit a separate PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josephfrazier Thanks for the kind words. This fix seems like the solution to our problems. Feel free to make a PR, that would be awesome! We would also need a modification in the docs so users know they're able to use lockCenter

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick feedback, see #482

}
};

Expand Down