chore: adding missing translation function #21479

AndVK · 2022-09-15T12:18:41Z

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue: Fixes Superset translation native filters bug #17779
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

rusackas · 2022-12-08T21:47:30Z

superset-frontend/src/components/ListView/Filters/DateRange.tsx

@@ -62,7 +62,7 @@ function DateRangeFilter(

  return (
    <RangeFilterContainer>
-      <FormLabel>{Header}</FormLabel>
+      <FormLabel>{t(Header as string)}</FormLabel>


Can you explain the "as string" need here? If this is a dynamic value, it might not be wise to use the t() here.

rusackas · 2022-12-08T21:48:50Z

superset-frontend/src/components/TableLoader/index.tsx

-          Header: column,
+      ? tableColumns.map((column: string[] | string) => ({
+        accessor: column[0],
+        Header: column[1],


Is this bycatch? Might need a little explanation of why this is tackled in this PR.

rusackas · 2022-12-08T21:49:55Z

superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx

@@ -318,7 +318,7 @@ export default function DateFilterLabel(props: DateFilterControlProps) {
        {!validTimeRange && (
          <IconWrapper className="warning">
            <Icons.ErrorSolidSmall iconColor={theme.colors.error.base} />
-            <span className="text error">{evalResponse}</span>
+            <span className="text error">{t(evalResponse)}</span>


I don't know what strings would be expected here, but this sounds highly dynamic and unlikely to be in the translation files. Correct me if I'm mistaken.

rusackas · 2022-12-08T21:50:01Z

superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx

@@ -373,7 +373,7 @@ export default function DateFilterLabel(props: DateFilterControlProps) {
      >
        <Tooltip placement="top" title={tooltipTitle}>
          <Label className="pointer" data-test="time-range-trigger">
-            {actualTimeRange}
+          {t(`${actualTimeRange}`)}


I don't know what strings would be expected here, but this sounds highly dynamic and unlikely to be in the translation files. Correct me if I'm mistaken.

rusackas · 2022-12-08T21:51:28Z

superset-frontend/src/profile/components/CreatedContent.tsx

@@ -53,7 +56,7 @@ class CreatedContent extends React.PureComponent<CreatedContentProps> {
    const search = [{ col: 'created_by', opr: 'created_by_me', value: 'me' }];
    const query = rison.encode({
      keys: ['none'],
-      columns: ['created_on_delta_humanized', 'dashboard_title', 'url'],
+      columns: ['changed_on_utc', 'dashboard_title', 'url'],


Please add context on this change... doesn't seem related to the general translation theme of the PR.

rusackas · 2022-12-08T21:51:50Z

superset-frontend/src/profile/components/CreatedContent.tsx

@@ -63,8 +66,8 @@ class CreatedContent extends React.PureComponent<CreatedContentProps> {
    const mutator = (data: DashboardResponse) =>
      data.result.map(dash => ({
        dashboard: <a href={dash.url}>{dash.dashboard_title}</a>,
-        created: dash.created_on_delta_humanized,
-        _created: dash.created_on_delta_humanized,
+        created: moment.utc(dash.changed_on_utc).fromNow(),


Again, more context here would be helpful.

rusackas · 2022-12-08T21:52:13Z

superset-frontend/src/profile/components/Favorites.tsx

@@ -64,16 +68,19 @@ export default class Favorites extends React.PureComponent<FavoritesProps> {
    const mutator = (data: DashboardResponse) =>
      data.result.map(dash => ({
        dashboard: <a href={dash.url}>{dash.dashboard_title}</a>,
-        created: dash.created_on_delta_humanized,
-        _created: dash.created_on_delta_humanized,
+        created: moment.utc(dash.changed_on_utc).fromNow(),


Context for this change also welcomed, thanks!

rusackas · 2022-12-08T21:54:47Z

superset-frontend/src/views/components/LanguagePicker.tsx

@@ -76,7 +76,7 @@ export default function LanguagePicker(props: LanguagePickerProps) {
        >
          <StyledLabel className="f16">
            <i className={`flag ${languages[langKey].flag}`} />
-            <a href={languages[langKey].url}>{languages[langKey].name}</a>
+            <a href={languages[langKey].url}>{t(languages[langKey].name)}</a>


Just to make sure, I hope having these translated doesn't cause an issue with people having a harder time switching to their preferred language!

rusackas · 2022-12-08T21:58:28Z

Most of this seems a-ok! Thanks for the contribution! The vast majority of the t() calls seem unobjctionable.

There is, however, a fair amount of change in this PR involving Moment and utc timestamps. A description of how that fits in here (and any PR description at all, for that matter) would be helpful.

There are also a couple of translated strings in there that look like they might be fairly dynamic in terms of content. We shouldn't use the t() wrapper unless the translated string is static/stable.

Also noting that this should close out #21200 and this can be done automatically by adding "Fixes: #21200" in your PR description.

rusackas · 2023-01-12T04:29:00Z

Hi @AndVK :) I haven't forgotten about this... any chance this can get a rebase and we can push it through? It's so close

rusackas · 2023-01-19T05:21:44Z

@AndVK if this is getting to painful to rebase until CI passes and so forth, I'm also fine with splitting it up into smaller PRs that require less strenuous reviews ;) Either way, I still appreciate all this great/hard work!

AndVK · 2023-01-20T07:39:36Z

Hi, @rusackas! I will split this PR into smaller ones.

rusackas · 2023-01-23T22:05:06Z

Hi, @rusackas! I will split this PR into smaller ones.

Fantastic, looking forward to it! Ping me/us here and/or on Slack to make sure we pay attention to them! :D

hbruch · 2023-03-14T14:20:36Z

Hi @AndVK!

Thanks for the great work! Do you think you'll be able to split this PR in the next days? If not, I could give a helping hand, if you'd like to...

rusackas · 2023-04-06T22:01:15Z

@AndVK @hbruch do either of you have the bandwidth to carry any pieces of this forward? Leaving it open since it serves as a reminder, and hoping that we can close out #21200 sooner rather than later 🤞

hbruch · 2023-04-07T12:27:27Z

Would you mind reviewing #23402 before?

hbruch · 2023-04-09T07:05:21Z

I went through the suggested changes and most of them were already handled and merged via @AndVK's #21816 (thanks again!). For the very few missing translations I spotted, I'll submit a PR.

Besides this, two changes suggested by @AndVK are still open and require some code changes:

internationalize table columns
humanized times/ time deltas. For these, I also think, they should be handled client side

Another category of missing translations are error messages. Not sure, which of the are really end user facing, but a couple of them them will be (e.g. in controls.ts or TTestTable.jsx).

rusackas · 2023-04-10T18:06:50Z

Thanks @hbruch. Closing this PR for now, and appreciate the follow-up!

Adding missing translation function

2206b7e

AndVK requested review from michael-s-molina, geido and ktmud as code owners September 15, 2022 12:18

pull-request-size bot added the size/XL label Sep 15, 2022

AndVK and others added 3 commits September 15, 2022 16:33

Merge branch 'master' into modified-translate

69ee81c

fix failure

4094914

fix conflict and failure

f133351

rusackas changed the title ~~Adding missing translation function~~ chore: adding missing translation function Dec 8, 2022

rusackas reviewed Dec 8, 2022

View reviewed changes

rusackas mentioned this pull request Jan 12, 2023

Superset translation native filters bug #17779

Closed

3 tasks

hbruch mentioned this pull request Apr 7, 2023

German Translation not correctly implemented? #19431

Closed

2 tasks

rusackas closed this Apr 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: adding missing translation function #21479

chore: adding missing translation function #21479

AndVK commented Sep 15, 2022 •

edited by rusackas

Loading

rusackas Dec 8, 2022

rusackas Dec 8, 2022

rusackas Dec 8, 2022

rusackas Dec 8, 2022

rusackas Dec 8, 2022

rusackas Dec 8, 2022

rusackas Dec 8, 2022

rusackas Dec 8, 2022

rusackas commented Dec 8, 2022

rusackas commented Jan 12, 2023

rusackas commented Jan 19, 2023

AndVK commented Jan 20, 2023

rusackas commented Jan 23, 2023

hbruch commented Mar 14, 2023

rusackas commented Apr 6, 2023

hbruch commented Apr 7, 2023

hbruch commented Apr 9, 2023

rusackas commented Apr 10, 2023

chore: adding missing translation function #21479

chore: adding missing translation function #21479

Conversation

AndVK commented Sep 15, 2022 • edited by rusackas Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rusackas commented Dec 8, 2022

rusackas commented Jan 12, 2023

rusackas commented Jan 19, 2023

AndVK commented Jan 20, 2023

rusackas commented Jan 23, 2023

hbruch commented Mar 14, 2023

rusackas commented Apr 6, 2023

hbruch commented Apr 7, 2023

hbruch commented Apr 9, 2023

rusackas commented Apr 10, 2023

AndVK commented Sep 15, 2022 •

edited by rusackas

Loading