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

Fixed url prop bug for <ReactDisqusComments /> #95

Merged
merged 1 commit into from
Mar 3, 2019

Conversation

vzhou842
Copy link
Collaborator

@vzhou842 vzhou842 commented Feb 7, 2019

The url prop being passed into ReactDisqusComments is wrong - both siteUrl and postSlug are undefined:

  • siteUrl should actually be named url, see the graphql in post-template
  • postSlug is not included in the graphql for post-template and is thus undefined

This prop fixes those two values so now the URL is correct. I tested this on my own site (which is based on this starter project) and it works great!

@codecov
Copy link

codecov bot commented Feb 7, 2019

Codecov Report

Merging #95 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #95   +/-   ##
=======================================
  Coverage   95.71%   95.71%           
=======================================
  Files          28       28           
  Lines         140      140           
  Branches        8        8           
=======================================
  Hits          134      134           
  Misses          6        6
Impacted Files Coverage Δ
src/templates/post-template.js 100% <ø> (ø) ⬆️
src/components/Post/Comments/Comments.js 75% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d31e82a...4b3eeae. Read the comment docs.

@vzhou842
Copy link
Collaborator Author

@alxshelepenok just wanted to ping and see if you've seen this? I feel like it's a pretty important PR to merge given that without it Disqus doesn't work in this template. I'm already using this fix live on my blog and it works great for me.

@alxshelepenok
Copy link
Owner

@vzhou842 yes i saw it
#86 also includes such changes. But of course you don't need to close.

@vzhou842
Copy link
Collaborator Author

ok gotcha

@alxshelepenok alxshelepenok merged commit 2933b1f into alxshelepenok:master Mar 3, 2019
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

2 participants