Rewrite

[nateabele]

Can I just get a quick show of hands on people who are, like, super-invested in the current implementation of this library? Would anybody's feelings be hurt if I submitted a PR with a complete rewrite? The problems as I see them:

This library is pretty un-Angular

• While I appreciate the idea behind making the GMaps API's native object front-and-center, one thing Angular emphasizes is not imposing on your domain model (hence why 'model' in Angular === $scope). Having access to these objects is nice, but they absolutely should not be forced in front of my face.
• Way too imperative: there's a lot of low-hanging fruit in terms of functionality that I should be able to implement with an attribute. See: Angular Google Maps: the code might be an atrocity, but at least they nail the simplicity for common use cases.
• Untestable: relies on global variables exposed by the GMaps API. This is a no-no.
• Poorly abstracted: I should be able to write adapters for Leaflet or OSM. I can't.

I could go on, and I'll grant that I'm obsessively perfectionistic, but hey, you gotta start somewhere. Thoughts and feedback are welcome. /cc @ProLoser

[ProLoser]

Dude, if you want I can let you take ownership. Andy Joslin wrote the
original version (and I've seen alternatives out there) and he's not really
actively maintaining anymore so you are more than welcome to give it a
whirl. That said, I don't use it so I can't exactly say that I am going to
be annoyed if the API changes, lol.

[ProLoser]

Actually, what is the issue with just kind of folding this project and deferring to @nlaplante's version instead?

[nateabele]

@ProLoser IMO @nlaplante's version suffers from the opposite problem: it's too restrictive, and doesn't really help you if you want to do anything outside the features it exposes. It also has some of the problems above, like testability issues (it actually has no tests), is also inherently tied into GMaps, and it does a few very non-idiomatic things (see the refresh attribute). It also has problems managing markers.

[ProLoser]

@nateabele I added you to the UI team. Go crazy.

Also, trivial note:

When I was originally building TinyMCE I actually wanted to make it ui-wysiwyg and let you kind of specify what module to use. I was sort of hoping the same could be done with the map directive. At least for the convenient-wrapped features. This way you could swap out the external lib without missing a beat (except perhaps if you chose to do enhanced customization by directly accessing the lib object, etc).

[nateabele]

@ProLoser Yeah, I haven't yet identified a good pattern for doing extensible, adapter-driven directives as yet. I'll let you know what I come up with.

[ProLoser]

I have. Look at leveraging 'requires'. I should probably make a screencast on how to best use it's awesome might.

[ProLoser]

uiMap = {
  controller: function($scope, $element, $attributes) {
    // Place code / methods that need to be cross-accessed in here
    this.init = angular.noop
    this.addMarker = angular.noop
    this.zoom = angular.noop
    this.pan = angular.noop
  },
  link: function($scope, $elm, $attrs, uiMap){
    // bind attributes and scope watches to controller methods
    $attrs.$observe('zoom', function(value) {
      uiMap.zoom(value);
    }
  }
}
uiMapGoogle = {
  requires: 'uiMap',
  restrict: 'AC',
  link: function($scope, $elm, $attrs, uiMap) {
    uiMap.init = function() { ... }
    uiMap.addMarker = fn() { ... }
    // etc...
  }
}

<div ui-map="mapObject" class="ui-map-google">
or
<ui-map ui-map-google="mapObject">

Of course the ="mapObject" and all the other code is optional and for demonstrative purposes.

[nateabele]

Good call, didn't think of that.

[nateabele]

My syntax idea was something along these lines (not exhaustive):

<ui-map><!-- Defaults to Google Maps --></ui-map>

<ui-map center="mapData.center" zoom="mapData.zoom">
    <!-- Google Maps, no object binding, but scope bindings for zoom and center -->
</ui-map>

<ui-map ui-map-leaflet center="mapData.center" zoom="mapData.zoom">
    <!-- Leaflet w/ no object binding, but scope bindings for zoom and center -->
</ui-map>

<ui-map
    ui-map-google="mapObject"
    center="mapData.center"
    zoom="mapData.zoom"
    draggable="false"
    autofit="true"
    options="{ ... }"
><!-- GMaps w/ API object binding, center, zoom, and extended options --></ui-map>

<!-- Map with marker objects -->
<ui-map ui-map-google="mapObject" center="mapData.center" zoom="mapData.zoom">
    <ui-marker ng-repeat="marker in mapData.markers" position="marker">
        {{ marker.info }}
    </ui-marker>
</ui-map>

The above assumes the following controller:

angular.extend($scope, {
    mapObject: null, // gets populated by the directive where applicable
    mapData: {
        zoom: 9,
        center: { latitude : 45.5081, longitude : -73.5550 },
        markers: [{
            latitude : 45.5081,
            longitude : -73.5550,
            info: "Some info about the marker"
        }]
    }
});

Using nested directives lets us strike a balance between imperative and declarative code and does a better job separating the bindings from the presentation. Also, this style of nesting directives makes it easier to support other types of map overlays such as paths.

[joshkurz]

Another feature that people would appreciate would be a layer directive that syncs to the map element.

So you would have your map rendered and then any number of layer directives could be added to the map.

Or if not a separate directive, then an api to allow for dynamic layers that call different dom functions from one of the many different layer libraries available today.

Thanks
Josh Kurz (mobile)

----- Reply message -----
From: "Nate Abele" notifications@github.com
To: "angular-ui/ui-map" ui-map@noreply.github.com
Subject: [ui-map] Rewrite (#15)
Date: Sat, Jul 13, 2013 00:31
My syntax idea was something along these lines (not exhaustive):

<ui-map
ui-map-google="mapObject"
center="mapData.center"
zoom="mapData.zoom"
draggable="false"
autofit="true"
options="{ ... }"

{{ marker.info }}

The above assumes the following controller:

angular.extend($scope, {
mapObject: null, // gets populated by the directive where applicable
mapData: {
zoom: 9,
center: { latitude : 45.5081, longitude : -73.5550 },
markers: [{
latitude : 45.5081,
longitude : -73.5550,
info: "Some info about the marker"
}]
}
});

Using nested directives lets us strike a balance between imperative and declarative code and does a better job separating the bindings from the presentation. Also, this style of nesting directives makes it easier to support other types of map overlays such as paths.

—
Reply to this email directly or view it on GitHub.

[nateabele]

@joshkurz Sounds interesting. Got any examples you can point me to?

[joshkurz]

Well i created a heat map directive that works with leaflet but it's closed source atm.

Its pretty sweet to see the layers working with angular databindings and I believe others could benefit from it.

I'll see if I can open source it on Monday and then show you a live example.

Thanks
Josh Kurz (mobile)

----- Reply message -----
From: "Nate Abele" notifications@github.com
To: "angular-ui/ui-map" ui-map@noreply.github.com
Cc: "Josh Kurz" jkurz25@gmail.com
Subject: [ui-map] Rewrite (#15)
Date: Sat, Jul 13, 2013 01:44
@joshkurz Sounds interesting. Got any examples you can point me to?

—
Reply to this email directly or view it on GitHub.

[cboden]

+1 for re-thinking this. I'm in dire need of a Google Map implementation for Angular in a project. I'm happy to help contribute with what I can.

Something to be noted about @nlaplante version is r1-dev looks to be a re-write with a better architecture.

[nlaplante]

Spot on!

Something to be noted about @nlaplante version is r1-dev looks to be a re-write with a better architecture.

[MatthieuBarthel]

Hi, I'm also in need of a Google Map implementation for Angular.

The biggest issue I have with Google Map is memory leaks, every time you instantiate a map it consumes memory that will never be released. When you develop an Angular one page application, it's a big concern. The only option I imagine is the instantiate a map and reuse it (if a second map is needed at the same time, then instantiate a second one).

@nateabele I would be happy to know what if you think about it and if you plan to take into account that issue :)