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

Fix: Correct import path and Add Chinese on Giscus language #144

Merged
merged 3 commits into from
Feb 2, 2025

Conversation

clicelee
Copy link
Contributor

@clicelee clicelee commented Jan 31, 2025 β€’

πŸ“ Key Changes

  1. Fixed a typo in docs: 간체 쀑ꡭ어 λ²ˆμ—­ #120 where ch.mjs was incorrectly used. Changed it to ch.mts
  • changed file: config.mts
  1. Applied the approach defined in Feat: Improve Giscus theme visibility and add i18n support #127 for syncing Giscus language settings.
  • changed file: utils/index.ts
    • Added ch: "zh-CN" to GISCUS_LANG_MAP.
    • Reference: this comment.

πŸ€” Discussion

  • The Chinese documentation is currently labeled as ch, but according to ISO 639-1, zh might be more appropriate.
    • However, this does not cause any readability issues.
    • Since renaming the folder and files would take considerable effort, it would be best to decide based on the project’s direction and purpose. I’d love to hear your thoughts on this.

πŸ–ΌοΈ Before and After Comparison

Key Change 2: added chinese to GISCUS_LANG_MAP. utils/index.ts

Before After

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
* Fixed an issue where the Chinese locale was not applied in Giscus
* Fixed an incorrect import path in  ( β†’ ).
Copy link

vercel bot commented Jan 31, 2025

@clicelee is attempting to deploy a commit to the Toss Team on Vercel.

A member of the Team first needs to authorize it.

@clicelee clicelee changed the title Fix/giscus lang Fix: Correct import path and Add Chinese on Giscus language Jan 31, 2025
@Seol-JY
Copy link
Contributor

Seol-JY commented Jan 31, 2025 β€’

Like in other Toss library documents, I also thought it would be better to use more standard language codes like zh-Hans(preferred) or zh-CN that represent Simplified Chinese rather than ch.

If we were to make changes, I think we should use lowercase for these language codes (zh-hans, zh-cn) rather than mixed case (zh-Hans, zh-CN) since they're used in URL paths. This ensures consistent behavior across different systems and follows established URL standards.

Copy link
Collaborator

@milooy milooy left a comment

Choose a reason for hiding this comment

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

Thank you for the detailed explanation @clicelee , @Seol-JY πŸ‘

❌ ch is non-standard and should not be used.
❌ Using zh alone cannot distinguish between Simplified and Traditional Chinese.
βœ… zh-Hans is recommended as it covers all Simplified Chinese users.
🌍 For URLs, using lowercase (zh-hans) is more appropriate (e.g., example.com/zh-hans/).

It'll be great if you can make another PR for changing folder name /ch β†’ /zh-hans .

@milooy milooy merged commit a6884a6 into toss:main Feb 2, 2025
1 check failed
@clicelee
Copy link
Contributor Author

clicelee commented Feb 2, 2025

I'll do it right away πŸ‘

Kyujenius pushed a commit to Kyujenius/frontend-fundamentals that referenced this pull request Feb 3, 2025
* Fix: Ensure Giscus supports Chinese
* Fixed an issue where the Chinese locale was not applied in Giscus

* Fix: Correct import path for Chinese locale
* Fixed an incorrect import path in  ( β†’ ).

* Revert: yarn.lock
milooy pushed a commit that referenced this pull request Feb 4, 2025
* fix: change LCP image from png to webp

* docs: 간체 쀑ꡭ어 λ²ˆμ—­ (#120)

* chore: create ch files

* docs: translate into simplified chinese

* docs: translate into simplified chinese

* docs: translate into simplified chinese

* docs: translate into simplified chinese

* docs: translate into simplified chinese

* docs: translate into simplified chinese

* refactor: substitute with suitable terms

* docs: translate into simplified chinese

* feat: add chSearch

* fix: change chSearch option

* fix: fix typo with inspection

* οΏ½fix: fix typo in hidden-logic.md

εŒζ—Ά -> εŒδΊ‹

Co-authored-by: Jinyeong Seol <[email protected]>

* feat: implement dark mode image support in markdown

* docs: improve item-edit-modal.md by adding abstraction concepts

* fix: correct localized link

---------

Co-authored-by: Jinyeong Seol <[email protected]>

* Fix: Corrected localized link to Amazon (#141)

- The link was still redirecting to Yes24, so the link was updated properly

* Fix: Correct import path and Add Chinese on Giscus language  (#144)

* Fix: Ensure Giscus supports Chinese
* Fixed an issue where the Chinese locale was not applied in Giscus

* Fix: Correct import path for Chinese locale
* Fixed an incorrect import path in  ( β†’ ).

* Revert: yarn.lock

* οΏ½Fix: Add missing syntax and normalize Auth Token usage (#145)

* fix: Add missing 'from' keyword in import statement

* fix: Add missing period in email validation error message(ko)

* fix: add auth token to HTTP header

* fix: resolve badge component rendering issue by extending default theme (#146)

* feat: add webp Image in chinese ver.

---------

Co-authored-by: Owon <[email protected]>
Co-authored-by: Jinyeong Seol <[email protected]>
Co-authored-by: 𝒄𝒍 π’Š 𝒄 𝒆 <[email protected]>
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.

None yet

3 participants