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 count in mobile #185

Merged
merged 2 commits into from
Mar 19, 2018
Merged

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?

The comment count in mobile doesn't display. It's a necessary meta, so display it without a enable option.

Issue Number(s): #180

What is the new behavior?

Add the count meta in post.

  • Screens with this changes:

before
_20180318202823
after

_20180318202846

_20180318210916

_20180318211012

_20180318211138

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.

Other schemes?

@LEAFERx
Copy link
Contributor

LEAFERx commented Mar 18, 2018

recommend to add switch in _config.yml post meta

@ivan-nginx
Copy link
Member

I still can't understand why it was added to disabled by default? Trafic economy or what? What's the reason?

@LEAFERx
Copy link
Contributor

LEAFERx commented Mar 18, 2018

dont know either

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 18, 2018

All tested. The screenshots are added.

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 18, 2018

recommend to add switch in _config.yml post meta

I think it's a necessary meta.

@ivan-nginx
Copy link
Member

ivan-nginx commented Mar 18, 2018

Comments system? All tested? Again: need to try why it was added to disabled by default?
If all fine, that's good. But if not? Need to test it carefully. I don't think what it was maded by Vi simply for trafic economy or something like that.


Here is history, but i still no found any arguments on this addition (hide in mobile).


Ok, test it on other possible popular comments system and i think we can merge it. Strange situation.

@ivan-nginx ivan-nginx added this to the v6.0.7 milestone Mar 18, 2018
@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 18, 2018

OK, more comment systems would be tested.

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 18, 2018

Gitment comment system works fine. The comment count display with Disqus in homepage but not in post page. Check this demo.

@ivan-nginx
Copy link
Member

Disqus appear not instantly at most time and for now i see comments count in homepage and in post page too. So, i think it's fine.

@sli1989
Copy link
Collaborator Author

sli1989 commented Mar 18, 2018

OK, I doesn't use disqus. It seems all fine.

@sli1989 sli1989 merged commit 65a8a8d into theme-next:master Mar 19, 2018
@sli1989 sli1989 deleted the mobile-comment-count branch March 19, 2018 08:43
geekrainy pushed a commit to geekrainy/theme-next-geekrainy that referenced this pull request Apr 28, 2018
tongluyang pushed a commit to tongluyang/hexo-theme-next that referenced this pull request Nov 19, 2019
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.

3 participants