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

Integrate @formatjs/intl as a replacement for t() #7296

Merged
merged 17 commits into from
Jul 28, 2023

Conversation

Etheryte
Copy link
Member

What does this PR change?

Our current translation wrapper t() is a simple hand-rolled solution that works for basic translation, but notably does not support translations with DOM nodes mixed in etc. This PR integrates @formatjs/intl as a near drop-in replacement to add support for the missing functionality and simplify many of our existing translations.

@formatjs/intl, the library internally used by react-intl, was picked over a vanilla react-intl setup to accommodate for our SPA architecture around senna which makes it difficult to use global wrappers such as intl providers. If we ever drop senna, we can easily swap out @formatjs/intl for react-intl.

Notable changes:

No more numeric replacements, all replacements must now be named.

Before:

t("Items {0} - {1} of {2}", from, to, total);

After:

t("Items {from} - {to} of {total}", { from, to, total });

Support for mixing DOM and React components into translations has been added, meaning no more chopped up translations.

Before:

<p>
  {t("Successfully bootstrapped host! Your system should appear in ")}
  <a className="js-spa" href="/rhn/manager/systems/list/all">
    {t("systems")}
  </a>
  {t(" shortly. If it is a transactional system, please reboot it to finish registration.")}
</p>

After:

<p>
  {t(
    "Successfully bootstrapped host! Your system should appear in <link>systems</link> shortly. If it is a transactional system, please reboot it to finish registration.",
    {
      link: (str) => (
        <a className="js-spa" href="/rhn/manager/systems/list/all">
          {str}
        </a>
      ),
    }
  )}
</p>

GUI diff

No difference.

  • DONE

Documentation

  • No documentation needed: only internal and user invisible changes

  • DONE

Test coverage

  • Unit tests were added

  • DONE

Links

Fixes https://github.com/SUSE/spacewalk/issues/20449
Tracks https://github.com/SUSE/spacewalk/issues/21208

  • DONE

Changelogs

Make sure the changelogs entries you are adding are compliant with https://github.com/uyuni-project/uyuni/wiki/Contributing#changelogs and https://github.com/uyuni-project/uyuni/wiki/Contributing#uyuni-projectuyuni-repository

If you don't need a changelog check, please mark this checkbox:

  • No changelog needed

If you uncheck the checkbox after the PR is created, you will need to re-run changelog_test (see below)

Re-run a test

If you need to re-run a test, please mark the related checkbox, it will be unchecked automatically once it has re-run:

  • Re-run test "changelog_test"
  • Re-run test "backend_unittests_pgsql"
  • Re-run test "java_pgsql_tests"
  • Re-run test "schema_migration_test_pgsql"
  • Re-run test "susemanager_unittests"
  • Re-run test "javascript_lint"
  • Re-run test "spacecmd_unittests"

@Etheryte Etheryte requested a review from a team as a code owner July 20, 2023 15:07
@Etheryte Etheryte requested review from wweellddeerr and removed request for a team July 20, 2023 15:07
Copy link
Contributor

@parlt91 parlt91 left a comment

Choose a reason for hiding this comment

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

This is a really valuable addition. TYVM
See two really small remarks below

web/html/src/core/spa/spa-renderer.ts Outdated Show resolved Hide resolved
web/html/src/core/spa/spa-renderer.ts Outdated Show resolved Hide resolved
@Etheryte Etheryte merged commit a606f37 into uyuni-project:master Jul 28, 2023
10 of 14 checks passed
nodeg pushed a commit to nodeg/uyuni that referenced this pull request Aug 4, 2023
…7296)

* Initial implementation

* Update docs

* Test global binding

* Cleanup

* Update translations to new format

* Upgrade more translations

* Update more translations

* Add tests, clean up

* Deprecate test-utils t()

* Update licenses

* Cleanup

* Update commentary

* Fix pagination translation

* Refactor preferred locale

* Cleanup

* Add changes

* Cleanup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants