[r1-dev] Get map options from scope, instead of parsing HTML-embedded JSON

[dmitrybelyakov]

No description provided.

[nmccready]

Waiting on Laplante on this one as I don't fully know its complications. This should not affect any of my major changes to Marker,Window,Markers, and Windows directives.

[dmitrybelyakov]

Ok, no problem. It shouldn't have any complications unless you use json in html attributes in concrete usage cases. The travis build seems to pass and demo works well with all the markers and windows.

[nmccready]

Have you run the example with this as well and it all works?

[dmitrybelyakov]

Yes, the example demo works.

[Julion]

I've been using this changeset sucessfully on my project, any progress on merging?

[nmccready]

It is not that this can not be merged. I am waiting on @nlaplante to get back. He has been on a sweet vacation.

[nmccready]

Please fix this with the new api changes. @nlaplante is this something you are ok with? With regards to opting out of the JSON stuff?

[dmitrybelyakov]

Please fix this with the new api changes.

Sorry did you mean this must be fixed somehow?

[nmccready]

What I mean is pull the latest down from nlaplante:r1-dev into your dmitrybelyakov:r1-dev. Fix the conflicts and post back. The conflicts are usually in example.html, grunt file, and wherever you made changes :) .

[nmccready]

Before you go through the trouble I'd wait for @nlaplante 's response.

[dmitrybelyakov]

@nmccready no trouble at all - just to resolve a one-line conflict.

Yet I can see there are some problems with current state of r1-dev.
First I thought it was my merge but the nlaplante:r1-dev has the same error.
I'm getting "scope is not defined" on line 435.

Does the nlaplante:r1-dev example work well for you?

[dmitrybelyakov]

@nmccready And two more things I found:

Could we wrap this thing into some sort of if(window.google), otherwise it explodes if we don't have google loaded on every page meaning if there are pages that don't use the directive and thus do not include google it throws an exception.

There's also something wrong with the minified version.
I tried to ngmin&minify it myself, but to the same result.

[nmccready]

We could wrap it in the window.google or just google, but I would rather know about the error. If they are not loading the googlle_maps.js prior to our stuff; I would expect it to explode. Personally I'd rather see red then nothing.

[dmitrybelyakov]

@nmccready I meant to say let's wrap just this one call. Because when you are not in a single-page app it doesn't make sense to include google if you do not intend to use it on a particular page. You will still get red if you are actually using the directive without google. We just don't need it to initialize the angular-google-maps module.

[dmitrybelyakov]

github seems crazy today :)

[nmccready]

Yes I couldn't even look at the code you mentioned cause of the DDos
attacks. This is rather annoying, they need to change service providers to
have a autonomous back end that protects against it.

On Fri, Aug 16, 2013 at 10:04 AM, Dmitry Belyakov
[email protected]:

github seems crazy today :)

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/80#issuecomment-22767479
.

[nmccready]

I fixed this error last night btw on the scope issue.

On Fri, Aug 16, 2013 at 2:12 AM, Dmitry Belyakov
[email protected]:

@nmccready https://github.com/nmccready no trouble at all - just to
resolve a one-line conflict.

Yet I can see there are some problems with current state of r1-dev.
First I thought it was my merge but the nlaplante:r1-dev has the same
error.
I'm getting "scope is not defined" on line 435https://github.com/nlaplante/angular-google-maps/blob/r1-dev/dist/angular-google-maps.js#L435
.

Does the nlaplante:r1-dev example work well for you?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/80#issuecomment-22749337
.

[nmccready]

@nmccready I meant to say let's wrap just this one call. Because when you are not in a single-page app it doesn't make sense to include google if you do not intend to use it on a particular page. You will still get red if you are actually using the directive without google. We just don't need it to initialize the angular-google-maps module.

I got you, ok this does make sense. Also it would be wise to not load out js page if you are not using it ;) .

[nmccready]

Add the if(window.google) item to a new pull or issue. If you want to take care of it by all means.

[nmccready]

@nlaplante can we merge this ticket to use scope options instead of json? I am about to add this to Window and Windows as well. I should add this to marker and markers too.

[solidspark]

+1 for options from scope. The attr version breaks when using arrays within arrays or if you use a google.maps object.

Sent from my iPhone

On Aug 16, 2013, at 10:21 AM, nmccready [email protected] wrote:

@nlaplante can we merge this ticket to use scope options instead of json? I am about to add this to Window and Windows as well. I should add this to marker and markers too.

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

[nlaplante]

Yeah going with scope props is OK with me.

[nmccready]

@nlaplante we need to figure out how to get grunt added to travis for build quality checks.

The following needs to be fixed for this pull

Running "jshint:all" (jshint) task
Linting src/js/directives/map.js...ERROR
[L94:C23] W075: Duplicate key 'events'.
                events: '=events',           // optional
Linting src/js/directives/map.js...ERROR
[L96:C17] E020: Expected '}' to match '{' from line 86 and instead saw 'bounds'.
                bounds: '=bounds'
Linting src/js/directives/map.js...ERROR
[L96:C23] E020: Expected '}' to match '{' from line 57 and instead saw ':'.
                bounds: '=bounds'
Linting src/js/directives/map.js...ERROR
[L96:C24] W033: Missing semicolon.
                bounds: '=bounds'
Linting src/js/directives/map.js...ERROR
[L96:C25] W030: Expected an assignment or function call and instead saw an expression.
                bounds: '=bounds'
Linting src/js/directives/map.js...ERROR
[L96:C34] W033: Missing semicolon.
                bounds: '=bounds'
Linting src/js/directives/map.js...ERROR
[L102:C23] E020: Expected ']' to match '[' from line 31 and instead saw ':'.
            controller: ['$scope', function ($scope) {
Linting src/js/directives/map.js...ERROR
[L102:C25] W069: ['$scope'] is better written in dot notation.
            controller: ['$scope', function ($scope) {
Linting src/js/directives/map.js...ERROR
[L102:C34] E020: Expected ']' to match '[' from line 102 and instead saw ','.
            controller: ['$scope', function ($scope) {
Linting src/js/directives/map.js...ERROR
[L102:C36] W116: Expected ')' and instead saw 'function'.
            controller: ['$scope', function ($scope) {
Linting src/js/directives/map.js...ERROR
[L102:C53] W033: Missing semicolon.
            controller: ['$scope', function ($scope) {
Linting src/js/directives/map.js...ERROR
[L109:C14] E030: Expected an identifier and instead saw ']'.
            }],
Linting src/js/directives/map.js...ERROR
[L109:C14] W030: Expected an assignment or function call and instead saw an expression.
            }],
Linting src/js/directives/map.js...ERROR
[L117:C17] W033: Missing semicolon.
            link: function (scope, element, attrs) {
Linting src/js/directives/map.js...ERROR
[L117:C17] W116: Expected '(end)' and instead saw ':'.
            link: function (scope, element, attrs) {

Warning: Task "jshint:all" failed. Use --force to continue.

The short of it remove the duplicate event and add a comma after options before bounds.

[dmitrybelyakov]

Ok, got some time this morning to pull latest updates from r1-dev. So all lint errors are now fixed. Added map options to example as well.

Should be good to merge.

[nmccready]

I will merge this first and then my new pull. Merging may be fun with the changes to the GruntFile....