search

dustin10 / VichGeographicalBundle

A Symfony2 bundle which provides geographical features for ORM and ODM entities and object oriented javascript maps rendering.
104 stars 26 forks source link

added infoWindow capabilities #8

Closed carlossg00 closed 13 years ago

carlossg00 commented 13 years ago

Hi dustin,

This pull request adds a clickable infoWindow to mapmarkers. http://yfrog.com/kefbmp

infoWindow will be populated with info obtained from the method annotated as @Vich\GeographicalInfoWindow, same way GeographicalQuery works. If no annotation found, no infoWindow will be displayed.

/**

TODO: Markers are uniquely named using getId() method from the entity. Find a way to figure out the identifier field of the entity and use it to name them, or simply use an internal variable in the foreach loop. MapHeper line 100.

Hope you find it right and merge this PR.

dustin10 commented 13 years ago

I will look at this more in depth when I get a chance. At first glance though, I'm not sure I like the idea of having the presentation logic defined in the entity itself. It just feels dirty. Also, I don't like that it relies on the entity having a getId() method. What are your thoughts? I will think about this some more in the next few days.

carlossg00 commented 13 years ago

I have followed your approach of having geographical info defined in the entity, but you are right, having presentation logic in the entity isn't the best approach, but where else? I'll think about too. About having getId() method don't worry, it's as simple as replacing it by a new variable. I've already fixed.

dustin10 commented 13 years ago

Here is my go at this. It is located in a new branch titled info-window. Please comment and make suggestions.

Basically there is a class MapMarkerInfoWindowBuilder that builds a MapMarkerInfoWindow object by loading a twig template specified in the configuration (there is a default template provided). This object holds the content html of the info window. The entity is passed to the template so the user can do whatever they want. Instead of using the entity itself to tell us whether or not the map should show info windows, there is now a boolean property on the Map class itself. It is only implemented in the Google Maps renderer right now and only supports Twig.

carlossg00 commented 13 years ago

As usual Dustin, excellent work!! Everything makes sense & logic, much better approach than mine. I've already tested and works flawless!! I close this PR.

+1 merge.