- Notifications
You must be signed in to change notification settings - Fork855
MakeresetBoundsOnResize preserve center when full-screened#482
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
MakeresetBoundsOnResize preserve center when full-screened#482
Uh oh!
There was an error while loading.Please reload this page.
Conversation
adding note about `resetBoundsOnResize` to readme (google-map-react#307)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for your PR! It is looking good, I left a comment that is more like a question
src/google_map.js Outdated
| @@ -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) { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Wondering if|| this.props.lockCenter is really needed, asresetBoundsOnResize should be set to true if we want to lock the center, not sure if I am explaining myself, but I believe the map always should re-set the center when reseting the bounds on resize.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hmm, I think I see what you're saying. I hadn't usedresetBoundsOnResize before, but I just tried it out on the master branch and on this PR's branch by runningyarn start and visitinghttp://localhost:4000/resizable. On both branches, resizing by dragging the lower- the map keeps the map centered, but only with this PR does clicking the fullscreen button also keep the map centered. I think it makes sense forresetBoundsOnResize to include thelockCenter behavior, so I removed thelockCenter stuff while keeping the behavioral changes. See692e15b
lockCenter prop to preserve center on resizeresetBoundsOnResize preserve center when full-screeneditsmichaeldiego commentedDec 21, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@josephfrazier Just to confirm, did you test latest changes? Could you upload something? |
Yes, I verified the fix as outlinedabove, by running |
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. |
This supersedes andresolves#308,
as specified in#308 (comment)