egeloen / ivory-google-map

Google Map API v3 integration for PHP 5.6+.
MIT License
325 stars 185 forks source link

Setting options on static map #237

Closed luizcesard closed 7 years ago

luizcesard commented 7 years ago

Apparently, setting marker options (via setStaticOption) have no effect on static maps (they're not rendered). However, when you set parameter as if they were "dinamic" (setIcon, for example), these parameters ARE rendered in the map. Moreover, some of static MAP options (such as zoom, but not height and width) are not having effect as well.

egeloen commented 7 years ago

@luizcesard Are you saying that https://github.com/egeloen/ivory-google-map/blob/master/tests/Helper/Functional/StaticMapFunctionalTest.php#L197-L219 does nothing (no effect)?

luizcesard commented 7 years ago

The marker gets plotted in the map, but with no option (such as color, label etc). The same happens when setting static map settings, where only width and height seem to have effect.

egeloen commented 7 years ago

Ok thanks for the feedback. Need more investigations...

egeloen commented 7 years ago

@luizcesard Can you share your code? I'm not able to reproduce your issue for markers style. For the zoom, there is indeed a bug since the zoom static option is not checked currently.

For markers, the following snippet generates: https://maps.googleapis.com/maps/api/staticmap?size=300x300&visible=0%2C0%7C1%2C1%7CLille%2C+France&markers=0%2C0&markers=anchor%3Abottomright%7Ccolor%3Ablue%7C1%2C1%7CLille%2C+France

$styles = [
    'anchor' => 'bottomright',
    'color'  => 'blue',
];

$marker = new Marker(new Coordinate());

$styledMarker = new Marker(new Coordinate(1, 1));
$styledMarker->setStaticOption('styles', $styles);

$styledMarkerAddress = new Marker(new Coordinate());
$styledMarkerAddress->setStaticOption('location', 'Lille, France');
$styledMarkerAddress->setStaticOption('styles', $styles);

$map = new Map();
$map->setAutoZoom(true);

$map->getOverlayManager()->addMarker($marker);
$map->getOverlayManager()->addMarker($styledMarker);
$map->getOverlayManager()->addMarker($styledMarkerAddress);

$this->renderMap($map);
luizcesard commented 7 years ago

Sure, @egeloen. The following code:

$mapOptions = [
    'height' =>720,
    'width'=>720,
    'center'=>$centerPoint
    'zoom' =>16
    ];
$map = new Map();
$map->setCenter($centerPoint);
foreach($mapOptions as $key => $value)
    if(in_array(
        $key,
        ['center','zoom','format','scale',
        'maptype','visible','height','width']
        ))
        $map->setStaticOption($key,$value);

//set position given
$map->getOverlayManager()->addMarker(new Marker($centerPoint));

foreach($busStops as $key => $stopRow){
    $busStopCoordinate = new Coordinate(
        $stopRow['stop']->getCoord()->getLat(),
        $stopRow['stop']->getCoord()->getLon());

    $busStopMarker = new Marker($busStopCoordinate);
    $busStopMarker
        ->setStaticOption('icon',
            "http://olhovivo.000webhostapp.com/img/markers/numbers/number_".($key+1).".png");
    $busStopMarker->setStaticOption('location',$busStopCoordinate);
    $map->getOverlayManager()->addMarker($busStopMarker);
}
$staticMapHelperBuilder = StaticMapHelperBuilder::create();
$staticMapHelperBuilder->setKey('key'); 
$staticMapHelper = $staticMapHelperBuilder->build();
echo $staticMapHelper->render($map);

gives: https://maps.googleapis.com/maps/api/staticmap?size=720x720&zoom=3&markers=-23.583565%2C-46.584628%7C-23.5839%2C-46.5848%7C-23.5832%2C-46.585%7C-23.5828%2C-46.5849%7C-23.5841%2C-46.5838%7C-23.584%2C-46.5833&key='key'

egeloen commented 7 years ago

And can you point what did you expect and so, what is buggy exactly? I see that your icon is not used as weel as the zoom.

egeloen commented 7 years ago

For the zoom and marker icon, it does not work this way. You need to configure the zoom on the map as you would configure it for "dynamic" map via Map::setMapOption('zoom', 5). For the icon, you need to add it on your marker with Marker::setIcon. The static option is here for options which are not shared between "dynamic" and "static" rendering.

For example, you don't need to configure the center static map option if you configure the map center via Map::setCenter but if you want to use an address for the center, you must use the center static option with an address as string because such option does not exist for "dynamic" map.

luizcesard commented 7 years ago

@egeloen, setting zoom and icons via the "dynamics" settings, it works indeed, but it does not apply for the map center... I've set the map center via Map::setCenter as well, but is wasn't generated on the final url.

egeloen commented 7 years ago

Hum, according to https://github.com/egeloen/ivory-google-map/blob/master/src/Helper/Subscriber/Image/CenterSubscriber.php#L44-L48 it should work except if your map uses auto zoom.

luizcesard commented 7 years ago

Yeah. It does works on dynamic maps, but not on static ones.

On May 20, 2017 16:11, "Eric GELOEN" notifications@github.com wrote:

Hum, according to https://github.com/egeloen/ivory-google-map/blob/master/ src/Helper/Subscriber/Image/CenterSubscriber.php#L44-L48 it should work except if your map uses auto zoom.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/egeloen/ivory-google-map/issues/237#issuecomment-302892739, or mute the thread https://github.com/notifications/unsubscribe-auth/AWkKZpc6Wv6DAz3_StulObcg4pYC3Qt6ks5r7zrEgaJpZM4Ndz1d .

egeloen commented 7 years ago

Sorry but I'm not able to reproduce your issue, If I exec:

$map = new Map();
$map->setCenter(new Coordinate(1, 1));

$this->renderMap($map);

The generated url is: https://maps.googleapis.com/maps/api/staticmap?center=1%2C1&size=300x300&zoom=3

Aren't you relying on autozoom somewhere? (Map::setAutoZoom(true) disables the center feature in favor of the visible feature for static map).

luizcesard commented 7 years ago

Well, that's really strange. I'm not setting autozoom anywhere, just setCenter, but the generated url doesn't have a "center" parameter, as if the setCenter method had no functionality.

egeloen commented 7 years ago

Can you share with me a very small reproducible case which fails that I can debug?

luizcesard commented 7 years ago

@egeloen, I prepared a standalone piece of code to show you and, guess what, it worked as it was supposed to. Then I understood the point: in my code, firstly I 'set center' via the setStaticOption method and then added the setCenter method at the beginning of the code while still calling setStaticOption('center',Coordinate(...,...)). After I dropped the call to this method, the former is perfectly working. Is this the supposed way it works? Why setting center via setStatic overrides the setCenter and neither of then show up in the final url?

egeloen commented 7 years ago

The issue is the static map helper uses the center static option for center address (string which is not supported in "dynamic" map) whereas the Map::setCenter supports center as coordinate (Coordinate object).

In the subscriber which converts the center into an http query string param, the center static option always win if it is given.

If you put a Coordinate object as center static map option, when passing in http_build_query, it seems PHP removes it automatically.

luizcesard commented 7 years ago

But according to the docs, the center static method could receive a Coordinate object as parameter.

egeloen commented 7 years ago

Hum, very good point :) Not sure what to do then, fix the doc or fix the code... The center is a bit special since it is supported semi-fully by the "dynamic" map... Personally, I would prefer let the code as it is and fix the doc because static options have been designed from the beginning to only deal with options which are not supported by "dynamic" map.

luizcesard commented 7 years ago

Hum, IMHO having two related methods that receive different parameters (one gets string and Coordinate and other gets only string) is a bit counter-intuitive and error prone, but is acceptable. However what I don't get as acceptable is that a method that supposedly accept only string remain silent when an object is given as parameter. I mean: it's OK to fix the docs, but setStaticOption('center', string) should throw an Exception if a Coordinate is given. Don't you think?

egeloen commented 7 years ago

I agree with you, let fix this part of the code

egeloen commented 7 years ago

I just fixed the center static map option in master. I have chosen to support Coordinate object as static map option as the code is more intuitive this way. Something else?

luizcesard commented 7 years ago

It's perfect! Nothing else. Everything working as a charm. Thank you very much for the effort and keep on the amazing job.