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 :)

[nateabele]

@cboden Good call. The next version of Angular Google Maps actually does look much better. @nlaplante Any interest in expanding beyond Google Maps and incorporating some of the other elements above? I'd be happy to help.

[nlaplante]

Right now Google Maps is the only implementation in the project's scope. Much work has to be done to integrate the various apis of the other implementations like leaflet. Feel free to fork it though,

@nlaplante Any interest in expanding beyond Google Maps and incorporating some of the other elements above? I'd be happy to help.

[joshkurz]

Hello,

Here is a version that uses some layers as a heat map with a simple leaflet
directive.

Pretty simple, but very flexible directive. Not really much watching going
on here at the map level.

I do think that some would consider it wrong to define the layer functions
in the controller, because they manipulate the dom, which is why I figured
a layer directive would be suitable.

But anyways here is something new to look at. Maybe it will spark some
ideas.

http://plnkr.co/edit/JCmj2WoxSLP3108HWhuR?p=preview

Thanks
Josh Kurz

On Mon, Jul 15, 2013 at 12:20 PM, Nicolas Laplante <notifications@github.com

wrote:

Right now Google Maps is the only implementation in the project's scope.
Much work has to be done to integrate the various apis of the other
implementations like leaflet. Feel free to fork it though,

@nlaplante https://github.com/nlaplante Any interest in expanding
beyond Google Maps and incorporating some of the other elements above? I'd
be happy to help.

—

Reply to this email directly or view it on GitHubhttps://github.com//issues/15#issuecomment-20981169
.

Josh Kurz
www.atlantacarlocksmith.com http://www.atlantacarlocksmith.com

[MatthieuBarthel]

Hi,

I worked on the memory leak issue and came up with a proof of concept:
http://plnkr.co/edit/zxhJgOQEhPxPAX9AUCGs?p=preview

It simply reuse the same map instead of instantiating a new one every time and it definitely gives good results (about 4 times less memory after loading a map 200 times).

It would be awesome to see this technique implemented in your rewrite.

Do not hesitate to tell me if I can help.

[cboden]

ping @dylanfprice

Dylan has also started an Angular map implementation called AngularGM that is inspired by both ui-map and Angular Google Maps.

How does everyone feel about collaborating on this? I think it's too soon for this to happen. :)

[nateabele]

I'm down. If we can come together on goals and the basic API, let's do it.

[getsetbro]

Please do this.

[nateabele]

It's next on my list after I get a couple more releases of UI-Router out the door.

[nfroidure]

👍 This API looks nicer, hope to see it shipped soon :-)

[nfroidure]

By the way, instead of conditionnally load the entire application, a $gmap service could expose a promise indicating if the gmap API is available:

Something like:

angular.module('ui.map.services')
  .service('uiGmap', ['$q',function($q) {
    var _loadDefer = $q.defer();
   window.onGoogleReady = _loadDefer.resolve.bind(_loadDefer);
   document.write('<script type="text/javascript" src="https://maps.googleapis.com/maps/api/js?v=3.exp&sensor=false&callback=onGoogleReady"></script>'); // quick, don't hit
    return {
      loaded: _loadDefer.promise
    }
  }]);

And in directives:

angular.module('ui.map.directives', ['ui.map.services'])
  .directive('uiMap' ['uiGmap', function(uiGmap) {
    return {
      link: uiGmap.loaded.then.bind(uiGmap.loaded, function() {
        /// add maps here
      })
    }
  });

[nateabele]

That looks awesome. Sorry I kinda dropped the ball on this. Things have been pretty hectic and UI Router is keeping me pretty busy in my spare time, but I'm gonna try to come back to this in the next week or two so we can at least put together a roadmap and start delegating tasks.

[cboden]

See https://github.com/angular-ui/angular-google-maps