Skip to content

Add gitalk comment system - #464

Merged
sli1989 merged 10 commits into
theme-next:masterfrom
jactor-sue:master
Nov 29, 2018
Merged

Add gitalk comment system#464
sli1989 merged 10 commits into
theme-next:masterfrom
jactor-sue:master

Conversation

@jactor-sue

@jactor-sue jactor-sue commented Nov 4, 2018

Copy link
Copy Markdown
Contributor

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).
    • I don't know where to update docs.

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): N/A

What is the new behavior?

Description about this pull, in several words...

How to use?

1. Create new Github Application

Create a new Github application at Register a new OAuth application, remember what you inputed, and then you will get Client ID and Client Secret used later.

2. Enable Gitalk

Enable gitalk in NexT _config.yml, put Client ID and Client Secret down here:

gitalk:
  enable: true
  githubID:  # Github repo owner
  repo:  # Repository name to store issues.
  ClientID:  # Github Application Client ID
  ClientSecret:  # Github Application Client Secret
  adminUser:  # GitHub repo owner and collaborators, only these guys can initialize github issues
  distractionFreeMode: true # Facebook-like distraction free mode
...

Does this PR introduce a breaking change?

  • Yes.
  • No.

@jactor-sue

jactor-sue commented Nov 4, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

The file md5.js under /source/js/src/ is third-party lib, source from https://github.com/blueimp/JavaScript-MD5 by Sebastian Tschan . This is used to hash url to fixed length string, and it works well in the tests for all themes.

@maple3142

maple3142 commented Nov 4, 2018

Copy link
Copy Markdown
Contributor

Both gitalk and md5.js should be able to configure cdn by _config.yml
Besides, there has been serveral attempts to add gitalk:
iissnan/hexo-theme-next#1814
iissnan/hexo-theme-next#2037
#58
#84
#370

@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 4, 2018
@theme-next theme-next deleted a comment Nov 9, 2018
@theme-next theme-next deleted a comment Nov 9, 2018
@theme-next theme-next deleted a comment Nov 9, 2018
@theme-next theme-next deleted a comment Nov 9, 2018
@theme-next theme-next deleted a comment Nov 9, 2018
@theme-next theme-next deleted a comment Nov 9, 2018
@theme-next theme-next deleted a comment Nov 9, 2018
@theme-next theme-next deleted a comment Nov 9, 2018
@theme-next theme-next deleted a comment Nov 9, 2018
@theme-next theme-next deleted a comment Nov 9, 2018
@theme-next theme-next deleted a comment Nov 9, 2018
@theme-next theme-next deleted a comment Nov 9, 2018
maple3142
maple3142 approved these changes Nov 10, 2018
View reviewed changes
Comment thread layout/_third-party/comments/gitalk.swig Outdated
<script type="text/javascript" src="{{ theme.gitalk.gitalk_cdn_prefix }}gitalk.min.js"></script>
<script type="text/javascript" src="{{ theme.gitalk.md5_cdn }}"></script>

{% set gitalk_js_url = "//cdn.bootcss.com/gitalk/1.4.0/gitalk.min.js" %}

@maple3142 maple3142 Nov 8, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

According to #105, it will be better to use jsDelivr as default cdn.

Comment thread _config.yml Outdated
# Example: https://cdn.jsdelivr.net/npm/valine@1.1.8/dist/Valine.min.js
valine:

# Internal version:

@maple3142 maple3142 Nov 8, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there any reason not to include a internal version of gitalk?

@ivan-nginx ivan-nginx Nov 10, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, i think need to include internal version.

@jactor-sue jactor-sue Nov 10, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed, both gitalk and md5 cdn source from jsDelivr now.

@jactor-sue

jactor-sue commented Nov 19, 2018

Copy link
Copy Markdown
Contributor Author

Then, what's the next step?

@stevenjoezhang stevenjoezhang mentioned this pull request Nov 26, 2018
Closed
11 tasks
@sli1989 sli1989 added this to the v6.6.0 milestone Nov 29, 2018
@sli1989
sli1989 merged commit fa99660 into theme-next:master Nov 29, 2018
@ivan-nginx ivan-nginx mentioned this pull request Dec 12, 2018
Closed
15 tasks
@stevenjoezhang stevenjoezhang changed the title add gitalk comment system Add gitalk comment system Oct 20, 2019
tongluyang pushed a commit to tongluyang/hexo-theme-next that referenced this pull request Nov 19, 2019
@jactor-sue Alex LEE
add gitalk comment system (theme-next#464)
2232c54 
* add gitalk comment system

* update cdn for gitalk

* update vendors config and fix typos

* remove local md5.js

* update cdn source

* Revert "update cdn source"

This reverts commit 8175b46.

* update cdn source

* update md5 cdn
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ivan-nginx ivan-nginx ivan-nginx left review comments
@maple3142 maple3142 maple3142 approved these changes

Assignees

No one assigned

Labels

📖 Docs ⭐ Feature

Projects

None yet

Milestone

v6.6.0

Development

Successfully merging this pull request may close these issues.

4 participants

@jactor-sue @maple3142 @sli1989 @ivan-nginx