Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up repeated static-string interpolations #4980

Open
sarayourfriend opened this issue Sep 23, 2024 · 4 comments · May be fixed by #5261
Open

Clean up repeated static-string interpolations #4980

sarayourfriend opened this issue Sep 23, 2024 · 4 comments · May be fixed by #5261
Assignees
Labels
🤖 aspect: dx Concerns developers' experience with the codebase 🧰 goal: internal improvement Improvement that benefits maintainers, not users help wanted Open to participation from the community 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: frontend Related to the Nuxt frontend 🟨 tech: javascript Involves JavaScript 🔧 tech: vue Involves Vue.js

Comments

@sarayourfriend
Copy link
Collaborator

sarayourfriend commented Sep 23, 2024

Current Situation

We use "static" interpolations for certain kinds of string content, typically brand marks. For example, we use {openverse} 73 times in our English messages to interpolate "Openverse" into strings. This is done to prevent translators from accidentally translating brand marks which should not be translated (and is usually most important for things that could be ambiguous to translators like the name of an institution which should not be translated, e.g., an American museum whose name is in English and the institution provides no translation into supported languages, a museum in Aotearoa/New Zealand whose name is in re reo Māori, etc).

In the case of Openverse, because it is so common, we repeat the interpolation arguments of openverse: "Openverse" at least 53 times when calling i18n.t and related functions.

For example, we have the following translation string with the key hero.disclaimer.content: "All {openverse} content is under a {license} or is in the public domain.". As such, the code utilising this string must interpolate "Openverse" for the openverse template variable:

<i18n-t
  scope="global"
  keypath="hero.disclaimer.content"
  tag="p"
  class="mt-4 text-sr"
>
  <template #openverse>Openverse</template>
  <template #license>
    <VLink
      href="https://creativecommons.org/licenses/"
      class="text-default underline hover:text-default"
      >{{ $t("hero.disclaimer.license") }}</VLink
    >
  </template>
</i18n-t>

This is both tedious to maintain, and easy to error.

Suggested Improvement

There are two methods we could use to avoid needing to pass interpolation arguments for static content in code utilising messages:

  1. We could use Vue I18n's "literal interpolation" syntax. To do so, we would replace all usages of things like {openverse} with {'Openverse'}. This allows us to reuse our existing translator help messages of not translating things between ### (though we need a small change to json-to-pot to allow any kinds of content for the the "variable" name, not just letters and hyphens).
  2. We could use Vue I18n's linked messages. We would add a new "static" message, "OPENVERSE": "Openverse" that is not sent to translators, and is instead added to the final translation JSON files. Then instances of things like {openverse} would be changed to @:OPENVERSE. We can use whatever kind(s) of key(s) we want for this to designate non-translatable content not to send to translators. We would still need to add an explanation for translators to ignore the @:<key> content, or we could just wrap it in ### and reuse the same message.

I think the first option may be the best, simply because it utilises our pre-existing tool for indicating to translators not to translate certain content. Linked messages are convenient and DRY (not relevant for the "Openverse" example but is so for longer strings), but require remembering the identification method, which may be harder to remember when writing or reading than the literal interpolation format.

Therefore, I propose we replace {openverse} in our messages with {'Openverse'} and do the same for other statically-interpolated content in our strings. When I searched, I found these, in addition to Openverse:

  • github
  • wordpress
  • openverseOrg (used for the domain name)
  • sectionName
  • sketchfab

There may be others, but we can just start with those and already clean up a lot of interpolation arguments passed in code calling i18n.t or using the i18n components.

Benefit

Remove redundant, repeated static strings from the codebase, and the need for them in the first place.

@sarayourfriend sarayourfriend added 🟩 priority: low Low priority and doesn't need to be rushed 🤖 aspect: dx Concerns developers' experience with the codebase 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🧱 stack: frontend Related to the Nuxt frontend labels Sep 23, 2024
@openverse-bot openverse-bot moved this to 📋 Backlog in Openverse Backlog Sep 23, 2024
@dhruvkb
Copy link
Member

dhruvkb commented Sep 25, 2024

This issue is excellently documented and the changes involved seem very straightforward. @sarayourfriend do you think this is a potential good-first-issue?

@sarayourfriend
Copy link
Collaborator Author

I'll put help wanted. I don't think this is necessarily a good first issue because to test it will require the contributor to run the POT generation and verify the outputs. They will need to make changes to the regex for interpolation detection, as mentioned briefly in the issue description (but not in detail). All of that on top of setting up the development environment makes it too much for a first issue, I think.

@sarayourfriend sarayourfriend added help wanted Open to participation from the community 🟨 tech: javascript Involves JavaScript 🔧 tech: vue Involves Vue.js labels Sep 25, 2024
@tejaswarathe
Copy link
Contributor

Hi @sarayourfriend, @dhruvkb , I would like to work on this issue.

@dhruvkb
Copy link
Member

dhruvkb commented Dec 6, 2024

Sure @tejaswarathe, please go ahead. I'll assign it to you.

@openverse-bot openverse-bot moved this from 📋 Backlog to 📅 To Do in Openverse Backlog Dec 6, 2024
@obulat obulat linked a pull request Dec 10, 2024 that will close this issue
8 tasks
@openverse-bot openverse-bot moved this from 📅 To Do to 🏗 In Progress in Openverse Backlog Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 aspect: dx Concerns developers' experience with the codebase 🧰 goal: internal improvement Improvement that benefits maintainers, not users help wanted Open to participation from the community 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: frontend Related to the Nuxt frontend 🟨 tech: javascript Involves JavaScript 🔧 tech: vue Involves Vue.js
Projects
Status: 🏗 In Progress
Development

Successfully merging a pull request may close this issue.

3 participants