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

Korean translation for package-manager.md #1979

Merged
3 commits merged into from
Feb 3, 2019
Merged

Korean translation for package-manager.md #1979

3 commits merged into from
Feb 3, 2019

Conversation

taggon
Copy link
Contributor

@taggon taggon commented Jan 10, 2019

This PR fixes some missing Korean translations in #1832.

ref nodejs/nodejs-ko#763
cc @nodejs/nodejs-ko

@taggon taggon added the i18n Issues/PRs related to the Website Internationalisation label Jan 10, 2019
@taggon taggon self-assigned this Jan 10, 2019
Copy link
Contributor

@yous yous left a comment

Choose a reason for hiding this comment

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

LGTM but one comment.

locale/ko/download/package-manager.md Outdated Show resolved Hide resolved
@yous
Copy link
Contributor

yous commented Jan 10, 2019

Is there a way to enforce to generate anchor with specified name? Variations between languages can make some confusions.

@yous
Copy link
Contributor

yous commented Jan 31, 2019

@taggon Can you give me a feedback of the suggestion?

@ghost
Copy link

ghost commented Feb 3, 2019

@taggon & @yous:The reason is that if you use "##" to generate an anchor and there's something out of 26 English alphabets, it will be ignored. And I've submitted this fixture you can have a review for that, this can keep what it is in the English version.

See:#2028. Hope it helps you :)

Use the pure HTML to fix the anchor link
@ghost
Copy link

ghost commented Feb 3, 2019

@yous:Help you to fix this anchor link problem, and you all can continue with #1994 :)

@yous
Copy link
Contributor

yous commented Feb 3, 2019

@Maledong It seems that h2 has the id, and a has an additional aria-labelledby attribute on the English site: https://nodejs.org/en/download/package-manager/

<h2 id="header-android">Android<a name="android" class="anchor" href="#android" aria-labelledby="header-android"></a></h2>

@ghost
Copy link

ghost commented Feb 3, 2019

@yous:Yeah, I've noticed that but it doesn't effect, and everything seems fine, if we keep the name, link the same name as what we see in the title.

@yous
Copy link
Contributor

yous commented Feb 3, 2019

@Maledong Got it, thanks! I think this is ready to be merged.

@yous
Copy link
Contributor

yous commented Feb 3, 2019

Oh wait, the file has multiple headers with CJK.

@ghost
Copy link

ghost commented Feb 3, 2019

@yous:No problem. If anything else welcome to have a discussion with me or all of you here to fix it:)

@yous
Copy link
Contributor

yous commented Feb 3, 2019

@Maledong Thank you for participating and fixing this issue! Now it's really ready to be merged.

@ghost ghost merged commit f0c1874 into nodejs:master Feb 3, 2019
@ghost
Copy link

ghost commented Feb 3, 2019

Nice! Thanks and I've helped to merge your change.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n Issues/PRs related to the Website Internationalisation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants