Skip to content
This repository has been archived by the owner on Nov 30, 2018. It is now read-only.

[r1-dev] Feature/many markers issue 69 #83

Merged
merged 50 commits into from
Aug 15, 2013
Merged

[r1-dev] Feature/many markers issue 69 #83

merged 50 commits into from
Aug 15, 2013

Conversation

nmccready
Copy link
Contributor

This is a major change and I am expecting some flak for moving to CoffeeScript. However, the performance improvements to not using the DOM as much for Markers and Windows is improved. There are areas for improvement, but the discussion for what to do next needs to start.

The areas for improvement specifically are in Windows and Markers "models" watchers. I am currently throwing in a size check (newValue.length != oldValue.length) as not rebuild the models when it is not necessary. Also the new api is more complicated, but the goal was to force some common functionality on common directives and model relationships.

nmccready added 30 commits July 23, 2013 09:54
a marker defines watches. However certain watches are enforced, along
with required properties.
- fixed destroy bug in marker.coffee
- hashing out Markers some more with MarkerModel, commented out as there
  is a bug currently
- IWindow
- Window
     .models
      - WindowModel
      - WindowModelFunctions

Windows to come
… -> and not => as no self references needed.[C
…s scope. This way Windows can listen in on Markers models to know when to clear its models.
…hey all share one Logger from directives.api.util.Logger which can turn on and off info logging by doLog flag.
@facultymatt
Copy link
Contributor

Hmm, let me try to explain it better.

I currently have this setup:

<marker ng-repeat="m in markers" coords="m" icon="m.icon">
    <window show="m.showWindow" closeClick="m.closeClick()">
        <p>{{m.name}}</p>
        <p>{{m.address}}</p>
    </window>
</marker>

@note that I am storing more in my markers than just their coords.

Question is, how would I convert this to utilize the new format you created:

<markers models="markers" coords="'self'" icon="'icon'" click="'onClicked'">
    <windows show="'showWindow'" closeClick="'closeClick'">
        <p ng-non-bindable >This is an info window at {{ latitude | number:4 }}, {{ longitude | number:4 }}!</p>
        <p class="muted">My marker will stay open when the window is popped up!</p>
    </windows>
</markers>

Does this make it more clear?

@nmccready
Copy link
Contributor Author

As long as it is in your model you should be able to get to it.
The logic is here in MarkersParentModel

        setContentKeys:(models)=>
            if(models.length > 0)
                @contentKeys = Object.keys(models[0])

@nmccready
Copy link
Contributor Author

Ahh

<markers models="markers" coords="'self'" icon="'icon'" click="'onClicked'">
    <windows show="'showWindow'" closeClick="'closeclick'"> 
        <div ng-non-bindable>
                <p>{{name}}</p>
                <p>{{address}}</p>
        </div>
     </windows>
</markers>

@nmccready
Copy link
Contributor Author

You wrap whatever you want in ng-nonbindable as the interpolation is being put off to be done by the WindowParentModel for each child.

@facultymatt
Copy link
Contributor

Got it, thanks for the help!

On Thu, Aug 15, 2013 at 1:10 PM, nmccready [email protected] wrote:

You wrap whatever you want in ng-nonbindable as the interpolation is being
put off to be done by the WindowParentModel for each child.


Reply to this email directly or view it on GitHubhttps://github.com//pull/83#issuecomment-22714407
.

Matt Miller
Creative Developer
m *609.335.4417
*twt
@mattmillerart http://twitter.com/#!/mattmillerart

faculty
twt @FacultyCreative http://twitter.com/#!/facultycreative
w www.facultycreative.com

2424 E. York Street
Suite 233
Philadelphia Pa. 19125

@nmccready
Copy link
Contributor Author

np

@nmccready
Copy link
Contributor Author

Guys.. I just had an epiphany!

<window ng-repeat="a in activeMarkers" show="a.showWindow" closeClick="a.closeclick" coords="a"> 
       <p>{{name}}</p>
       <p>{{address}}</p>
</window>
<markers models="markers" coords="'self'" icon="'icon'" click="'onClicked'">
</markers>

This eliminates the need for Windows unless you really want all of them loaded in memory ... why? This is dynamic as long as a controller somewhere has an activeMarkers array. This also could eliminate the logic for having window as a child of a marker directive. I think this is a good idea cause it eliminates complexity on both sides.

BTW there is a bug if you do not use ng-repeat in Window. This bug is only if you expect the html content to change in a Window for the {{}} side of things. If you do not use ng-repeat the content is not re-evaluated if the model changes. See Below.

//assume marker is one activeMarker that we keep track of in a controller
<window show="marker.showWindow" closeClick="marker.closeclick" coords="marker"> 
       <p>{{marker.name}}</p>
       <p>{{marker.address}}</p>
</window>

With that above scenario the Window will move and show as it is supposed to. BUT the html will not get evaluated by angular. If you use ng-repeat it forces it the redo the html! This is how I came to the top solution.

@nmccready
Copy link
Contributor Author

Matt what is your twitter?

@solidspark
Copy link
Contributor

This is all very interesting. Just reviewed the code. I've got a many-markers app that uses direct api calls instead of the r1-dev marker directive, and it's working fine. However, would like to branch off and then build an implementation that would use the directive and test it.

Nice work, @nmccready !

@nmccready
Copy link
Contributor Author

It is still got some weight to it as I would like it to be faster. The direct api calls are still way faster. I need to see what Matt sent me earlier.
@facultymatt commented 6 hours ago
@nmccready there is a nice example here, "lots of markers" http://embed.plnkr.co/PYDYjVuRHaJpdntoJtqL/preview and this is the repo https://github.com/dylanfprice/angular-gm

@facultymatt
Copy link
Contributor

@nmccready find me on twitter @mattmillerart

@facultymatt
Copy link
Contributor

@nmccready I like the thinking behind <window ng-repeat="a in activeMarkers"></window> seems like it would be much better resource wise.

@facultymatt
Copy link
Contributor

@nmccready any progress on the window functionality? Not sure if 1ee4fab addressed it...

@nmccready
Copy link
Contributor Author

No I am not sure how we should proceed with my recommendations above. This is because this is at a high level where a controller that contains activeMarkers is created by some one using our library. I am not sure if we should make a basic GoogleMapsController available or not. Due to the fact that this would force functionality on the user.

The commit from above is trying to make MarkerChildModel slimmer. I also think I can eliminate the watches for icon, coords and maybe more in MarkerChild model. They all seems to be caught in the watch 'model' .

I will experiment some more, but it could be due to watch using the deep watch argument being set to true.

@nmccready
Copy link
Contributor Author

I just switched up some logic in MarkerChildModel to use watches much less. This results in less traverses in angular.

@nmccready
Copy link
Contributor Author

Matt,
In AngularGM I like how he uses:

      scope: {
        gmObjects: '&',
        gmGetLatLng: '&',
        gmGetMarkerOptions: '&',
        gmEvents: '&'
      },

// used later
 var latLngObj = scope.gmGetLatLng({object: object});
          var position = objToLatLng(latLngObj);

This can get rid of the whole key stuff that is in our markers currently. As this allows the expression to be evaluated on our terms.

@facultymatt
Copy link
Contributor

I don't particularly like how the author uses:

<gm-markers gm-objects="volcanoes"
    gm-get-lat-lng="{ lat: object.location.lat, lng: object.location.lng }"
    gm-get-marker-options="{ title: object.name }"
    gm-on-click="selectedVolcano = object; infoWindow.open(marker.getMap(), marker);">
</gm-markers>

The gm-on-click code reminds me of how AngularJS bootstrap / ui maps worked. It's not the most elegant... although I suppose it does work... but I believe you guys are onto something more elegant here.

@nmccready
Copy link
Contributor Author

Yeah I'm not alluding to changing the click behavior, but just how we get to the model objects properties. It would be nice to have some of the attrs be able to grab from the parent scope or controller. This would be instead of being locked to model for each attribute.

@richardszalay
Copy link

@nmccready I'm using the above "separate window" recommendation, but the window is appearing over the (custom) icon. Will I have to do the offset myself or is this solvable in another way?

@nmccready
Copy link
Contributor Author

Use window options to set the offsets. It is documented in the google sdk for infowindow options

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants