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 darkmode of DisqusJS #408

Merged
merged 2 commits into from
Nov 30, 2021
Merged

Add darkmode of DisqusJS #408

merged 2 commits into from
Nov 30, 2021

Conversation

ljcbaby
Copy link
Member

@ljcbaby ljcbaby commented Nov 17, 2021

PR Checklist

  • The commit message follows guidelines for NexT.
  • 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 features).

PR Type

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

What is the current behavior?

Issue resolved: #141

What is the new behavior?

  • Link to demo site with this changes: None
  • Screenshots with this changes:
    image

@coveralls
Copy link

coveralls commented Nov 17, 2021

Pull Request Test Coverage Report for Build 1520242562

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 94.468%

Totals Coverage Status
Change from base Build 1411888750: 0.0%
Covered Lines: 138
Relevant Lines: 141

💛 - Coveralls

@stevenjoezhang
Copy link
Member

I personally recommend adapting the dark mode in the source code of DisqusJS itself, instead of NexT. This can benefit other developers and users who use disqusjs, even if they use other themes or frameworks.

@ljcbaby
Copy link
Member Author

ljcbaby commented Nov 26, 2021

The native dark mode is mostly implemented in the color of the inherited theme, however SukkaW wraps all elements in order not to pollute.

SukkaW/DisqusJS#40 (comment)

This Pull Request is an exact copy of SukkaW's blog, with the color names adjusted.

@ljcbaby
Copy link
Member Author

ljcbaby commented Nov 26, 2021

It looks like skk doesn't want to build in a dark mode, otherwise it would have done that already. But I also agree that a dark mode should be built in or a basic template should be given for adaptation.

@@ -0,0 +1,39 @@
if (hexo-config('disqusjs.enable') and hexo-config('darkmode')) {
@media (prefers-color-scheme:dark) {
html #dsqjs a {
Copy link
Member

Choose a reason for hiding this comment

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

The indentation seems incorrect here, should be 2 spaces

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@stevenjoezhang stevenjoezhang merged commit 847fc59 into master Nov 30, 2021
@stevenjoezhang stevenjoezhang deleted the dsqjs-darkmode branch November 30, 2021 14:29
lingyf pushed a commit to lingyf/hexo-theme-next that referenced this pull request Jan 27, 2022
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.

Adapt to the dark mode of DisqusJS
3 participants