Additional Attributes Example Error: navigationMode must be 'css-transforms' or 'classic'.

[tomwayson]

Only appears to happen when linking to that example after viewing other examples, does not appear to happen when you go directly to the example page (http://esri.github.io/angular-esri-map/#/examples/additional-attributes).

[jwasilgeo]

Hey @tomwayson I don't mind trying to take a stab at this one if you aren't already. I thought I saw this too, but at the time wasn't sure if it was something temporary I was experiencing while focusing on another task. I wonder what's going on with the navigationMode property, or if something more sinister is happening to the map.

[tomwayson]

All yours. I don't think it's anything sinister, just something in the way that the scope is bound to that attribute.

[jwasilgeo]

$attrs.navigationMode (and the other map attributes) are not compiled/resolved properly in the esriMap directive's controller method when the user comes back to this example page for a 2nd view. It is still {{map.navigationMode}}. I've experimented some with this, and there's a few things to note:

• I wonder if the router is affecting the timing of directive compilations. You might recall my demo to you of showing how map(s) get hosed when returning for a 2nd view via the ngRouter.
• In the esriMap directive's controller method, the Dojo require appears to create a delay long enough to get $attrs to go from "{{xyz}}" to "some valid value", but only when the user views this sample page for the 1st time. You can stop prior to, and then within the require, to see the difference.
• In the esriMap directive, when experimenting with a link method instead of a compile method, attrs with the correct value are available, but still after the controller method had attempted to work with $attrs. Thus, on coming back to this sample for a 2nd viewing, our Errors are still thrown prior to $attrs.navigationMode being "classic". Not exactly related, but I found this 1yr old angular discussion interesting; check out the plunkrs in the description and in the first comment: Error when controller is specified at the same level of directive angular/angular.js#8043
• If we just do navigation-mode="classic" for all of our additional map attributes in the html template, then it appears we luck out regardless. That would appear to be the quickest fix. 😕

[tomwayson]

Nice detective work @jwasil!

So it looks like the fundamental problem is that the current way we're reading the map options attributes actually fails for expressions - it's just that we don't notice the first time b/c they're not read until require() returns - which by chance is after the expression has been evaluated.

As a short term fix, you're right, we can use string literals for all of these attributes.

As a long term fix, I want to be able to support both literal values or expressions. In the case of expressions - it's not b/c these are scope properties that are going to change, but just in case they are coming from a config file or something. I don't know the best way to accomplish this though.

One way would be to bind them to the isolated scope (i.e. follow the pattern used by the basemap attribute). For example, the following code works for navigationMode (but still fails for subsequent expressions that are read directly from $atts:

            // isolate scope
            scope: {
                // two-way binding for center/zoom
                // because map pan/zoom can change these
                center: '=?',
                zoom: '=?',
                itemInfo: '=?',
                // one-way binding for other properties
                basemap: '@',
                navigationMode: '@',
                // function binding for event handlers
                load: '&',
                extentChange: '&'
            },

// ... then below in directive's controller


                        if ($scope.navigationMode) {
                            if ($scope.navigationMode !== 'css-transforms' && $scope.navigationMode !== 'classic') {
                                throw new Error('navigationMode must be \'css-transforms\' or \'classic\'.');
                            } else {
                                mapOptions.navigationMode = $scope.navigationMode;
                            }
                        }

It's just like basemap only w/o the subsequent $scope.watch() to update the map when the attribute changes. Originally, I was thinking of only using scope properties for attributes that can change during the lifetime of the map, but I suppose they could be used for these "one-time" attributes that are only used during map load (and to my knowledge, none of these options are things we can/want to support changing after the map loads).

Alternatively, it seems like the other way to be able to read an attribute as either an expression is to use $attrs.observe() (see example 2 from this gist), however, I don't think that will work here, b/c we'd need to observe multiple attributes and have to deal w/ multiple async callbacks - whi

Only other thing I can think of is to have a map-options attribute that would take a JSON hash of the option poperties as either a reference from the parent scope or an inline JSON string (see example 7 from this gist). So either:

  <esri-map map-options="map.options" ...

or:

  <esri-map map-options="{ minZoom: 10, navigationMode: 'classic', resizeDelay: 500}" ...

I actually like that directive API better than the current one, and I think it would make the directive's controller code much sparser and easier to grok/extend. However, I'm not super keen on it being 2 way bound to the scope for something that we're only going to read once and forget about.

I don't know why, I'm just really resistant to add properties to the scope if they're not something we're going to actually watch for changes, but maybe that's just a hangup that I need to get over.

Does anyone know a better way to take in attribute values as either a literal or an expression?

Thoughts @jwasil, @eas604?

[eas604]

I really like the JSON syntax with a map-options attribute. As you said, it's very simple to understand and won't clutter the attributes.

[tomwayson]

Cool!

Actually, looking at example 4 on that gist it seems like that could be achieved w/ just $scope.$eval() and no two-way data binding (so I would not have to get over my hangups 😄). It says "one can combine this with interpolation, if the configuration is a JSON string" - which I guess means it might look like:

<esri-map map-options="{{ JSON.stringify(map.options) }}" ...

Don't know if you can call JSON.stringify() in an expression, but I guss you could do the following in your controller:

 $scope.mapOptions = JSON.stringify({...});

And then in the markup

<esri-map map-options="{{ mapOptions) }}" ...

[eas604]

You indeed have to do JSON.stringify() in the controller rather than the expression.

[tomwayson]

OK, good to know.

I don't think that's a big deal b/c this is all just one-time config. Any option that we care about changing will be handled like basemap, center, and zoom - specified as their it own attributes that are bound to the scope and watched for changes.

[jwasilgeo]

Nice--I like where this is going. @tomwayson that gist is a great reference that I should bookmark!

Do we agree (and do I follow) to add mapOptions to our isolate scope for these 1-time map "config" properties, and then do JSON.stringify(), and finally $scope.eval()? Then, work that into the map properties?

If that sounds good I'd be happy to commit a draft on a branch and get some feedback.

[tomwayson]

Agreed on all counts except the part about adding mapOptions to the isolated scope - that's not needed. We'll just read them in using $scope.eval() effectively replacing this line with:

  var mapOptions = $scope.$eval($attrs.mapOptions) || {};

Or something like that (whatever you need to do to handle the case where no map-options attribute is supplied).

Note how that should come before all the code that tries to read in the center/zoom/basemap - the values in those scope-bound attributes should override any duplicates supplied in the options hash (we'll want to document that).

Beyond that you would need to modify the validation code on certain options (like navigationMode) to operate on the options object instead of the individual attributes, and pretty much remove all the other code that reads in options from the individual attributes.

Would love it if you could take a stab at this.

[jwasilgeo]

Awesome. All that sounds good. I'm on it!

[jwasilgeo]

While I'm working with existing $attrs, @tomwayson @eas604 do y'all know the history behind the extent attribute? See also #39 (comment).

I haven't yet found an example of this being used here live, and am in the process of deciding to either finding a safe way to bring it in or perhaps cut it. We could add it to the isolate scope for those times someone wants to use it with a legit JSAPI Extent object.

Which leads me to a more general question, do we want to start supporting any other optional map construction attributes that aren't simple JS types? @eas604 already did a solid job of putting together strings, booleans, and numbers. I know we don't want to set out to recreate the JSAPI, but for the sake of $attrs.extent, I thought I'd ask.

[eas604]

I don't know the history (predates my contributions), but it looks like it was added for convenience but never really used as an attribute.

[tomwayson]

At this time I want to stay focused on just handling the attributes that are already implemented. Ultimately I think it would be great to support all the map options, and maybe we can open up a separate issue for that. For now I just want to get this bug closed so we can merge the outstanding pull requests.

The extent attribute came from when I merged two codebases originally. One was based on extent and the other was based on center and zoom. It deserves some extra consideration. I want to make sure users are able to create a map in any projection, but again, that could be handled in a separate issue. Just leave that code as is for now. It can be handled in a separate issue. Just leave that code houses for now.

Sent by Outlookhttp://taps.io/outlookmobile for Android

On Thu, Aug 20, 2015 at 6:30 AM -0700, "Edwin Sheldon" <notifications@gh.neting.ccmailto:notifications@github.com> wrote:

I don't know the history (predates my contributions), but it looks like it was added for convenience but never really used as an attribute.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/65#issuecomment-133008159.

[jwasilgeo]

Thank you both for the information and context. I'll leave $attrs.extent alone for now. I should be able to have something committed and perhaps a preliminary PR set up in the coming day or two.

FYI I haven't had to do any JSON conversion just yet. $scope.eval() is not something I have used before, but it seems like a great solution that is fixing the original crux of our issue with $attrs not being evaluated in time.

[tomwayson]

Whoa - hold the phone! I just realized that example 7 from the above gist is using function binding, not 2-way binding. That's kind of wild. While it is slightly less intuitive for me as a dev (as I expect function binding to be used for events), I think it would be better for users of the directive (they'd get to ditch the JSON.stringify() and string interpolation {{}}).

I'm leaning towards using function binding instead of $scope.$eval. What do you guys think?

BTW - by passing the map options as JSON, regardless of which pattern (4 or 7) we use, I think we'll basically get support for all the other currently unsupported options for for free b/c we're just handing JSON over to the map's constructor. The only thing is that we're not validating them, but I don't know if we should be doing that anyway - that's the JSAPI's job.

[jwasilgeo]

@tomwayson interesting point re: option 7 from that gist. I had started on $scope.$eval and massaging the existing validation checks, but just now I tried out adding mapOptions to the isolate scope as function binding, and I am getting the exact same object accessible to me, and thankfully regardless of if it is the 1st or Nth time visiting this example page via the ngRouter.

I've been developing with this in the example template:

<esri-map ... map-options="{
                minZoom: 10,
                maxZoom: 15,
                resizeDelay: 500,
                navigationMode: 'classic',
                sliderOrientation: 'horizontal',
                sliderPosition: 'top-right',
                displayGraphicsOnPan: true,
                fadeOnZoom: false,
                logo: false,
                wrapAround180: false
            }" ... >

and either this (gist option 4):

// isolate scope
scope: {
    // two-way binding for center/zoom
    // because map pan/zoom can change these
    center: '=?',
    zoom: '=?',
    itemInfo: '=?',
    // one-way binding for other properties
    basemap: '@',
    // function binding for event handlers
    load: '&',
    extentChange: '&'
},
...
controller: function($scope, $element, $attrs) {
  // ... later in the dojo require, if not $attrs.webmapId
  var mapOptions = $scope.$eval($attrs.mapOptions) || {};
  // ... all that validation goodness
},

or this (gist option 7):

// isolate scope
scope: {
    // two-way binding for center/zoom
    // because map pan/zoom can change these
    center: '=?',
    zoom: '=?',
    itemInfo: '=?',
    // one-way binding for other properties
    basemap: '@',
    // function binding for event handlers
    load: '&',
    extentChange: '&',
    mapOptions: '&'
},
// ...
controller: function($scope, $element, $attrs) {
  // ... later in the dojo require, if not $attrs.webmapId
  var mapOptions = $scope.mapOptions();
  // ... all that validation goodness
},

Note: I still haven't encountered having to use JSON.stringify() for option 4, but again have only focused on that big old string in the html so far; I have not experimented with it being defined elsewhere yet.

Honestly, I could get behind either style. If we do option 7, I agree that it is slightly less obvious what is happening for developers looking at this, but that's nothing that a few good comments or documentation can't solve.

Regarding what you mentioned about JSAPI dealing with invalid incoming options on map construction, I have also been thinking twice about the validation we do on the angular side. I have no qualms with cutting it and letting JSAPI deal.

Should we move this over to an online meeting sometime? 😄

[jwasilgeo]

PS: for cutting validation, I meant anything other than the isolate scope overrides on mapOptions (e.g. $scope.basemap takes precedence over mapOptions.basemap)

[tomwayson]

Great!

Just to be clear - this also works when you don't supply the map options (i.e. you can run the other examples that don't include a map-options attribute?

If so, let's go w/ function binding (option 7) and just add a comment like this:

    // function binding for event handlers
    load: '&',
    extentChange: '&',
    // function binding for reading object hash from attribute string
    // or from scope object property
    // see Example 7 here: https://gist.github.com/CMCDragonkai/6282750
    mapOptions: '&'

Let's also cut the validation code for now and just pass the object to the JSAPI.

Once you have that, PR and we can decide if we want to try and deal w/ the extent issue w/in the context of this issue.

Thanks @jwasil.

[jwasilgeo]

@tomwayson yes good question, that wasn't fleshed out in the snippets above, but other example docs should run fine. Thanks, all, for your advice and input. PR #71 is finally up!