-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
JGoogleEmbedMaps::addMarker with event(s) #6381
Conversation
I found that when adding a marker to the map it is not considered to bind events to it. Isn't it a tyical scenario to click on a marker to show a balloon with short info or a link to more details? The [Google Maps API](https://developers.google.com/maps/documentation/javascript/reference) presents as an example to create the marker and the create the desired event which expects to receive the marker as a parameter. I extended those functions responsible for marker creation by adding them another argument for the events. Also, the function creating the script was extended to evaluate the currently set markers not only for their options but also for events to bind. I tested this functionality by taking a view template and writing down the following: ``` // Target element echo '<h1>JGoogleEmbedMaps Test</h1>'; echo '<div class="row">'; echo '<div class="col col-xs-12">'; echo '<div id="map" style="height:500px; background:gainsboro; border:gray">LOADING ...</div>'; echo '</div>'; echo '<div class="col col-xs-12">'; echo '<h5>Monitor</h5>'; echo '<p id="monitor" style="padding:1em 0.5em; border:1px solid gainsboro; border-radius:3px"></p>'; echo '</div>'; echo '</div>'; // Config $options = new JRegistry(array( 'extrascript' => 'locationmap=map;', 'key' => '', 'mapclass' => 'mapclass', 'mapid' => 'map' )); // Get instance $google = new JGoogle($options); $map = $google->embed('Maps'); // Define load type $map->setAutoload('jquery'); $map->setMapType('HYBRID'); $map->setZoom('3'); $map->setCenter(array('49','10'), 'Center Marker', array( 'clickable' => true ), array( 'click' => 'function(e) { map.setCenter(this.getPosition()); document.getElementById("monitor").innerHTML = this.title; }' )); // Add markers $map->addMarker(array('52','10'), 'Test Marker', array( 'clickable' => true ), array( 'click' => 'function(e) { map.setCenter(this.getPosition()); document.getElementById("monitor").innerHTML = this.title; }' )); // Prepare callback $callback = <<<ENDSCRIPT (function($, gm, element) { $('#monitor').append('<em>Callback executed</em>') })(jQuery.noConflict(), window.google || {}, document.getElementById({$options->get('mapid')})); ENDSCRIPT; // Register custom script(s) $map->setAdditionalJavascript($callback); $map->echoHeader(); $map->echoBody(); ``` So far this works. I wonder why this hasn't been considered yet. Also, i am yet missing to pass events for the map itself. What do you think about this?
The tests appear to fail although the code works properly. Can somebody please tell me what causes the test to fail? Is there probably a change required in the test class? Mabe current tests don't consider the new arguments and therefore cause the test to fail? |
After adding additional function parameter to `setCenter()` and `addMarker()` the test have to be extended as well.
The tests fail because you have a PHP syntax error in your changed test file on line 360. You are doing a |
Thanks for pointing me to the issue. Finally its passing all tests. I'll add one more test case with a non-empty events-array. |
There we go. Extra test for empty or non-empty events array added and passed. |
Sorry i couldn´t test successfully. By using your code above with the patch nothing happens by click on the markers. This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6381. |
Of course it works. I tested it right this second via copy+paste the above example 1:1. You have to apply the patch to |
Oh sorry, don´t know how to apply something to JGoogleEmbedMaps. I think i tested it wrong. This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6381. |
After integrating the event binding to markers and fixing the corresponding unit tests i have now created another PR to add event handlers for map events. |
$setup .= substr(json_encode($options), 1, -1); | ||
$setup .= '});'; | ||
|
||
if (isset($marker['events'])) |
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.
You should test here, if it's an array:
if (isset($marker['events']) && is_array($marker['events']))
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.
Agree. I will update this line.
I find it a usefull addition, thanks! @test: Works fine for me. |
@Erftralle thanks for testing and your feedback. |
I have tested this item ✅ successfully on 7bb7f93 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6381. |
I have tested this item ✅ successfully on 7bb7f93 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6381. |
I have tested this item ✅ successfully on 7bb7f93 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6381. |
1 similar comment
I have tested this item ✅ successfully on 7bb7f93 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6381. |
RTC. Thanks This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6381. |
Thanks everybody. Merged into 3.5-dev with commit 269b781 |
I found that when adding a marker to the map it is not considered to bind events to it. Isn't it a tyical scenario to click on a marker to show a balloon with short info or a link to more details?
The Google Maps API presents as an example to create the marker and immediately create the desired event which expects to receive the marker created as reference to bind to. I extended those functions responsible for marker creation by adding them another argument for the events (find a PR for map events here). Also, the function creating the script was extended to evaluate the currently set markers not only for their options but also for events to bind.
I tested this functionality by taking a view template and paste the following:
So far this works. I wonder why this hasn't been considered yet. What do you think about this?