Fix markers position on fullscreen mode by jbcochery · Pull Request #452 · google-map-react/google-map-react

@jbcochery

On fullscreen mode, the view size should not be computed from the DOM element size, but from the window dimensions.

Fixes #296

istarkov

const mapDom = ReactDOM.findDOMNode(this.googleMapDom_);
this.geoService_.setViewSize(mapDom.clientWidth, mapDom.clientHeight);
if (document.fullscreen || document.webkitIsFullScreen || document.mozFullScreen) {
_this.geoService_.setViewSize(window.innerWidth, window.innerHeight);

Choose a reason for hiding this comment

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

_this is not defined

Choose a reason for hiding this comment

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

Fixed, sorry about that!

@emilpalsson

Great find @jbcochery! I was just about to start looking for a fix for #296 when I saw your PR. Crazy timing, for an issue 10 month old. Thanks @istarkov for a great lib!

@cochery

@itsmichaeldiego

Wow @emilpalsson same here! I was looking at that issue right now.

This is looking great @jbcochery, thank you! And thank you @istarkov for the lib!

Looking forward to get this merged.

@nicolasdelfino

Any status of when this is getting merged?

emilpalsson, itsmichaeldiego, vitaliy-bykovets, husek, GauthierD-, sheraz-sarwar, edhelb, Turee, MaxFrolov, kimlai, and 3 more reacted with heart emoji

@qur2

I also got bitten by that issue. While the fix offered seems to work (thanks for that!), the real problem is that the ref that GooleMapReact keeps does not point to the div used to render the fullscreen map. The fix works because incidentally, the div to which we should have a ref to has the same size than the window.

@MaxFrolov

@ShepelievD

@istarkov really need this stuff, merge, please! 😢 😢 😢

@nicolasdelfino

@istarkov Please communicate if this is getting merged or not. It would be a shame if people started working on their own forks and this project would stagnate.

@jkettmann

@istarkov We also need this fix. Do you plan on merging this?

@istarkov

Guys I need collaborator-leader of this project, see the readme top. I have no desire now to do anything :-(

@nicolasdelfino

@istarkov I understand your position and hope you'll find someone willing to help. I'm sure you'll do.
In the meantime there is no reason for you not to merge this.

paupalou

huwr

huwr approved these changes Dec 15, 2017

minhkl

emilpalsson

nicolasdelfino

@itsmichaeldiego

@lock

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