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 hover text for PV UV #110

Closed
wants to merge 1 commit into from
Closed

add hover text for PV UV #110

wants to merge 1 commit into from

Conversation

xu-song
Copy link
Contributor

@xu-song xu-song commented Feb 3, 2018

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines.
  • Tests for the changes have been added (for bug fixes / features).
    • Muse | Mist have been tested.
    • Pisces | Gemini have been tested.
  • Docs have been added / updated (for bug fixes / features).

PR Type

What kind of change does this PR introduce?

  • Bugfix.
  • Feature.
  • Code style update (formatting, local variables).
  • Refactoring (no functional changes, no api changes).
  • Build related changes.
  • CI related changes.
  • Documentation content changes.
  • Other... Please describe:

What is the current behavior?

Issue Number(s): #108

image

What is the new behavior?

Add hover text for some confusing icon. In the above case , someone maybe be confusing with two date.
Hover text, such as “created” “modified” maybe more favorable in many cases.

How to use?

In NexT _config.yml:

...

Does this PR introduce a breaking change?

  • Yes.
  • No.

Copy link
Member

@ivan-nginx ivan-nginx left a comment

Choose a reason for hiding this comment

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

Please, add i18n support.

@xu-song
Copy link
Contributor Author

xu-song commented Feb 9, 2018

update

@ivan-nginx
Copy link
Member

ivan-nginx commented Feb 9, 2018

For example, here is i18n support for post.visitors translate:

<span class="post-meta-item-text">{{__('post.visitors')}}&#58;</span>

{{__('post.visitors')}}

This mean what this value will be read from current language in next/languges directory. If we will use language: zh-CN in main Hexo config, then this value will read from here:

visitors: 阅读次数

post:
  visitors: 阅读次数

E.g. post.visitors value.


So, no need to add language value into NexT config. Need just add __xxx parameter and add translates into at least default _en.yml and zh-CN.yml (or any other) language files. This called i18n multilanguage support.


Need to refactor all previous langs values from config to languages files:

site_uv_header: Site visited by
site_uv_footer: people.

site_pv_header: Site visited by
site_pv_footer: times.

page_pv_header: Post read by
page_pv_footer: times.

P.S. And maybe add %s to 1, 2, 5 digits strings rounding. Like here:

counter:
tag_cloud:
zero: No tags
one: 1 tag in total
other: "%d tags in total"

@sli1989
Copy link
Collaborator

sli1989 commented Feb 9, 2018

i have localized about hover test already. Not the display directly.


NO, i saw your suggestion above, It seems no need to add hover test.

@wafer-li
Copy link
Member

wafer-li commented Feb 9, 2018

@ivan-nginx

My opinion is to just leave the site_xx_header and site_xx_footer here and don't touch them.

Just provide the i18n for site_xx_title is enough and will be easy to be done.

Because there is a grammar issue:

When we say Site visited by 100 people, translated into Chinese will be 网站被 100 人所访问

And in en, it will be

site_uv_header: Site visited by
site_uv_footer: people.

But in zh-CN, it will be

site_uv_header: 网站被
site_uv_footer: 人所访问

Due to the grammar difference, I suggest not to touch the site_xx_header things, only provide i18n for the hover text(ie. the title property), to explain what the data is.

Eg:

site_uv_title: Site visitor count

@xu-song
Copy link
Contributor Author

xu-song commented Feb 9, 2018

For your issue, @wafer-li , footer is not required, we can choose another expression. such as 总访问量 总访客量. (Totlal visited. Unique visitor)

I have taken another PR which works well, and follows i18n #129 . This issue can be closed

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.

4 participants