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: locales: zh-CN, zh-HK, zh-TW #751

Merged
merged 4 commits into from
Dec 8, 2024
Merged

Conversation

Davidzhu001
Copy link
Contributor

I update the locale file of those three languages

@Davidzhu001
Copy link
Contributor Author

Davidzhu001 commented Dec 2, 2024

#610
#608
#609

@Davidzhu001 Davidzhu001 changed the title Add zh-TW, zh-HK, zh-TW pagination entry Add zh-CN zh-HK, zh-TW pagination entry Dec 2, 2024
@benkoshy
Copy link
Collaborator

benkoshy commented Dec 4, 2024

Thank you - your contribution is much appreciated.

Here is what I have checked / verified:

Step 1: Check the pluralization in rails-i18n and note it down.

  • zh-cn pluralisation is other.
  • zh-hk pluralisation is other
  • zh-tw pluralisation is other

All of the pluralization rules. are other.

Step 2: Check that pagy defines other

  • Yes it does. We do not need to define other because it is already there.

Step 3: Add an entry to the locales hash

  • Yes. This is done I see it in the PR:
                 hash['zh-CN'] = RULE[:other]
                 hash['zh-HK'] = RULE[:other]
                 hash['zh-TW'] = RULE[:other]

Step 4: Check all the .yml files for the correct entries:

I have made some modifications see the chinese locales branch here

  • All the nav entries default to 'other'. (Please confirm whether this is correct.)

@benkoshy

This comment was marked as resolved.

@Davidzhu001
Copy link
Contributor Author

I checked this branch https://github.com/ddnexus/pagy/tree/chinese-locales.
It seems you already added the translation and fixed it.

Should I close the pull request or what should I do next?

@benkoshy

This comment was marked as resolved.

remove: :other from aria_label key

change: item_name which had one_other keys to default to the :other key

remove: code comment
@benkoshy
Copy link
Collaborator

benkoshy commented Dec 5, 2024

@Davidzhu001 I have pushed onto your master branch and attributed you as author via your previously signed commits (and have deleted the previously chinese-locales branch). There is nothing more you need to do. Thank you for your contribution.

@ddnexus ready for merging, subject to your review.

@benkoshy benkoshy changed the title Add zh-CN zh-HK, zh-TW pagination entry Add: zh-CN zh-HK, zh-TW locales Dec 5, 2024
@benkoshy benkoshy changed the title Add: zh-CN zh-HK, zh-TW locales Add: locales: zh-CN, zh-HK, zh-TW Dec 5, 2024
@Davidzhu001
Copy link
Contributor Author

Thanks so much, i was little busy this week. sorry for the late response

@ddnexus ddnexus changed the base branch from master to dev December 8, 2024 11:10
@ddnexus ddnexus merged commit 1472557 into ddnexus:dev Dec 8, 2024
4 checks passed
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