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

enhance(l10n): localize en-US link title #11011

Merged
merged 11 commits into from
May 2, 2024
Merged

Conversation

caugner
Copy link
Contributor

@caugner caugner commented Apr 26, 2024

Summary

Problem

Links in other locales that point to the English locale have a title saying "Currently only available in English (US)", but this is not translated in the locales, and so it cannot be understood by users who don't understand English.

Solution

Translate the title in all locales.


Screenshots

Before

image

After

image

How did you test this change?

Ran yarn dev and checked http://localhost:3000/es/docs/Web/API#a locally.

Approvals

  • es
  • fr
  • ja
  • ko
  • pt-BR
  • ru
  • zh-CN
  • zh-TW

@caugner caugner requested a review from a team as a code owner April 26, 2024 13:04
@github-actions github-actions bot added flaw-system issues and feature requests related to the flaws system macros tracking issues related to kumascript macros labels Apr 26, 2024

export const ONLY_AVAILABLE_IN_ENGLISH = localString({
"en-US": "This page is currently only available in English.",
es: "Esta página está disponible solo en inglés.",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mdn/yari-content-es Can you please check this translation? 🙏

Copy link
Member

Choose a reason for hiding this comment

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

It looks good to me 👍 cc @Graywolf9

export const ONLY_AVAILABLE_IN_ENGLISH = localString({
"en-US": "This page is currently only available in English.",
es: "Esta página está disponible solo en inglés.",
fr: "Cette page est actuellement disponible uniquement en anglais.",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mdn/yari-content-fr Can you please check this translation? 🙏

"en-US": "This page is currently only available in English.",
es: "Esta página está disponible solo en inglés.",
fr: "Cette page est actuellement disponible uniquement en anglais.",
ja: "このページは現在、英語のみで利用可能です。",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mdn/yari-content-ja Can you please check this translation? 🙏

libs/l10n/l10n.ts Outdated Show resolved Hide resolved
fr: "Cette page est actuellement disponible uniquement en anglais.",
ja: "このページは現在、英語のみで利用可能です。",
ko: "이 페이지는 현재 영어로만 제공됩니다.",
"pt-BR": "Esta página está disponível apenas em inglês no momento.",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mdn/yari-content-pt-br Can you please check this translation? 🙏

libs/l10n/l10n.ts Outdated Show resolved Hide resolved
libs/l10n/l10n.ts Outdated Show resolved Hide resolved
libs/l10n/l10n.ts Outdated Show resolved Hide resolved
Copy link
Member

@JuanVqz JuanVqz left a comment

Choose a reason for hiding this comment

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

As for ES team this looks good! thanks for updating it 🚀

Copy link
Member

@LeoMcA LeoMcA left a comment

Choose a reason for hiding this comment

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

Mostly typescript changes, one query about escaping characters:

Comment on lines 1 to 11
type Locale =
| "en-US"
| "es"
| "fr"
| "ja"
| "ko"
| "pt-BR"
| "ru"
| "zh-CN"
| "zh-TW";
type TranslatedLocale = Exclude<Locale, "en-US">;
Copy link
Member

Choose a reason for hiding this comment

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

nit: I imagine these will be useful to use elsewhere, perhaps we export them already to save the code change later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 8f33cc6.

libs/l10n/l10n.ts Outdated Show resolved Hide resolved
libs/l10n/l10n.ts Outdated Show resolved Hide resolved
@@ -5,10 +5,13 @@ import * as util from "./util.js";

import { CONTENT_ROOT } from "../../../libs/env/index.js";
import { KumaThis } from "../environment.js";
import { DEFAULT_LOCALE } from "../../../libs/constants/index.js";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import { DEFAULT_LOCALE } from "../../../libs/constants/index.js";

kumascript/src/api/web.ts Outdated Show resolved Hide resolved
@caugner caugner requested a review from LeoMcA April 30, 2024 16:48
Copy link
Contributor

github-actions bot commented May 2, 2024

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions github-actions bot added merge conflicts 🚧 Please rebase onto or merge the latest main. and removed merge conflicts 🚧 Please rebase onto or merge the latest main. labels May 2, 2024
Copy link
Contributor Author

@caugner caugner left a comment

Choose a reason for hiding this comment

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

Self-approval to land the change (Leo's review comments have been addressed).

@caugner caugner merged commit 56a18b0 into main May 2, 2024
14 checks passed
@caugner caugner deleted the localize-en-us-link-title branch May 2, 2024 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flaw-system issues and feature requests related to the flaws system macros tracking issues related to kumascript macros
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants