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

Improve page rendering by deferring disqus loading #891

Merged
merged 2 commits into from
Jun 4, 2019

Conversation

KaitoHH
Copy link
Contributor

@KaitoHH KaitoHH commented Jun 1, 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 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: Improvement

What is the current behavior?

The whole web page is blocked and shows nothing until loading embed.js for Disqus is completed. Things get worse when some users (especially those Chinese users) have trouble connecting Disqus since they will see a blank page for a long time until the requested timeout.

image

In addition, for those who enable pace.js, they will see the progress bar circling for a long time until timeout.

What is the new behavior?

The main cause of this problem is that loading *.js for Disqus blocks the rendering of the whole web page, so I simply defer the js loading until the whole page loading is completed.

window.addEventListener('load', loadCount, false);

Here I use window.onload callback function instead of directly inserting script tag to prevent blocking the whole web page.

Does this PR introduce a breaking change?

  • Yes.
  • No.

@CLAassistant
Copy link

CLAassistant commented Jun 1, 2019

CLA assistant check
All committers have signed the CLA.

@stevenjoezhang
Copy link
Contributor

How about disqusJS? #705

@KaitoHH
Copy link
Contributor Author

KaitoHH commented Jun 2, 2019

Fixed. Notice that DisqusJS also has the same problem as #395. So I fixed it by adding essential url and identifier attribute.

url: {{ page.permalink | json }},
identifier: {{ page.path | json }},

@ivan-nginx ivan-nginx added this to the v7.2.0 milestone Jun 2, 2019
@1v9 1v9 changed the title Improved page rendering by deferring disqus loading Improve page rendering by deferring disqus loading Jun 4, 2019
@1v9 1v9 merged commit 72273b6 into theme-next:master Jun 4, 2019
@1v9
Copy link
Member

1v9 commented Jun 4, 2019

@allcontributorsadd please add @KaitoHH for code

@allcontributors
Copy link
Contributor

@1v9

I've put up a pull request to add @KaitoHH! 🎉

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.

5 participants