[RFC] Internationalize wallet #3985
Conversation
dangdennis
requested review from
bkase,
es92,
figitaki,
imeckler,
o1pranay and
Schmavery
as code owners
|
|
|
@Schmavery need your input :D |
|
I don't want the translation to get any deeper than this. Have a couple issues to resolve before the PR is technically done for part 1
|
There was a problem hiding this comment.
@dangdennis thanks for doing this work.
I don't think you need to wrap the provider unless there's some other issue, their provider component should work as long as we're not doing anything unusual with it.
Re: translations, react-intl should work with partial translations right? I think it's OK if you don't spend more time on the vn translations and just focus on the english ones that are relevant to the PR.
I'm actually a little confused about the multiple layers of defaultMessage -- there seems to be one in the code, and then one in each translation. Which takes precedence? Is it really necessary to keep these all in sync? Is it possible to just fall back to english translations when we don't have an id translated to the desired language?
Were you able to get the webpack file-loader working with fastpack in order to avoid having to use the file loading code we wrote?
The textTransform sounds fine to me (cc @figitaki ).
I'm not sure what you meant by "How to write translations in the locale json" so plz clarify if I didn't manage to answer above.
…-intl Conflicts: frontend/wallet/src/render/views/transactions/Transactions.re
|
Lmk what else I need to do. This PR completes integration of react intl. |
|
It's looking pretty good @dangdennis, sorry for the delay. |
| @@ -24,10 +24,17 @@ let isFaker = | |||
| Js.Dict.get(Bindings.ChildProcess.Process.env, "GRAPHQL_BACKEND") | |||
| == Some("faker"); | |||
|
|
|||
| let getTranslation = name => | |||
| Bindings.Fs.readFileSync( | |||
| "./src/render/i18n/translations/" ++ name ++ ".json", | |||
There was a problem hiding this comment.
My only concern is that our bundler might not include these files in the build output. A better place for the translation JSON files to live might be under public/translations/ since all the files in that folder will automatically copied into the destination. Might be worth just running yarn dist and running that build to double check.
There was a problem hiding this comment.
I’ll test this!
Are there any tests running against the prod build yet? Was thinking it would catch this error since loading a missing file would throw an error
There was a problem hiding this comment.
@Schmavery can we pair again sometime to finish this up?
- Unable to commit now, receiving this error. Unsure why that particular exe is missing.
INFO Not using Docker
find: _build/default: No such file or directory
find: _build/default: No such file or directory
find: _build/default: No such file or directory
dune exec --profile=dev src/app/reformat/reformat.exe -- -path . -check
make: dune: No such file or directory
make: *** [check-format] Error 1
** REFUSING TO COMMIT: "$ make check-format" failed
yarn distleaves a long error stack trace.
Can't locate Mac/Memory.pm in @INC (you may need to install the Mac::Memory module)...
There was a problem hiding this comment.
@dangdennis unfortunately no tests on the dist build right now
-
that's a post commit hook that checks the native code (surprised you never ran into it before). If you have opam+dune installed, you can
eval $(opam env)and then it should work, otherwise, you can skip the hooks withgit commit --no-verify -
ouch, that's an ugly error. @figitaki did you see this when running dist build lately? It could be that upgrading to
"electron-builder": "^21.2.0"(from here) might help?
There was a problem hiding this comment.
Cool bypassed the git hook.
Did a test upgrade to electron-builder to 21.2.0. I'm no longer getting that scary stack trace. But what's build doing in this script:
"dist": "yarn run build && yarn run pack && build"
How does electron-builder actually get run in your setup?
Edit:
"dist": "yarn run build && yarn run pack && electron-builder build" produced a running .dmg!
|
is the electron-builder upgrade going to happen soon? i didn't commit the upgrade nor the script change. |
|
We're about to land #4084 which will fix the |
|
If you rebase on develop electron-builder has been upgraded. |
Everything's working fine now. Hope all if not most of the CI tests pass. I tried to squash my commits, but it's a bit hairy since I haven't been rebasing. Whoever does the merge, will you please use Github's squash & merge feature? |
|
finally merged! thanks again @dangdennis |
|
Awesome! Looking forward to contributing in other ways one day. |
Explain your changes here.
RFC for the react-intl grant.
Explain how you tested your changes here.
My markdown and command of the English language is decent.
Checklist: