Skip to content

Fix missing fblike button in blog#821

Merged
yangshun merged 5 commits intofacebook:masterfrom
endiliey:fblike
Jul 1, 2018
Merged

Fix missing fblike button in blog#821
yangshun merged 5 commits intofacebook:masterfrom
endiliey:fblike

Conversation

@endiliey
Copy link
Contributor

@endiliey endiliey commented Jul 1, 2018

Motivation

Fix #814

It should be

className="fb-like"

According to https://developers.facebook.com/docs/plugins/like-button/

Instead of

className="fbLike"

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

asd

Related PRs

#757 renamed "fb-like" to "fbLike"

@endiliey endiliey requested a review from JoelMarcey July 1, 2018 05:00
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jul 1, 2018
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Jul 1, 2018

Deploy preview for docusaurus-preview ready!

Built with commit 68c0fa6

https://deploy-preview-821--docusaurus-preview.netlify.com

Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

Thanks for fixing something I broke 😅

}

.fbLike {
.fb-like {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment here that we shouldn't change this class because it's determined by FB SDK?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it should be in the js ? the css here just add extra style

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes but I renamed it thinking it was our own styling 😥

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah sure, let's write it in the JS then.

<div className="blogSocialSectionItem">
<div
className="fb-comments"
className="fb-comments" // Facebook SDK require it to be 'fb-comments'
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make sure this doesn't show up in the output. I don't remember being able to write comments in JSX like that. I had to do

{ /* Facebook SDK require it to be 'fb-comments */ }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah that's true. I almost forgot 😢

}

.fbLike {
.fb-like {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah sure, let's write it in the JS then.

@yangshun yangshun merged commit 60852a2 into facebook:master Jul 1, 2018
@JoelMarcey
Copy link
Contributor

@endiliey @yangshun Thanks a lot for working to fix this. I am glad it is back 👍 I guess we will just have to live with the fact that we lost all of our likes for the two Docusaurus blog posts? 😢

@yangshun
Copy link
Contributor

yangshun commented Jul 2, 2018

I don't think the likes will be lost. Likes don't work that way 🤔

If I'm not wrong, I think the like button paths were not correctly configured in the past, and they were not post-specific. The buttons on all the posts were pointing to the same path.

@JoelMarcey
Copy link
Contributor

I think the like buttons were pointing to a Dev version maybe. Maybe I am the only one in the world who saw hundreds of likes on the announcement blog post 🤣

Sent with GitHawk

@yangshun
Copy link
Contributor

yangshun commented Jul 2, 2018

Nope, I saw it too (940+ likes). Actually, I just saw it 5 mins ago while fiddling with the config trying to find the right config for it. But I changed something and am unable to get back that state. Good news is it's definitely there, bad news is I need more time to find it back 😞

@yangshun
Copy link
Contributor

yangshun commented Jul 2, 2018

I got it. It had to be non-HTTPS URL and using the .html version.

HTTPS has it

screen shot 2018-07-01 at 10 29 38 pm

HTTP doesn't have it

screen shot 2018-07-01 at 10 31 47 pm

Play around with the URL http://docusaurus.io/blog/2017/12/14/introducing-docusaurus.html on https://developers.facebook.com/docs/plugins/like-button/

Now that we have changed to clean URLs we probably can't get back to that state too 😥

cc @endiliey

@endiliey
Copy link
Contributor Author

endiliey commented Jul 2, 2018

We can point to the html version if we want even in clean url setting.
But if its only for http then we won't be able to get it back

@yangshun
Copy link
Contributor

yangshun commented Jul 2, 2018

We can point to the html version if we want even in clean url setting.

It'll affect end users unless we add another config? But then again, not too keen on adding more configs.

But if its only for http then we won't be able to get it back

Weird though, not sure how the HTTP version got so many likes when we were on HTTPS the whole time.

@endiliey
Copy link
Contributor Author

endiliey commented Jul 2, 2018

It'll affect end users unless we add another config? But then again, not too keen on adding more configs.

Hmm lets leave it then.

Weird though, not sure how the HTTP version got so many likes when we were on HTTPS the whole time.

I think when they first launched it's still on http, just like when jest first changed to jestjs.io its still http

Or .. it points to the wrong one before

@endiliey endiliey deleted the fblike branch July 4, 2018 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Signed Facebook CLA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants