kas-catholic / confessit-web

Source code for https://confessit.app
MIT License
18 stars 7 forks source link

clean up `Trans` comps #64

Closed JohnRDOrazio closed 11 months ago

JohnRDOrazio commented 11 months ago

make sure all nodes are simple, without any attributes.

This will help ensure that i18n-scanner and i18next in general can keep the nodes rather than using indexed nodes.

Fixes issue #63

netlify[bot] commented 11 months ago

Deploy Preview for confessit-web ready!

Name Link
Latest commit c06def26d673324e2a4d6d121c8669254ac5ac43
Latest deploy log https://app.netlify.com/sites/confessit-web/deploys/646d48f118bbe20008dd70cc
Deploy Preview https://deploy-preview-64--confessit-web.netlify.app/
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

JohnRDOrazio commented 11 months ago

I defined a new component githubicon, which will probably need to be added to i18next-scanner configuration...

JohnRDOrazio commented 11 months ago

This PR can be pulled before #60 without any trouble, as far as I can tell. Once it's pulled, we can check PR #60 again to check if there are any conflicts that need resolution, and then pull that one too. Then the key definition process in the translation files should be much easier.

JohnRDOrazio commented 11 months ago

This will break some translations if I merge it.

Only temporarily, until PR #60 is also merged: when Trans components are set correctly, i18n-scanner can then also work correctly in scanning the source code and setting the source translation strings.

Of course even then translations in other languages will break until the translations are updated: that is normal in the translation process. Any time a source translation file is changed, Weblate will mark the translations in other languages as dirty, letting the translators know that the translations are outdated and need to be aligned with the source translation. Once the translators take care of this, everything will be working perfectly again. But it's a process, which involves both those who write the source code, and those who curate the translations.

If you want to double check whether the icon will actually work once the translation files are updated, just go ahead and manually change <i></i> to <githubicon /> in public/locales/en/translation.json at line 60. Then start up the app in English, and check that the icon is there (I just did, and it worked perfectly). Once you're happy, you can git restore public/locales/en/translation.json, then merge the PR. And from there we can move on to finalizing PR #60.

kas-catholic commented 11 months ago

Appreciate the effort to make smaller PRs, I just typically expect PRs to be individually mergable/revertable without breaking things. We can just merge both quickly in sequence in this case.