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

feat(i18n): improve i18n #1692

Merged
merged 25 commits into from
May 26, 2021
Merged

feat(i18n): improve i18n #1692

merged 25 commits into from
May 26, 2021

Conversation

justindomingue
Copy link
Contributor

@justindomingue justindomingue commented May 24, 2021

replace i18next with linguijs

  • extract and compile tool built-in (reconciles translations at build time)
  • use po format for development and js for prod
  • replace most t with Trans component

follow-up pr to wrap hard coded english text around Trans

To test: https://uniswap-interface-git-lingui-uniswap.vercel.app/?lang=pseudo-en

@vercel
Copy link

vercel bot commented May 24, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/uniswap/uniswap-interface/DTXziA7CUJzQzwsh3QDo9cEJToto
✅ Preview: https://uniswap-interface-git-lingui-uniswap.vercel.app

<span role="img" aria-label="has socks emoji" style={{ marginTop: -4, marginBottom: -4 }}>
<span
role="img"
aria-label={t({ id: 'wallet.hasSocks', message: 'has socks emoji' })}
Copy link
Contributor

Choose a reason for hiding this comment

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

does this work? i think not because it's a constant, and not a function component

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's why I'm using t():string here instead of . t returns a string.

Copy link
Contributor

@moodysalem moodysalem May 24, 2021

Choose a reason for hiding this comment

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

i mean the SOCKS is a constant and is only evaluated at compile first run time so the label wouldnt change when the locale changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right.. it doesn't change. I'll revert this until I figure out a better solution.

Copy link
Contributor

Choose a reason for hiding this comment

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

just make it a function component instead of a const

Copy link
Contributor Author

Choose a reason for hiding this comment

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

aria-label needs a string, but I could make an image component

Copy link
Contributor

@moodysalem moodysalem May 27, 2021

Choose a reason for hiding this comment

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

I was saying, instead of doing const SOCK = (<jsx.../>)

do

function Sock() {
  return <jsx.../>
}

and

<Sock />

This will fix the translation issue

@justindomingue justindomingue requested a review from callil May 26, 2021 16:45
@justindomingue justindomingue marked this pull request as ready for review May 26, 2021 16:45
@justindomingue justindomingue merged commit c4846c8 into main May 26, 2021
@justindomingue justindomingue deleted the lingui branch May 26, 2021 20:34
daniel-dev1990 pushed a commit to daniel-dev1990/uniswap-interface that referenced this pull request Jul 8, 2021
* replaced i18next with lingui

* integrate lingui in i18n and update dev setup

* updated components to @lingui

* fix compile error after rebase

* detect locale

* add all previous languages to linguirc

* address pr feedback

* remove it for now

* ignore generate *js files, various fixes

* added more translations

* fixed yarn build command

* wrapped more hardcoded english around <Trans>

* finished second round of translations

* added support for pseudo-en locale

* improvements

* moved copy.tsx to different branch

* moved extra files to different branch

* regenerated po

* clean up

* more fixes

* regenerate po

* remove messages.js

* clean up

* addressed pr feedback

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

Successfully merging this pull request may close these issues.

Re-enable internationalization of the UI
4 participants