Skip to content
This repository has been archived by the owner on Jul 27, 2024. It is now read-only.

Add checks for validating translations in {% schema %} #68

Merged
merged 4 commits into from
Dec 10, 2020

Conversation

macournoyer
Copy link
Contributor

@macournoyer macournoyer commented Dec 10, 2020

{% schema %} contains JSON that can contain locales. Similar to the ones under locales/:

{% schema %}
  {
    "locales": {
      "en": {
        "title": "Welcome",
        "missing": "Product"
      },
      "fr": {
        "title": "Bienvenue",
        "extra": "Extra"
      }
    }
  }
{% endschema %}

This new check MatchingSchemaTranslations, applies the same check to "locales" as MatchingTranslations does to locales/*.json. I refactored the common code under LocaleDiff.

Also, an undocumented feature of the schema tag is that all keys that end up in the UI can be localized too:

{% schema %}
{
  "name": {
    "cs": "Seznam kolekcí",
    "da": "Kollektionsliste",
    "de": "Kategorieliste",
    "en": "Collection list",
    "es": "Lista de colecciones",
  },
  "settings": [
    {
      "type": "text",
      "id": "title",
      "label": {
        "cs": "Nadpis",
        "da": "Overskrift",
        "de": "Überschrift",
        "en": "Heading",
        "es": "Encabezado",
    ...
}
{% endschema %}

This new check, makes sure all keys have defined all the known locales used in the schema.

Not perfect, but it does match the errors of existing checks on project-64k (yarn build --checkTargetLocales):

sections/collection-list.liquid:67: suggestion: MatchingSchemaTranslations: settings.swipe_on_mobile.label missing translations for cs, da, de, es, fi, fr, it, ja, ko, nb, nl, pl, pt-BR, pt-PT, sv, th, tr, vi, zh-CN, zh-TW.
	{% schema %}
	   ^^^^^^^

sections/featured-collection.liquid:105: suggestion: MatchingSchemaTranslations: settings.swipe_on_mobile.label missing translations for cs, da, de, es, fi, fr, it, ja, ko, nb, nl, pl, pt-BR, pt-PT, sv, th, tr, vi, zh-CN, zh-TW.
	{% schema %}
	   ^^^^^^^

sections/main-collection.liquid:98: suggestion: MatchingSchemaTranslations: settings.center_align_text.label missing translations for cs, da, de, es, fi, fr, it, ja, ko, nb, nl, pl, pt-BR, pt-PT, sv, th, tr, vi, zh-CN, zh-TW.
	{% schema %}
	   ^^^^^^^

Fix #68

@macournoyer macournoyer changed the base branch from main to master December 10, 2020 13:21
@macournoyer macournoyer marked this pull request as ready for review December 10, 2020 19:03
Copy link
Contributor

@jbourassa jbourassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat! 👍

keys.map { |path| (key_prefix + path).join(".") }.join(", ")
end

def visit_object(default, other, path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌 good renaming, much better than the previous ones!

END
)
assert_offenses(<<~END, offenses)
unexpected token at '{ in JSON at sections/product.liquid:1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, we might even be able to increment that :1 with the line number from the {% schema %} tag (one day, not now).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah! I created a ticket to do this: #71. That would be awesome!

@macournoyer macournoyer merged commit 1006280 into master Dec 10, 2020
@macournoyer macournoyer deleted the schema-translations branch December 10, 2020 22:08
@macournoyer macournoyer temporarily deployed to rubygems January 6, 2021 18:53 Inactive
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.

2 participants