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

Japanese translation has been completed. #751

Merged
merged 8 commits into from
Apr 14, 2022

Conversation

kuragehimekurara1
Copy link
Contributor

Please confirm that the Japanese translation has been completed.

Japanese translation has been completed.
Added Japanese translation of timeline
@commit-lint
Copy link

commit-lint bot commented Apr 9, 2022

Code Refactoring

  • update login_sign_up_link_target (2cea34d)

Contributors

kuragehimekurara1, Chesire

Commit-Lint commands

You can trigger Commit-Lint actions by commenting on this PR:

  • @Commit-Lint merge patch will merge dependabot PR on "patch" versions (X.X.Y - Y change)
  • @Commit-Lint merge minor will merge dependabot PR on "minor" versions (X.Y.Y - Y change)
  • @Commit-Lint merge major will merge dependabot PR on "major" versions (Y.Y.Y - Y change)
  • @Commit-Lint merge disable will desactivate merge dependabot PR
  • @Commit-Lint review will approve dependabot PR
  • @Commit-Lint stop review will stop approve dependabot PR

@Chesire
Copy link
Owner

Chesire commented Apr 9, 2022

Thanks for this!
The translations are currently replacing the English strings, would it be possible to put them in libraries/core/src/main/res/values-ja/strings.xml and can look to get this merged in

@kuragehimekurara1
Copy link
Contributor Author

Thanks for this!
The translations are currently replacing the English strings, would it be possible to put them in libraries/core/src/main/res/values-ja/strings.xml and can look to get this merged in

Should I create a new file and put strings.xml in it?

@Chesire
Copy link
Owner

Chesire commented Apr 10, 2022

If you create the required directories and put the new strings.xml there it should be all good.

The idea is to have the values folder which contains the strings file for English, and a values-ja folder which contains the strings file for Japanese

@kuragehimekurara1
Copy link
Contributor Author

I understand!  Do I just create a new pull request when I'm done?

@Chesire
Copy link
Owner

Chesire commented Apr 10, 2022

You can update this one (assuming the English strings are put back), or create a new one. Whichever you find easier for you 😃

@kuragehimekurara1
Copy link
Contributor Author

Yes, sir.

Please confirm that the Japanese translation has been completed.
Corrected the translation of strings.xml
@kuragehimekurara1
Copy link
Contributor Author

I created a values-ja folder and added the strings.xml file translated into Japanese in it. Also, the original strings.xml file has been restored to English. How about this?

@codecov
Copy link

codecov bot commented Apr 10, 2022

Codecov Report

Merging #751 (2cea34d) into master (c82f189) will decrease coverage by 0.19%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #751      +/-   ##
============================================
- Coverage     68.98%   68.79%   -0.20%     
+ Complexity      441      436       -5     
============================================
  Files           109      109              
  Lines          1680     1679       -1     
  Branches        143      145       +2     
============================================
- Hits           1159     1155       -4     
  Misses          478      478              
- Partials         43       46       +3     
Impacted Files Coverage Δ
...com/chesire/nekome/core/flags/HomeScreenOptions.kt 87.50% <0.00%> (-12.50%) ⬇️
...ain/java/com/chesire/nekome/core/settings/Theme.kt 88.88% <0.00%> (-11.12%) ⬇️
.../com/chesire/nekome/core/flags/UserSeriesStatus.kt 92.30% <0.00%> (-7.70%) ⬇️
...in/java/com/chesire/nekome/kitsu/user/KitsuUser.kt 87.50% <0.00%> (ø)
...e/datasource/auth/remote/AuthRefreshInterceptor.kt 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c82f189...2cea34d. Read the comment docs.

Copy link
Owner

@Chesire Chesire left a comment

Choose a reason for hiding this comment

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

The changes look good, thanks for this.
One additional change to get this working is to update the app/build.gradle to include

    defaultConfig {
        applicationId "com.chesire.nekome"
        minSdk 21
        targetSdk sdk_version
        versionCode 22040915 // Date of build formatted as 'yyMMddHH'
        versionName "1.5.3"
        testInstrumentationRunner "com.chesire.nekome.TestRunner"
        resConfigs "en", "ja"
    }

as the resConfig is only setup for en currently.

<string name="login_forgot_password_url">https://kitsu.io/password-reset</string>
<string name="login_sign_up">まだKitsuに登録されていませんか? こちらからKitsuに登録出来ます</string>
<!-- This text should be exactly what part of login_sign_up should be a link -->
<string name="login_sign_up_link_target">サインアップ</string>
Copy link
Owner

Choose a reason for hiding this comment

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

The text in this string will need to be a part of the string above it. In English

    <string name="login_sign_up">Not yet registered? Sign up</string>
    <!-- This text should be exactly what part of login_sign_up should be a link -->
    <string name="login_sign_up_link_target">Sign up</string>

it uses the Sign up part as a link.
Could this be こちらからKitsuに登録出来ます?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How should I change the sign up part?

Copy link
Owner

Choose a reason for hiding this comment

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

login_sign_up needs to contain the text from login_sign_up_link_target as it sets up a link using login_sign_up_link_target.
login_sign_up_link_target is not displayed to the user, its just used to lookup which part of the login_sign_up should contain a link.

It should be whichever part of login_sign_up is the equivalent to "Sign up"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the late correction.
The translation has been corrected. How about this?

Copy link
Owner

Choose a reason for hiding this comment

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

Made a change to what I think it needs to be. If you are happy with the change I'll give it a test locally and look to merge it all in

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for changing the translation!
We're happy enough with it and you can merge it!

@kuragehimekurara1
Copy link
Contributor Author

I got it

Add "ja" to resconfig
@kuragehimekurara1
Copy link
Contributor Author

Updated build.gradle. Is this correct?

@Chesire
Copy link
Owner

Chesire commented Apr 11, 2022

Updated build.gradle. Is this correct?

Yup looks good 👍

kuragehimekurara1 and others added 2 commits April 15, 2022 00:33
Some translations were changed and corrected.
Update string to target part of the login_sign_up string.
<string name="login_forgot_password_url">https://kitsu.io/password-reset</string>
<string name="login_sign_up">こちらからKitsuに登録出来ます</string>
<!-- This text should be exactly what part of login_sign_up should be a link -->
<string name="login_sign_up_link_target">こちらから</string>
Copy link
Owner

Choose a reason for hiding this comment

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

I've made a change here to show what I think the string should be. It needs to have a matching section in the login_sign_up part. Its used just to set up some formatting for the string above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for changing the translation!

@Chesire Chesire merged commit aa321cb into Chesire:master Apr 14, 2022
@Chesire
Copy link
Owner

Chesire commented Apr 14, 2022

Thanks for this 👍

@Chesire Chesire mentioned this pull request Apr 14, 2022
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.

2 participants