activist icon indicating copy to clipboard operation
activist copied to clipboard

Standardize i18n keys

Open andrewtavis opened this issue 1 year ago β€’ 1 comments

Terms

Behavior

Something that came up recently is that the i18n keys are not standardized - i.e. sometimes we have kebab case or snake case. This issue is to check the keys and change them all to either snake case or camel case, with the reason for this being that we want to be able to easily replace full sections of the key.

andrewtavis avatar Jun 15 '24 20:06 andrewtavis

@andrewtavis I came upon your issue by chance. What we are currently discussing internally in our team might also be of interest to you. Simply use human readable and random ids and let the tooling generate them. This will solve many problems

https://github.com/opral/monorepo/issues/1892

jannesblobel avatar Jun 17 '24 08:06 jannesblobel

hi @andrewtavis! I would like to help with this issue, is it still available?

jennethydyrova avatar Aug 13 '24 13:08 jennethydyrova

Hey @jennethydyrova πŸ‘‹ If you wanted to send along a PR that converts all of the kebab case to snake case, it'd be much appreciated!

andrewtavis avatar Aug 13 '24 22:08 andrewtavis

Let me know if there's anything I can do to help :) I'll also send along a commit to this that will make a check to see if the keys can be effectively combined. We did discuss this a bit internally, and feel that the current system is valid as it allows the localization team to easily filter for parts of the app 😊

andrewtavis avatar Aug 13 '24 22:08 andrewtavis

I just wondered if would it be fine if I send all changes in one PR or would it be easier for you to review if I split it to multiple PRs?

jennethydyrova avatar Aug 14 '24 09:08 jennethydyrova

I guess one PR would be easier as that would allow me to plan around it, bring in some others, and then merge it all at once and not need to deal with conflicts soon after that 😊

Thanks so much for asking!

andrewtavis avatar Aug 14 '24 10:08 andrewtavis

I've got one more question. According to style guide folder names should be separated by dash

Localization keys should be defined based on their component or page within the platform and the content that they refer to (CONTENT_REFERENCE below). Please use the following rules as a guide if you find yourself needing to create new localization keys: Separate directories and references by . and CamelCase file name words by - in keys Ex: "components.search-bar.CONTENT_REFERENCE" for the SearchBar component

then, for example, components.btn-action.save-settings key should be renamed to components.btn-action.save_settings, is that right?

jennethydyrova avatar Aug 14 '24 17:08 jennethydyrova

Glad to have you working on this, @jennethydyrova! Looks like the styleguide needs to be updated as well. Let's use underscores for the whole thing. That was a poor choice by us earlier :)

Would you be able to send along an update to the styleguide with this as well?

andrewtavis avatar Aug 15 '24 06:08 andrewtavis

yes, sure! πŸ™‚

jennethydyrova avatar Aug 15 '24 09:08 jennethydyrova

Thank you, @jennethydyrova!

andrewtavis avatar Aug 15 '24 10:08 andrewtavis

Closed by #948 πŸš€ Lots of work to get through this, @jennethydyrova, but with the new workflow we won't have to worry about naming keys anymore except for an identifier at the end for a hint for context. The prior parts of the key are based on the files in which it's used, and the new workflows will suggest corrections for those as we go 😊

Thanks so much for the contribution!

andrewtavis avatar Aug 30 '24 00:08 andrewtavis

Closed by #948 πŸš€ Lots of work to get through this, @jennethydyrova, but with the new workflow we won't have to worry about naming keys anymore except for an identifier at the end for a hint for context. The prior parts of the key are based on the files in which it's used, and the new workflows will suggest corrections for those as we go 😊

Thanks so much for the contribution!

Thank you, Andrew!

While addressing this issue, I noticed that the components.meta-tag-video.view-video key is utilized in frontend/components/card/search-result/CardSearchResult.vue, but it doesn’t seem to be defined in frontend/i18n/en-US.json. This might be something worth looking into.

jennethydyrova avatar Sep 02 '24 08:09 jennethydyrova

Good catch, @jennethydyrova :) That's likely just a placeholder, but definitely should be removed at some point. Do you want to do a quick PR to fix it?

andrewtavis avatar Sep 02 '24 10:09 andrewtavis

Good catch, @jennethydyrova :) That's likely just a placeholder, but definitely should be removed at some point. Do you want to do a quick PR to fix it?

yes, sure! do you want this tag to be removed from labels?

jennethydyrova avatar Sep 02 '24 19:09 jennethydyrova

Hmmm, good question :) Let's actually add it in. So this key's only in CardSearchResult, so based on the naming conventions we should change the instances of components.meta-tag-video.view-video to components.card_search_result.view_video, and then we can add this key and View video as it's value to the en-US i18n keys? How does this sound? You can link the PR to this issue, btw 😊

andrewtavis avatar Sep 02 '24 19:09 andrewtavis

hi, @andrewtavis! all done https://github.com/activist-org/activist/pull/966

jennethydyrova avatar Sep 03 '24 10:09 jennethydyrova