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

Chinese translation of Accessibility chapters and templates bugs fix #1139

Merged
merged 74 commits into from
Aug 12, 2020

Conversation

chengxicn
Copy link
Contributor

@chengxicn chengxicn commented Aug 1, 2020

Makes progress on #1067

chengxicn and others added 30 commits July 17, 2020 23:24
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
@tunetheweb
Copy link
Member

One more request. Could you change the first line of featured_chapters.html from this:

{%- set featured_chapter = ("accessibility","caching","cdn","cms","compression","css","ecommerce","fonts","http2","javascript","markup","media","mobile-web","page-weight","performance","pwa","resource-hints","security","seo","third-parties") | random %}

to this?:

{%- set featured_chapter = ("accessibility","accessibility") | random %}
{#
  Below is the full set of chapters. If all are translated then replace above line with this one.
  Until then, add chapters to above first line as they are translated (min of two chapters so repeat if only one chapter)
  {%- set featured_chapter = ("accessibility","caching","cdn","cms","compression","css","ecommerce","fonts","http2","javascript","markup","media","mobile-web","page-weight","performance","pwa","resource-hints","security","seo","third-parties") | random %}
#}

This will mean only the translated Accessibility chapter will show as a random featured chapter on the hope page - no point in showing a featured chapter that isn't fully translated yet as that will only disappoint those that click on it! We can add to this list as you translate more chapters. Note also the random function requires at least two in the list hence why we repeat accessibility twice for now.

Let me know when good for me to re-review!

@chengxicn
Copy link
Contributor Author

One more request. Could you change the first line of featured_chapters.html from this:

{%- set featured_chapter = ("accessibility","caching","cdn","cms","compression","css","ecommerce","fonts","http2","javascript","markup","media","mobile-web","page-weight","performance","pwa","resource-hints","security","seo","third-parties") | random %}

to this?:

{%- set featured_chapter = ("accessibility","accessibility") | random %}
{#
  Below is the full set of chapters. If all are translated then replace above line with this one.
  Until then, add chapters to above first line as they are translated (min of two chapters so repeat if only one chapter)
  {%- set featured_chapter = ("accessibility","caching","cdn","cms","compression","css","ecommerce","fonts","http2","javascript","markup","media","mobile-web","page-weight","performance","pwa","resource-hints","security","seo","third-parties") | random %}
#}

This will mean only the translated Accessibility chapter will show as a random featured chapter on the hope page - no point in showing a featured chapter that isn't fully translated yet as that will only disappoint those that click on it! We can add to this list as you translate more chapters. Note also the random function requires at least two in the list hence why we repeat accessibility twice for now.

Let me know when good for me to re-review!

First line updated, thanks for reviewing!

@tunetheweb
Copy link
Member

Ready for me to re-review? Or still changing things?

@chengxicn
Copy link
Contributor Author

Ready for me to re-review? Or still changing things?

Sorry, OK now,won't change in this PR, please review. I'll create a new branch next time to reduce noise.

Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

Spotted a few more things!

src/content/zh-CN/2019/accessibility.md Outdated Show resolved Hide resolved
src/content/zh-CN/2019/accessibility.md Outdated Show resolved Hide resolved
src/content/zh-CN/2019/accessibility.md Outdated Show resolved Hide resolved
src/content/zh-CN/2019/accessibility.md Show resolved Hide resolved
src/content/zh-CN/2019/accessibility.md Show resolved Hide resolved
src/content/zh-CN/2019/accessibility.md Outdated Show resolved Hide resolved
src/templates/zh-CN/2019/methodology.html Outdated Show resolved Hide resolved
src/templates/zh-CN/2019/methodology.html Outdated Show resolved Hide resolved
src/templates/zh-CN/2019/methodology.html Outdated Show resolved Hide resolved
src/templates/zh-CN/2019/table_of_contents.html Outdated Show resolved Hide resolved
@chengxicn
Copy link
Contributor Author

I'm checking on all the CN links, it will take a while.

@chengxicn
Copy link
Contributor Author

chengxicn commented Aug 8, 2020

done, searched and deleted spaces, links updated, but Chinese commas are a little different from English ,so didn't commit the suggestions.

Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

Looks great - thanks so much for working through all the feedback!

Just one more small question.

@limhenry do you have time to check the actual translation now I'm happy with the technicalities?

src/content/zh-CN/2019/accessibility.md Outdated Show resolved Hide resolved
@tunetheweb tunetheweb merged commit 5c2cefc into HTTPArchive:main Aug 12, 2020
@tunetheweb
Copy link
Member

I’ve merged this as planning on doing a release. @limhenry if you get a chance to review later and spot anything than can you open another pull request to correct?

@limhenry
Copy link
Member

@chengxicn is it better to use ":" instead of ":" ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
translation world wide web
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants