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: Allow changing languages #1450

Merged
merged 23 commits into from
Nov 11, 2023
Merged

feat: Allow changing languages #1450

merged 23 commits into from
Nov 11, 2023

Conversation

Ushie
Copy link
Member

@Ushie Ushie commented Oct 30, 2023

No description provided.

@validcube
Copy link
Member

validcube commented Oct 31, 2023

This will result in conflict for all PR targeting dev.

I suggest that we merged all ready to be merge PR before merging this.

@validcube validcube self-requested a review October 31, 2023 10:04
Copy link
Member

@validcube validcube left a comment

Choose a reason for hiding this comment

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

Bleep

assets/i18n/strings.i18n.json Outdated Show resolved Hide resolved
assets/i18n/strings.i18n.json Outdated Show resolved Hide resolved
pubspec.yaml Outdated Show resolved Hide resolved
pubspec.yaml Show resolved Hide resolved
Copy link
Contributor

@Aunali321 Aunali321 left a comment

Choose a reason for hiding this comment

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

image

Copy link
Member

@validcube validcube left a comment

Choose a reason for hiding this comment

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

Bloop: The documentation should be updated to mentions that translation must be generated using the command:

dart run slang

because we are not using slang build runner

additionally, the CI should also be updated too


👀 Checkout the full conversation discussed @ Discord/ReVanced (Team chat; private)
https://canary.discord.com/channels/952946952348270622/952987428786941952/1168894670579966012

Copy link
Member

@validcube validcube left a comment

Choose a reason for hiding this comment

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

🥞 (Application is) Tested & Approved

@oSumAtrIX
Copy link
Member

Are the translation strings supposed to be included in this PR? They should be added separately by Crowdin

@validcube
Copy link
Member

validcube commented Nov 5, 2023

@oSumAtrIX Are the translation strings supposed to be included in this PR? They should be added separately by Crowdin

I'm sure that they aren't supposed to be included because it's out-of-scope, I decided to add translation to this PR because the repository don't have translation merged yet and I don't really know how would I (automatically) add translation to the repository because the translation file have to be renamed from *.json to strings_*.i18n.json

@Ushie
Copy link
Member Author

Ushie commented Nov 5, 2023

That change can be done in Crowdin, once this PR is ready to merge

@validcube
Copy link
Member

validcube commented Nov 5, 2023

@Ushie That change can be done in Crowdin, once this PR is ready to merge

This PR is ready to be merge into dev.

@validcube I don't really know how would I (automatically) add translation to the repository because the translation file have to be renamed from *.json to strings_*.i18n.json

On top of that, the translation need to be clean up due to missing translation in base language

@oSumAtrIX
Copy link
Member

Alright, in this case, you can remove the translations via a commit which will be squashed with the rest of the commits

@validcube validcube linked an issue Nov 5, 2023 that may be closed by this pull request
4 tasks
@Ushie
Copy link
Member Author

Ushie commented Nov 5, 2023

  • Please work on the translation popup, the lack of actions makes it look odd so you can add an "OK" button or such
  • Is there any way to use friendly language names? e.g. English (US)

@oSumAtrIX oSumAtrIX changed the title feat: allow changing languages feat: Allow changing languages Nov 5, 2023
@oSumAtrIX
Copy link
Member

oSumAtrIX commented Nov 5, 2023

@Ushie Can you assign the person who you refer to in this PR?

@validcube
Copy link
Member

Please work on the translation popup, the lack of actions makes it look odd so you can add an "OK" button or such
Is there any way to use friendly language names? e.g. English (US)

🥞 All of these have been fixed in 3c18c4e.

Copy link
Contributor

@Aunali321 Aunali321 left a comment

Choose a reason for hiding this comment

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

Nice work!, looks good to me

@validcube validcube mentioned this pull request Nov 10, 2023
8 tasks
Copy link
Member

@validcube validcube left a comment

Choose a reason for hiding this comment

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

🥞 Approved :troll, self-approved

Copy link
Member

@validcube validcube left a comment

Choose a reason for hiding this comment

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

The only thing that left is to resolve conflicts and rebase changes on of latest dev branch

@validcube validcube merged commit 92bc174 into dev Nov 11, 2023
1 check passed
@revanced-bot revanced-bot deleted the feat/l10n branch November 11, 2023 13:25
Ushie added a commit that referenced this pull request Nov 11, 2023
Ushie added a commit that referenced this pull request Nov 11, 2023
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.

Addition of Japanese translation
4 participants