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

Fix twitter card image path #609

Merged
merged 4 commits into from
Sep 23, 2020

Conversation

johnkuney
Copy link
Contributor

I realized twitter card image needs an absolute path to work. So used the static folder and referenced the url directly.

  • moved twitter image to static folder
  • added bitcoincash.org url + file path to twitter image so it will work when live on bitcoincash.org

Copy link
Contributor

@deadalnix deadalnix left a comment

Choose a reason for hiding this comment

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

Why dos it need an absolute path to work?

@@ -73,7 +72,7 @@ function SEO({ description, lang, meta, title, twitter_image }) {
},
{
name: `twitter:image`,
content: metatwitterImage,
content: "https://www.bitcoincash.org/images/" + metatwitterImage,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not bake the domain in the code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For sure. What about if we define it in the gatsby config meta data and bring it in like that?

Copy link
Contributor

Choose a reason for hiding this comment

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

OK so it seems like there is no good way to do this. Then I guess we can go with this.

Can you set the images/ as part of metatwitterImage rather than prebaked?

@johnkuney
Copy link
Contributor Author

You'd have to ask twitter, but its stated in this thread https://twittercommunity.com/t/card-error-unable-to-render-or-no-image-read-this-first/62736 third from last bullet

Copy link
Contributor

@deadalnix deadalnix left a comment

Choose a reason for hiding this comment

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

So, I've loaded the site from the branch in your repo. I can't see anything different in either chrome or firefox.

It remains unclear what problem this is solving if any, and, in any case, the solution needs to adjust to the source from which the content is served.

@@ -2,6 +2,7 @@ module.exports = {
siteMetadata: {
title: "Bitcoincash.org",
author: "Bitcoincash.org",
siteURL: "https://www.bitcoincash.org/",
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see how putting the absolute url somewhere else solves anything.

@johnkuney
Copy link
Contributor Author

So this problem is as of now this twitter card image is not working with twitter. You can test it out here https://cards-dev.twitter.com/validator

According to their docs the image needs an absolute path to work

So need to define the absolute path. I agree having it adjust to the serving source is ideal. Will see if I can make that happen

@johnkuney
Copy link
Contributor Author

hmm may be stuck with it. Seems gatsbys own docs recomend defining the domain since gatsby cant really see it
Screen Shot 2020-09-22 at 4 04 54 PM

@deadalnix
Copy link
Contributor

Alright, but then it mean that passing the twitter image as argument is not going to work, so we got to remove that option.

Copy link
Contributor

@deadalnix deadalnix left a comment

Choose a reason for hiding this comment

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

One small change and this can be merged.

@@ -73,7 +72,7 @@ function SEO({ description, lang, meta, title, twitter_image }) {
},
{
name: `twitter:image`,
content: metatwitterImage,
content: "https://www.bitcoincash.org/images/" + metatwitterImage,
Copy link
Contributor

Choose a reason for hiding this comment

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

OK so it seems like there is no good way to do this. Then I guess we can go with this.

Can you set the images/ as part of metatwitterImage rather than prebaked?

@johnkuney
Copy link
Contributor Author

Okay made that edit. Was that other comment still relevant? It looks to me we can still use the argument. Will just need to pass a string of the image path wanted. Seems to work in testing

@deadalnix deadalnix merged commit e47d933 into bitcoincashorg:master Sep 23, 2020
EyeOfPython pushed a commit to EyeOfPython/bitcoincash.org that referenced this pull request Nov 17, 2020
* add image to static, redefine path

* absolute path fix

* put url back in seo

* move images/ to metatwitterImage
@shadi19838410 shadi19838410 linked an issue Mar 23, 2023 that may be closed by this pull request
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.

2 participants