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

Add Korean localization #494

Merged
merged 4 commits into from
Sep 27, 2023
Merged

Add Korean localization #494

merged 4 commits into from
Sep 27, 2023

Conversation

LIMMIHEE
Copy link
Contributor

@LIMMIHEE LIMMIHEE commented Sep 16, 2023

Hi

I want Korean support, but in the case of the existing Pull Request, the process has been stopped since then, so I will improve the sentence more naturally and request it again.

please review it.
Thank you.

Test.Recording13.40.49.mp4

@AlexV525
Copy link
Member

@LeGoffMael Can you help review the language?

@LeGoffMael
Copy link
Collaborator

I think the translation is good and also it uses the proper language code ko that the other PR was not using (c.f. locale list).
I added a single suggestion regarding accessAllTip

const KoreanAssetPickerTextDelegate();

@override
String get languageCode => 'kr';
Copy link
Member

Choose a reason for hiding this comment

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

kr stands for Kanuri? This should be ko?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In Korea, I used 'kr' more often and used the code, but I changed it to 'ko' following the review!

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure you'll get the Korean localization if you put Locale('kr') in Flutter. Can you identify that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but based on the comment LeGoffMael wrote, it looks better to apply it as 'ko'

@AlexV525
Copy link
Member

Could you also test with TalkBack/VoiceOver and provide a corresponding screen recording (with the accessibility speaker sound)? This will make sure sentences make sense when reading for disabled people.

@LIMMIHEE
Copy link
Contributor Author

@AlexV525

I've never used the TalkBack/VoiceOver, so the movement might be awkward. is it okay?

RPReplay_Final1695272629.mov
33c92dc3a5b0a523febc.1.mp4

Copy link
Member

@AlexV525 AlexV525 left a comment

Choose a reason for hiding this comment

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

Thanks for the new language

@AlexV525
Copy link
Member

I've never used the TalkBack/VoiceOver, so the movement might be awkward. is it okay?

@LeGoffMael Could you verify if the sentences are appropriate in screen recordings?

@LeGoffMael
Copy link
Collaborator

I've never used the TalkBack/VoiceOver, so the movement might be awkward. is it okay?

@LeGoffMael Could you verify if the sentences are appropriate in screen recordings?

Since @LIMMIHEE shared a screen recording from an iPhone, here is how it looked like on an android. It seems good to me

XRecorder_27092023_011342.mp4

@AlexV525 AlexV525 merged commit 74ce214 into fluttercandies:main Sep 27, 2023
4 checks passed
@AlexV525
Copy link
Member

@all-contributors Add @LIMMIHEE for translation, add @LeGoffMael for translation.

@allcontributors
Copy link
Contributor

@AlexV525

I've put up a pull request to add @LIMMIHEE! 🎉

AlexV525 pushed a commit that referenced this pull request Sep 27, 2023
Adds @LIMMIHEE as a contributor for translation.

This was requested by AlexV525 [in this
comment](#494 (comment))

[skip ci]

---------

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
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.

3 participants