Missing schema file

@tjwelde

thanks for the patch - it looks like a good thing to do.

I have a couple of comments:

1) The addition of SafeMarkup::checkPlain($address_value) is redundant as twig auto escapes variables by default.

2)As a way of double checking the patch ... I looked for a one-to-one correspondence between your list in

config/schema/simple_gmap.schema.yml

and the list in SimpleGMapFormatter::defaultSettings()

The following are missing:

static_scale
apikey

  public static function defaultSettings() {
    return array(
      "include_map" => "1",
      "include_static_map" => "0",
      "include_link" => "0",
      "include_text" => "0",
      "iframe_height" => "200",
      "iframe_width" => "200",
      "static_scale" => 1,
      "zoom_level" => "14",
      "link_text" => "View larger map",
      "map_type" => "m",
      "langcode" => "en",
      "apikey" => "",
    ) + parent::defaultSettings();

I reviewed patch below patch file. Its working fine.
simple_gmap_schema

@geekinpink thank you, I will take that as an RTBC.

I will take a close look and hopefully commit soon....

On final review I find that I have an unanswered question, I hope it is a simple matter of correcting a misunderstanding I may have.

As it says in the issue summary there is a tool useful in debugging schemas

When I try and use it to check the schema - I do not see things as I expect.

so with simple_map and config_schema enabled I visit

admin/config/development/configuration/inspect

when I search for our new config_key [field.formatter.settings.simple_gmap] and I fail to find it ....

Am I doing something wrong .....

Have we made a error in the schema definition such that the system is not recognising it?

We should not have any config files, only config schema, in the config directory of this module. I don't know anything about the tool, but if it is looking for something in config/install or config/optional, it isn't going to find anything, and it shouldn't.

What should happen is that if you have a site with a field using our formatter, that should create a config entry for that field formatter... Which I think is part of the display mode, not its own config item.

I also reviewed the patch and there are quite a few labels that need some work:

1. +++ b/config/schema/simple_gmap.schema.yml
   @@ -0,0 +1,43 @@
   +  label: 'Link format settings'
   

   This line is incorrect. This was probably copied from a Link field? It should say something like settings for the simple google map field.

2. +++ b/config/schema/simple_gmap.schema.yml
   @@ -0,0 +1,43 @@
   +      label: 'Whether to include a text with the map'
   

   a text => text

3. +++ b/config/schema/simple_gmap.schema.yml
   @@ -0,0 +1,43 @@
   +      label: 'Height of embedded map'
   

   wrong label, this is the zoom level

4. +++ b/config/schema/simple_gmap.schema.yml
   @@ -0,0 +1,43 @@
   +      label: 'The default type of map to be displayed'
   

   not really "default"?

5. +++ b/config/schema/simple_gmap.schema.yml
   @@ -0,0 +1,43 @@
   +      label: 'The language, in which the map should be displayed'
   

   comma should be removed

Sorry, item 3 above isn't clear. Also I found one other problem:

1. +++ b/config/schema/simple_gmap.schema.yml
   @@ -0,0 +1,43 @@
   +      type: integer,
   

   This seems to be a typo, I don't think it ends in ,

2. +++ b/config/schema/simple_gmap.schema.yml
   @@ -0,0 +1,43 @@
   +    zoom_level:
   +      type: integer
   +      label: 'Height of embedded map'
   

   And here's the context on item 3 from the previous comment.

Nice work and very good detailed review!

I have addressed everything from #8 and #9. Patch and interdiff provided.

@jhodgdon regarding zoom level label (#8.3 and #9.2), I have googled a bit and I didn't find any more descriptive label for it, looks like "Zoom level" is sufficient. Also same label is used in formatter settings form.

Sorry I wasn't clear. The problem was that the Zoom Level item in the schema had a description saying 'Height of embedded map', which you have fixed -- the new description of "Zoom level" also seems fine to me.

Now, one more thing -- sorry for missing this earlier!

There is a distinction in schema files between text that can be translated using Config Translation (type: label) and text that cannot be translated (type: string). See:
https://www.drupal.org/docs/8/api/configuration-api/configuration-schema...

So, 'string' type is not translatable, and 'label' type is. Some of the items in this schema file should be 'label', so that if the field is displayed in a different language, that setting can be different in those other languages.

The two that I think need to be type 'label' are 'link_text' and 'langcode'.

Thanks!

@jhodgdon I didn't know about that. Thanks for info.

Btw. In my opinion translation of "langcode" without additional context can lead to unexpected results, that's why it would be good to also provide context for that configuration option. You can find patch with proposal for that too (one with "_with_context" in name).

I did not know about translation context in schema -- great idea! I think the "with context" patch should be chosen here. Thanks!

Thank you all, fixed.

I am comfortable with that ... here is our shiny new 8.x-1.3 release

https://www.drupal.org/project/simple_gmap/releases/8.x-1.3