Skip to content

Refactoring comments #711

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

Closed
wants to merge 26 commits into from
Closed

Refactoring comments #711

wants to merge 26 commits into from

Conversation

jiangtj
Copy link
Member

@jiangtj jiangtj commented Mar 19, 2019

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines.
  • Tests for the changes was maked (for bug fixes / features).
    • Muse | Mist have been tested.
    • Pisces | Gemini have been tested.
  • Docs in NexT website have been added / updated (for new 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 resolved: N/A

What is the new behavior?

I put all the comments code into a folder. So we can add comment plug-ins more easily. Only add or modify files in their folder.
image

But a lot of testing is needed, and incompatibility with the original configuration can affect many users.

So do you think we should make such a change?

How to use?

In NexT _config.yml:

+ # Global settings for comments system.
+ # You can set one type to enable.
+ # And need to provide additional configurations are required for those types.
+ # See doc: https://theme-next.org/docs/third-party-services/comments-and-widgets/
+ # Example:
+ #   comments:
+ #     type: Disqus # enable disqus.
+ #     count: 
+ #       page: true
+ #       post: true
+ # Additional configurations:
+ #   disqus:
+ #     shortname: shartname
+ #     lazyload: false
+ comments:
+   # Type list:
+   # disqus | disqusjs | changyan | valine | livere | gitment | gitalk | facebook_comments_plugin | vkontakte 
+   type:
+   # If comment system support, show comments count in meta area
+   count: 
+     page: true
+     post: true

disqus:
-   enable: false	
-   count: true

disqusjs:
-   enable: false

gitment:
-  enable: false

changyan:
-  enable: false

valine:
-  enable: false
-  comment_count: true # if false, comment count will only be displayed in post page, not in home page

gitment:
-   enable: false
-   count: true

gitalk:
-   enable: false

facebook_comments_plugin:
-   enable: false

vkontakte_api
-   comments:     true

Does this PR introduce a breaking change?

  • Yes.
  • No.

Sorry, something went wrong.

@jiangtj
Copy link
Member Author

jiangtj commented Mar 19, 2019

Another problem is that some plug-ins provide multiple services, like Facebook Comments and Likes and VKontakte Comments and Likes. So how to split config? Function or provider ?

Function:

# choice your comments service
comments_service: 
# choice your likes service
likes_service: 

Provider:

# choice VKontakte provider 
VKontakte: enable: true 
# choice Facebook provider 
Facebook: enable: true 

@stevenjoezhang
Copy link
Contributor

It might be better to split them according to function, like how we deal with Google calendar, Google analytics, Google site verification, etc

@jiangtj
Copy link
Member Author

jiangtj commented Mar 20, 2019

I've been a little busy recently, will test every comment plug-in over a period of time.
In addition, if you have any ideas about this, you can mention it here.

@jiangtj
Copy link
Member Author

jiangtj commented Mar 21, 2019

image

I need help to test Changyan. I don't have ICP Orz.

@jiangtj
Copy link
Member Author

jiangtj commented Mar 21, 2019

image

😰 Anybody can help me?
Just this and Changyan not tested....

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.

I'll test VK soon, ok.

@1v9
Copy link
Member

1v9 commented Mar 23, 2019

Does anyone need help?

@1v9 1v9 added this to the v7.1.0 milestone Mar 23, 2019
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.

It seems VK API must work's fine.

@stevenjoezhang
Copy link
Contributor

So only Changyan is needed to be tested now?

@theme-next theme-next deleted a comment Apr 10, 2019
@theme-next theme-next deleted a comment Apr 10, 2019
@theme-next theme-next deleted a comment Apr 10, 2019
@theme-next theme-next deleted a comment Apr 10, 2019
@theme-next theme-next deleted a comment Apr 10, 2019
@theme-next theme-next deleted a comment Apr 10, 2019
@theme-next theme-next deleted a comment Apr 10, 2019
@theme-next theme-next deleted a comment Apr 10, 2019
@theme-next theme-next deleted a comment Apr 10, 2019
@theme-next theme-next deleted a comment Apr 10, 2019
@theme-next theme-next deleted a comment Apr 10, 2019
@jiangtj jiangtj removed this from the v7.1.1 milestone May 9, 2019
@stevenjoezhang
Copy link
Contributor

@jiangtj Conflict needs to be resolved

@stevenjoezhang stevenjoezhang added this to the v7.2.0 milestone Jun 21, 2019
@stevenjoezhang stevenjoezhang removed this from the v7.2.0 milestone Jul 2, 2019
@jiangtj
Copy link
Member Author

jiangtj commented Jul 3, 2019

image
There are too many conflicting files 😂 , I'll refactor in future. Closed temporarily 😄

@jiangtj jiangtj closed this Jul 3, 2019
@stevenjoezhang
Copy link
Contributor

@jiangtj Then merge #764 first? 😂

@1v9
Copy link
Member

1v9 commented Jul 3, 2019

@stevenjoezhang You can merge it, but #764 needs a lot of improvement.

@stevenjoezhang
Copy link
Contributor

Any new progress here?

@jiangtj
Copy link
Member Author

jiangtj commented Jul 20, 2019

Any new progress here?

我还在考虑怎么弄,想将评论显示的位置也作为一个注入点,那样与post-meta body-end一起,对应文件夹下的那三个文件,而且用注入的方式理论上不会打破兼容性

但这样,就得到了一个数组,所以对多评论系统的支持这块,需要提前思考🤔

I'm still thinking about how to do it, and I want to use the location of the comment display as a injection point, so that, together with post-meta body-end, the three files under the folder, and the injection method doesn't theoretically break compatibility,

but in this way, I get an array, so the support for the multi-comment system needs to think about it in advance.

@jiangtj jiangtj deleted the comments branch July 23, 2019 10:22
whyliam pushed a commit to whyliam/hexo-theme-next that referenced this pull request Apr 19, 2025

Unverified

The key that signed this is expired.
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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

5 participants