In Makefile:

> @@ -40,7 +40,7 @@ help:
>  all: prod dev lint test apache test/karma-conf-prod.js deploy/deploy-branch.cfg
>
>  .PHONY: prod
> -prod: prod/lib/build.js prod/style/app.css prod/index.html prod/mobile.html prod/info.json prod/layers.json prod/img/ prod/style/font-awesome-3.2.1/font/ prod/locales/
> +prod: prod/lib/build.js prod/style/app.css prod/index.html prod/mobile.html prod/info.json prod/img/ prod/style/font-awesome-3.2.1/font/ prod/locales/

The corresponding prod/layers.json target should be removed.

I saw that and changed it afterwards, it has been merged correctly.

I should also remove

# Temporary: the entire rule should go away eventually
prod/layers.json: src/layers.json
	cp $< $@

In src/components/permalink/PermalinkService.js:

> @@ -72,6 +72,10 @@
>      this.updateParams = function(p) {
>        angular.extend(params, p);
>      };
> +
> +    this.getLang = function() {
> +      return params.lang;
> +    };

The permalink service should not be the service returning the current
language. Use $translate.uses() instead.

I didn't like it either, thanks!


> -      ['$scope', 'gaGlobalOptions', function($scope, gaGlobalOptions) {
> -        $scope.options = {
> -          url: gaGlobalOptions.serviceUrl + '/FIXME'
> -        };
> +      ['$scope', 'gaPermalink', 'gaGlobalOptions',
> +        function($scope, gaPermalink, gaGlobalOptions) {
> +          $scope.options = {
> +            defaultTopicId: 'inspire',
> +            setActiveTopicId: function(topicId) {
> +              this.activeTopicId = topicId;
> +              this.updateUrl();
> +            },
> +            updateUrl: function() {
> +              this.url = gaGlobalOptions.serviceUrl + '/rest/services/' + this.activeTopicId +
> +                '/MapServer/layersconfig?lang=' + gaPermalink.getLang() + '&callback=' +
> +                'JSON_CALLBACK'

The topic controller should not be the one to know how to create
layersconfig URLs. The topic controller is responsible for the topics, not
for the layers themselves. For now, I'd say that you can just hard-code the
layersconfig URL template in the gaLayers service. Later, we can make that
template configurable, through the gaLayersProvider.


Ok, that makes perfect sense.

loadForTopic should just take a topicId. And the gaLayers service should be
the one responsible for creating the layersconfig URL. See my other
comments on that matter

ok

In src/js/TopicController.js:

> -      ['$scope', 'gaGlobalOptions', function($scope, gaGlobalOptions) {
> -        $scope.options = {
> -          url: gaGlobalOptions.serviceUrl + '/FIXME'
> -        };
> +      ['$scope', 'gaPermalink', 'gaGlobalOptions',
> +        function($scope, gaPermalink, gaGlobalOptions) {
> +          $scope.options = {
> +            defaultTopicId: 'inspire',
> +            setActiveTopicId: function(topicId) {
> +              this.activeTopicId = topicId;
> +              this.updateUrl();
> +            },
> +            updateUrl: function() {
> +              this.url = gaGlobalOptions.serviceUrl + '/rest/services/' + this.activeTopicId +
> +                '/MapServer/layersconfig?lang=' + gaPermalink.getLang() + '&callback=' +
> +                'JSON_CALLBACK'

So I think the only options that the topic controller should pass to the
topic directive are the topics URL (which was in the options previously)
and the default topic.

ok i'll change that.

Thank you for your comments and I'll fix it in a new pull request.


On Thu, Jul 25, 2013 at 3:30 PM, Gilbert Jeiziner
<notifications@github.com>wrote:

> In src/components/map/MapService.js:
>
> >                      ],
> >                      dimensions: {
> > -                      'Time': layer.timestamps[0]
> > +                      'Time': layer.timestamps[0],
> > +                      'Format': layer.format,
> > +                      'Layer': id
>
> That's what I was asking myself. But like this, ol is capable of replacing
> the corresponding string in the wmtsGetTileUrl string.
>
> We could do the replacement for format and layerid here...
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/geoadmin/mf-geoadmin3/pull/164/files#r5397391>
> .
>