Pushkal-Gupta commented on code in PR #76096:
URL: https://github.com/apache/airflow/pull/76096#discussion_r4436469089
##########
airflow-core/src/airflow/ui/src/i18n/ariaLabels.test.ts:
##########
@@ -28,22 +28,26 @@ const sourceFiles: Record<string, string> =
import.meta.glob("../**/*.tsx", {
query: "?raw",
});
-// Only a fully quoted value, with or without braces: aria-label="Next page" or
-// aria-label={"Next page"}. A template literal that interpolates a value
needs a key with
-// its own interpolation, so those are left for a follow-up rather than
exempted here.
-const LITERAL_ARIA_LABEL = /aria-label=\{?"(?<label>[^"]*)"/gu;
+// A fully quoted value, with or without braces: aria-label="Next page" or
aria-label={"Next page"}.
+const LITERAL_ARIA_LABEL = /aria-label=\{?"[^"]*"/gu;
+// A value that is itself a template literal, such as aria-label={`Remove
${name} filter`}. Even
+// when every interpolated part is already translated, building the sentence
here pins English word
+// order, so it belongs in the locale as one key with its own interpolation. A
template literal
+// nested inside a larger expression is not matched:
`translate(`docs.${key}`)` composes a key, not
+// a sentence.
+const TEMPLATE_ARIA_LABEL = /aria-label=\{`[^`]*`\}/gu;
Review Comment:
Thanks, I missed that, I have fixed it.
The toggle uses `alerts.expandAlerts` and `alerts.collapseAlerts` now, with
the same `{{count, number}}` plural as the `showMoreAlerts` badge right next to
it, so the hand-rolled plural is gone too. Three new en keys, nothing else
touched.
I did it even bigger because leaving the hole felt like asking for the same
bug back later. It matches a literal anywhere in the value, with two exceptions
so it doesn't start yelling about keys: anything between a `translate(` or `t(`
and its closing paren is ignored, and a literal only counts if its static text
has a letter in it.
Both of those came out of running it. My first attempt only ignored a
literal sitting directly after `translate(`, and it immediately flagged four
things that are fine:
```
pages/Dag/Code/Code.tsx: {translate(`common:wrap.${wrap ? "un" : ""}wrap`)}
pages/DagsList/DagsList.tsx: {translate(showFolders ? "dags:folders.hide" :
"dags:folders.show")}
```
The letter rule is what keeps `` `${errorTitle} ${String(status)}` `` in
`XComEntry` out of it, since there's no actual word in there to translate.
It's clean on the branch. Put the four component sites back to their old
text and it names exactly those four:
```
components/FilterBar/FilterPill.tsx: {`Remove ${filter.config.label} filter`}
components/FilterBar/filters/DateInput.tsx: {`Clear ${field} ${inputType}`}
layouts/Details/Grid/VersionIndicator.tsx: {`Version ${dagVersionNumber}
indicator`}
pages/Dashboard/Dashboard.tsx: { isCollapsed ? `Show ${restAlerts.length}
more alert...` : "Show fewer alerts" }
```
`Dashboard.test.tsx` was already checking that button by its accessible
name, so I had its i18next mock hand back `COLLAPSE` and `EXPAND 2` for the two
new keys. Four of its tests fail if the English goes back in.
I've updated the PR description too, since the line in there about nested
template literals never being flagged isn't true any more.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
