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

feat(docs): add Chinese translation (jump link) #5839

Merged
merged 3 commits into from Mar 13, 2024

Conversation

100ask
Copy link
Contributor

@100ask 100ask commented Mar 12, 2024

I created this PR with a new branch, the old PR: #5750

kisvegabor
kisvegabor previously approved these changes Mar 12, 2024
Copy link
Member

@kisvegabor kisvegabor left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you! 😊

Copy link
Collaborator

@PGNetHun PGNetHun left a comment

Choose a reason for hiding this comment

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

  1. Some minor changes needed
  2. Sorry, but I think we should show the external link ( https://lvgl.100ask.net/ ) on the home page, or in header/footer instead of adding same link_to_translation to every doc file. New doc files will not contain it (ppl might forget to add), and btw there could be other translations too, not only chinese, so having a link in header/footer/sidenav to external sites with the translations is better.

docs/_ext/link_roles.py Outdated Show resolved Hide resolved
docs/integration/framework/arduino.rst Outdated Show resolved Hide resolved
@100ask
Copy link
Contributor Author

100ask commented Mar 13, 2024

2. instead of adding same link_to_translation to every doc file. New doc files will not contain it (ppl might forget to add),

Good suggestion! I will improve it today.

@100ask
Copy link
Contributor Author

100ask commented Mar 13, 2024

2. and btw there could be other translations too

What do you think? @kisvegabor

Copy link
Collaborator

@PGNetHun PGNetHun left a comment

Choose a reason for hiding this comment

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

Good changes, thanks, only some minor comments are added.

PS:
you don't have to squash commits in your branch, simply add a new commit, so reviewers can see what is changed since last review (since last commit).
Then after PR is approved, we do a squash merge, and in the end only 1 commit will go to master branch. But till then it is good to see all commits and change commits in feature branch.

docs/_ext/link_roles.py Outdated Show resolved Hide resolved
docs/add_translation.py Outdated Show resolved Hide resolved
@100ask 100ask requested a review from PGNetHun March 13, 2024 08:24
@kisvegabor
Copy link
Member

Sorry, but I think we should show the external link ( https://lvgl.100ask.net/ ) on the home page, or in header/footer instead of adding same link_to_translation to every doc file.

What about near to the image link?

@100ask
Copy link
Contributor Author

100ask commented Mar 13, 2024

Sorry, but I think we should show the external link ( https://lvgl.100ask.net/ ) on the home page, or in header/footer instead of adding same link_to_translation to every doc file.

What about near to the image link?

  1. Not obvious, may not notice.
  2. When reading on a small screen, it becomes difficult, even invisible:
    image

I think it's appropriate to put it at the beginning of the text:
image

@PGNetHun
Copy link
Collaborator

Yes, I think at the moment it is enough to add to top of page (like in ESP-IDF), so later we might implement other option.

(I personally prefer having a language selector dropdown / button on top of sidenav, or top-right of page, that we don't need to search, and does not reserve/waste too much space on screen)

docs/add_translation.py Show resolved Hide resolved
Copy link
Member

@kisvegabor kisvegabor left a comment

Choose a reason for hiding this comment

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

It's unlikely that we will add more translations in the future due to maintenance issues. China is special because:

  • We have a lot of users in China
  • Many people don't know English
  • Websites outside of China are super slow

@kisvegabor kisvegabor merged commit 442efee into lvgl:master Mar 13, 2024
16 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.

None yet

4 participants