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

Component style not updating #34

Closed
neohed opened this issue Jul 24, 2019 · 8 comments · Fixed by #35
Closed

Component style not updating #34

neohed opened this issue Jul 24, 2019 · 8 comments · Fixed by #35
Assignees
Labels
bug Something isn't working

Comments

@neohed
Copy link

neohed commented Jul 24, 2019

Hi,
I'm using disqus-react with this gatsbyjs starter project

The problem is when I use the theme switcher the styles for disqus-react don't update without a page refresh. I tried passing the theme name into react-disqus as a prop to force an update but that didn't work:

<DiscussionEmbed theme={themeName} shortname={disqusShortname} config={disqusConfig} />

Any thoughts on how to force the styles to update?

@tterb
Copy link
Contributor

tterb commented Jul 29, 2019

@neohed I'll look into trying to reproduce this today with the gatsby starter and see if there's a viable solution or if the component can be updated to accommodate style changes.

@tterb
Copy link
Contributor

tterb commented Jul 29, 2019

@neohed I was able to reproduce this issue, and it looks like passing the theme name wasn't forcing an update because the DiscussionEmbed components shouldComponentUpdate method is only checking for changes in specific props.

I've created a PR that I think should address your issue, if you want to test it out and confirm.

@tterb tterb added the bug Something isn't working label Jul 29, 2019
@neohed
Copy link
Author

neohed commented Jul 30, 2019

@tterb I ran a test just now but still see the same issue. I'll test this more later to confirm...

@tterb
Copy link
Contributor

tterb commented Jul 30, 2019

@neohed Just to clarify, the PR #35 hasn't been merged/published yet so you won't see an changes unless you pull from that branch and use the local package in the gatsby-starter, instead of the published version from npm.
I just wanted to ensure that the changes fixed your issue before merging it and publishing a new version.

@neohed
Copy link
Author

neohed commented Aug 3, 2019

@tterb I pulled the PR branch into a local repo but didn't know how to import that into my gatsby site. I tried import from the filepath but got compile errors - it seemed like webpack was trying to resolve gatsby dependencies in disqus-react...?

What's the best way to test this?

@tterb
Copy link
Contributor

tterb commented Aug 3, 2019

@neohed The easiest way to test the PR locally would be to pull the branch into a local disqus-react directory that is in the same parent directory as your gatsby-starter-egghead-blog/ repo. Then you can cd into your gatsby starter and create a symlink to the local package with the command:

yarn link ../disqus-react

@neohed
Copy link
Author

neohed commented Aug 4, 2019

@tterb cool symlink worked great - I learned something new! And your fix does work. It updates correctly. What I'm seeing here now seems to be a different bug with my gatsby site where the themeName inside post.js is always "default" even when using "dark" theme... But that's unrelated to disqus-react.

I can confirm your fix works. Thanks.

@tterb
Copy link
Contributor

tterb commented Aug 5, 2019

@neohed I'm glad we were able to your fix your issue and I appreciate your responsiveness in providing feedback to help improve the package 👍

I'll merge #35 tomorrow and get a patch published to npm.

@tterb tterb closed this as completed in #35 Aug 9, 2019
tterb added a commit that referenced this issue Aug 9, 2019
Fix #34: Modify shouldComponentUpdate condition
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants