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 comment text #186

Merged
merged 4 commits into from
Mar 19, 2018
Merged

Add comment text #186

merged 4 commits into from
Mar 19, 2018

Conversation

sli1989
Copy link
Collaborator

@sli1989 sli1989 commented Mar 18, 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?

At present, there is a loss in comment text.

Facebook comments has added already (example).

https://github.com/sli1989/hexo-theme-next/blob/e6d4dd5ad7f178708754e0bab01863f267c7a900/layout/_macro/post.swig#L139

Disqus auto return the Comments: 1 Comment example, 2 Comments , 6 Comments

Hypercomments (example) & changyan (example) & valine & gitment are needed to add comment text.

gitment example

2

Issue Number(s): N/A

What is the new behavior?

To keep consisting of disqus comment (auto display), the style of commentCount+Comments in <a </a> as hyperlink is added in hypercomments/changyan/gitment/valine. And change comments to Comments in facebook comments.

  • Screens with this changes:

valine

1

2

How to use?

In NexT _config.yml:

...

Does this PR introduce a breaking change?

  • Yes.
  • No.

@ivan-nginx
Copy link
Member

ivan-nginx commented Mar 18, 2018

No. In some comments (e.g. Disqus) label «Comment(s)» added automatically. Check it.

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 18, 2018

i have thought about disqus as exception. facebook and changyan comment are needed to check.
there is a abnormal indent...😂

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 19, 2018

Confirm in all existed comment systems.

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.

Ok, i think need to add in post meta or somewhere:
If mobile style - icon only, without label.

And second: 1 comment, 2 comment(s) — how about this?

@@ -136,7 +136,7 @@
<i class="fa fa-comment-o"></i>
</span>
<a href="{{ url_for(post.path) }}#comments" itemprop="discussionUrl">
<span class="post-comments-count fb-comments-count" data-href="{{ post.permalink }}" itemprop="commentCount">0</span> comments
<span class="post-comments-count fb-comments-count" data-href="{{ post.permalink }}" itemprop="commentCount">0</span> Comments
Copy link
Member

Choose a reason for hiding this comment

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

What about here? It is not translated text?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh, it's a mistake. I will fix it.

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 19, 2018

If mobile style - icon only, without label.

At present, it follows the display rules of Post meta display settings. The text will not display in mobile.

hexo-theme-next/_config.yml

Lines 250 to 257 in 1975fd0

# Post meta display settings
post_meta:
item_text: true
created_at: true
updated_at: false
# Only show 'updated' if different from 'created'.
updated_diff: false
categories: true

.post-meta-item-text {
+tablet() {
display: none;
}
+mobile() {
display: none;
}
}

And second: 1 comment, 2 comment(s) — how about this?

The disqus comment returns '* Comments' automatically. I think it's hard to unify them. Other comment systems can suit the singular and plural forms according to the tags/categories count.

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

@ivan-nginx
Copy link
Member

ivan-nginx commented Mar 19, 2018

The disqus comment returns '* Comments' automatically. I think it's hard to unify them. Other comment systems can suit the singular and plural forms according to the tags/categories count.

Yes, it is. I'm talking about this at the begining of this pull. And impossible to resolve how many comments count will and there is bug in languages:

  1. 0 comments
    1 comments
    5 comments
  2. 0 комментариев
    1 комментарий
    2 комментария
    5 комментариев

About Chinese IDK, but i think there is 2 variants, like in EN language.

So, how about this?

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 19, 2018

I don't know how to get the element of comment count, for valine: <span class="post-comments-count valine-comment-count" data-xid="{{ url_for(post.path) }}" itemprop="commentCount"></span>...

@ivan-nginx
Copy link
Member

ivan-nginx commented Mar 19, 2018

Nohow. 😄 This is 3rd-party JS applications with dynamic values changing, NexT can't resolve this data during generate.


I suggest something like:
Comments: X

So, it will be:

  1. Comments: 0
    Comments: 1
    Comments: 5
  2. Комментариев: 0
    Комментариев: 1
    Комментариев: 2
    Комментариев: 5

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 19, 2018

Can the comment number be counted like this?

<div class="tag-cloud-title">
{% set visibleTags = 0 %}
{% for tag in site.tags %}
{% if tag.length %}
{% set visibleTags += 1 %}
{% endif %}
{% endfor %}
{{ _p('counter.tag_cloud', visibleTags) }}
</div>

@ivan-nginx
Copy link
Member

They can be counted like this, yep. But only if it's internal NexT or Hexo plugin(s). Comments services are external.

@ivan-nginx
Copy link
Member

ivan-nginx commented Mar 19, 2018

MB symbol : from tranlsate?

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 19, 2018

OK, set the text in or out of hyperlink ?

@ivan-nginx
Copy link
Member

Replace &#58 with {{ __('symbol.colon') }}.

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 19, 2018

I say the style of text. Comments: 1 or Comments: 1

@ivan-nginx
Copy link
Member

Oh. IDK. See as will be better. FB comments was out of the link.

@ivan-nginx
Copy link
Member

ivan-nginx commented Mar 19, 2018

Let's see how in Wordpress?


It seems in WP used second variant: Comments: 1.
So, yeah, i think good click on whole word, not to target mouse just in 1 digit.

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 19, 2018

Within the hyperlink. That's it.

@ivan-nginx
Copy link
Member

Yep, i see. Nice!
image

@sli1989 sli1989 added this to the v6.0.7 milestone Mar 19, 2018
@sli1989 sli1989 merged commit 55bb2c7 into theme-next:master Mar 19, 2018
@sli1989 sli1989 deleted the comment-text branch March 19, 2018 08:39
geekrainy pushed a commit to geekrainy/theme-next-geekrainy that referenced this pull request Apr 28, 2018
* Add comment text

* Update the comment text in hyperlink

* Update comment text's style

* Add symbol.colon in comment text
tongluyang pushed a commit to tongluyang/hexo-theme-next that referenced this pull request Nov 19, 2019
* Add comment text

* Update the comment text in hyperlink

* Update comment text's style

* Add symbol.colon in comment text
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants