eschwartz / backbone.googlemaps

A Backbone JS extension for interacting with the Google Maps API (v3.10)
MIT License
139 stars 55 forks source link

mapEvents handlers should be bound to the object, not the map marker #19

Closed variousauthors closed 10 years ago

variousauthors commented 10 years ago
var CustomView = Backbone.GoogleMaps.MarkerView.extend({
  icons: {
    selected: "icons/map-marker.png"
  },

  mapEvents: {
    'mouseover': 'select'
  },

  select: function (e) {
    e.setIcon(this.icons.selected); // this.icons is undefined
  }
});

The code above doesn't work, and I've had to do some magic to get those icons in there (put the whole thing in an immediate function, scope the icons to the closure, etc...). I'm no JS wizard, but I think we should be able to do the following:

  select: function (e) {
    e.setIcon(this.icons.selected);
    this.trigger('application:event', this);
  }

Maybe I'm missing something, but as it stands it seems like I am going to have a hard time getting my app to know that a marker event has occurred.

Would it be too much to ask that the event handlers be bound to the object, rather than whatever it is they are bound to now (the gOverlay, I guess?). If not, I'm open to suggestions of best practices to get around this (as I said: maybe I've missed something you intended).

variousauthors commented 10 years ago

I noticed in the code you have

    constructor: function() {
      GoogleMaps.MapView.prototype.constructor.apply(this, arguments);

      _.bindAll(this, 'render', 'close', 'openDetail', 'closeDetail', 'toggleSelect');

      // Add default mapEvents
      _.extend(this.mapEvents, {
        'click' : 'toggleSelect'                            // Select model on marker click
      });

    toggleSelect: function() {
      this.model.toggleSelect();
    },

So you do bind this to some mapEvents, but not the user defined ones. Would it suffice to loop over the user provided mapEvents and bind them?

eschwartz commented 10 years ago

That should do it for you. You can also now call:

myView.bindMapEvents({
  click: function() { 
    // do something
  }, myView);

to bind map events in any context.

Sorry again for the slow response.

variousauthors commented 10 years ago

Thanks, awesome!