search

buttercup / buttercup-mobile

:iphone: React-Native mobile application for Buttercup
https://buttercup.pw
GNU General Public License v3.0
392 stars 70 forks source link

i18n #156

Closed ph1p closed 4 years ago

ph1p commented 5 years ago

Hi!

This PR adds i18n functionality and is currently WIP. I'll add auto detection and a language chooser or something else. I have copied some code from buttercup-desktop and experimented a little (:

I am pleased about your thoughts and suggestions.

[UPDATE]

julianpoemp commented 5 years ago

That's what I was looking for 😁👍

perry-mitchell commented 5 years ago

Great stuff @ph1p! This'd be super handy.

ph1p commented 5 years ago

I'm almost done translating all the strings. Some parts are a bit confusing (sheets.js), but it should work (: Currently I have added English and German. Buttercup auto detects the language.

perry-mitchell commented 5 years ago

Great @ph1p! Amazing work. Should we give it an in-depth review now? Would you say the WIP is done?

ph1p commented 5 years ago

Sure! Feel free to review. I know there's definitely room for improvement, but it's a first version (:

ph1p commented 5 years ago

I also tried to fix the build bugs. I made android work, but ios still fails.

Found this config: https://travis-ci.org/ankidroid/Anki-Android/builds/526250449/config

ph1p commented 5 years ago

@sallar @perry-mitchell I've also updated the react-navigation, because there is a beta inside the package.json.

rn-nodeify > ^10.0.1 (https://github.com/tradle/rn-nodeify/issues/74) react-navigation > ^2.18.2 (https://reactnavigation.org/blog/2018/05/07/react-navigation-2.0.html) react-navigation-redux-helpers > ^2.0.9 (https://github.com/react-navigation/redux-helpers/releases/tag/2.0.0) react > ^16.6.1 react-test-renderer > ^16.6.1 react-native > ^0.57.5 (https://github.com/react-native-community/releases/blob/master/CHANGELOG.md#0575) This version needs react on version 16.6.1 react-native-cell-components > github:lodev09/react-native-cell-components (https://github.com/lodev09/react-native-cell-components/pull/11)

The version 3 of React-Navigation doesn't work very well. The StackNavigator doesn't animate the page change smoothly, so I decided to go with 2.x.x. Tested it on iOS and Android and added -UseModernBuildSystem=NO to testIOS.sh. (https://github.com/facebook/react-native/issues/20774)

perry-mitchell commented 5 years ago

Thanks @ph1p - We'll take a look at this tomorrow. Sorry for the delay!

perry-mitchell commented 5 years ago

@ph1p Is this still WIP or ready to go?

ph1p commented 5 years ago

Everything is all right (: If you want me to create a new branch, I can do that, because I experimented a little bit.

ph1p commented 5 years ago

@ph1p Is this still WIP or ready to go?

I think it is ready. Maybe later we can create a new repository just for translations. Desktop, Browser and Mobile can implement it and we've one source.

There was no way for me to run the tests. I also tried Bitrise and CircleCI (Android only).

perry-mitchell commented 5 years ago

Hi @ph1p - Once again, really sorry for the delay in moving this. I'll try to help get it through asap. Currently also trying to get Google Drive integration working, so perhaps this can go out with that!

ph1p commented 5 years ago

Hi @perry-mitchell ! No problem (: Take your time! I know it's not always easy to keep track of things. (Issues, PRs, different repositories and so on)

Please take look at https://github.com/buttercup/buttercup-mobile/pull/156#issuecomment-489357037 while reviewing + maybe we need a settings section to set the language and other things.

perry-mitchell commented 5 years ago

Hi @ph1p - I've now gotten around to reviewing.. Really sorry for how long this has taken. I removed Google Drive from the deployment pathway as it was just too problematic to release just yet. Once the reviews are satisfied I can release this asap.

perry-mitchell commented 5 years ago

I also reverted the travis config.. this is what you'd submitted, for reference:

matrix:
  include:
    - language: node_js
      script: npm run test
      env:
        - NAME=Jest
      before_install:
        - npm i -g jest-cli
      install:
        - npm install
      node_js:
        - "8"
    - language: objective-c
      node_js:
        - "10.13.0"

      os: osx
      osx_image: xcode10.2

      install:
        - npm ci &>/dev/null
        - ./scripts/testIOS.sh "iPhone XS" "12.0" "build"
        - ./scripts/testIOS.sh "iPhone XS" "12.0" "test"

    - language: objective-c
      node_js:
        - "10.13.0"

      os: osx
      osx_image: xcode10.2

      install:
        - npm ci &>/dev/null
        - ./scripts/testIOS.sh "iPhone 6s" "10.3.1" "build"
        - ./scripts/testIOS.sh "iPhone 6s" "10.3.1" "test"

    - language: android
      sudo: required
      jdk: oraclejdk8
      env:
        - ANDROID_API=23
        - ADB_INSTALL_TIMEOUT=8 # minutes
        - ANDROID_ABI=x86_64
        - EMU_FLAVOR=default
        - ANDROID_HOME=/usr/local/android-sdk
        - TOOLS=${ANDROID_HOME}/tools
        - PATH=${ANDROID_HOME}:${ANDROID_HOME}/emulator:${TOOLS}:${TOOLS}/bin:${ANDROID_HOME}/platform-tools:${PATH}
        - ANDROID_NDK_HOME=${ANDROID_HOME}/ndk-bundle

      android:
        components:
          - tools

        licenses:
          - 'android-sdk-preview-license-.+'
          - 'android-sdk-license-.+'
          - 'google-gdk-license-.+'

      install:
        - echo 'count=0' > /home/travis/.android/repositories.cfg # Avoid harmless sdkmanager warning
        - echo y | sdkmanager "platform-tools" >/dev/null
        - echo y | sdkmanager "ndk-bundle"
        - echo y | sdkmanager "cmake;3.6.4111459"
        - echo y | sdkmanager "lldb;3.1"
        - echo y | sdkmanager "tools" >/dev/null # A second time per Travis docs, gets latest versions
        - echo y | sdkmanager "build-tools;28.0.3" >/dev/null # Implicit gradle dependency - gradle drives changes
        - echo y | sdkmanager "platforms;android-$ANDROID_API" >/dev/null # We need the ANDROID_API of the emulator we will run
        - echo y | sdkmanager "platforms;android-28" >/dev/null # We need the API of the current compileSdkVersion from gradle.properties
        - echo y | sdkmanager --channel=4 "emulator" # Experiment with canary, specifying 28.0.3 (prior version) did not work
        - echo y | sdkmanager "extras;android;m2repository" >/dev/null
        - echo y | sdkmanager "system-images;android-$ANDROID_API;$EMU_FLAVOR;$ANDROID_ABI" #>/dev/null # install our emulator
        - echo no | avdmanager create avd --force -n test -k "system-images;android-$ANDROID_API;$EMU_FLAVOR;$ANDROID_ABI" -c 10M
        - emulator -verbose -avd test -no-accel -no-snapshot -no-window $AUDIO -camera-back none -camera-front none -selinux permissive -qemu -m 2048 &
        - android-wait-for-emulator
        - adb shell input keyevent 82 &
        - curl -sL https://deb.nodesource.com/setup_8.x | sudo -E bash -
        - sudo apt-get install -y nodejs
        - npm install

      script:
        - ./scripts/testAndroid.sh

Would it be possible to ask you to create a new PR with it? I'd be happy to debug how it runs on travis with you on a separate PR, it was just too much to include in one.

ph1p commented 5 years ago

thanks for reviewing @perry-mitcheell (: I'll work on it

perry-mitchell commented 5 years ago

@ph1p Let us know when you're ready for another review.

ph1p commented 5 years ago

I'm always ready.

perry-mitchell commented 5 years ago

Looks good to me. I can do some testing on this tomorrow. I'll leave it to @sallar to make a review as well.

perry-mitchell commented 4 years ago

Apologies @ph1p once again for missing this - I'll try to handle the conflicts so you don't need to again. We upgrade react-navigation to get the OTP codes feature working, so I'll skip that in this MR.

perry-mitchell commented 4 years ago

Hi @ph1p - I've merged your branch with master over at https://github.com/buttercup/buttercup-mobile/pull/209 and it seems to work locally. There's now a few missing localisations as we added text in the OTP codes MR. There's also the error messages and some other pieces of text (eg. "OK") which need localising, but that should be quite easy I guess.

I did remove the changes for the react-navigation upgrade as we'd done the same, sorry, and had also removed the redux portion of it (as we were originally planning to do away with it anyway). I'm sorry if you spent any great amount of time on this.

Now the final part: What would you like to do? Should we take it from here and finish it off, or would you prefer to take the merge-commit into your branch and finalise it yourself? We're entirely happy either way.. just sorry that we left it sit for so long.

Thanks for the help and patience :)

ph1p commented 4 years ago

Hi @ph1p - I've merged your branch with master over at #209 and it seems to work locally. There's now a few missing localisations as we added text in the OTP codes MR. There's also the error messages and some other pieces of text (eg. "OK") which need localising, but that should be quite easy I guess.

I did remove the changes for the react-navigation upgrade as we'd done the same, sorry, and had also removed the redux portion of it (as we were originally planning to do away with it anyway). I'm sorry if you spent any great amount of time on this.

Now the final part: What would you like to do? Should we take it from here and finish it off, or would you prefer to take the merge-commit into your branch and finalise it yourself? We're entirely happy either way.. just sorry that we left it sit for so long.

Thanks for the help and patience :)

Hi @perry-mitchell, That was a long way, until now :D

I did remove the changes for the react-navigation upgrade as we'd done the same, sorry, and had also removed the redux portion of it (as we were originally planning to do away with it anyway). I'm sorry if you spent any great amount of time on this.

That's all right. It's your project and I see a PR more as a proposal and discussion basis to get the best out of it. And if just everything is merged without meaning, nobody is helped (:

Now the final part: What would you like to do? Should we take it from here and finish it off, or would you prefer to take the merge-commit into your branch and finalise it yourself? We're entirely happy either way.. just sorry that we left it sit for so long.

I can do it gladly. I want to help you and finish it in the next days ;) Should I create a new PR and we close this one or should I override this?

[EDIT]: nevermind, I force pushed it to this one.

ph1p commented 4 years ago

@perry-mitchell I think I'm done. Let me know if something is missing or if I have to adjust the structure of the locale files.

perry-mitchell commented 4 years ago

Thanks @ph1p - Taking this forward to another branch so we can merge it with our rn upgrade and get it out! Great work on this!