lukeorth / poison

Professional Hugo theme for dev bloggers. Based on Mdo's classic Hyde theme.
https://poison.lukeorth.com
GNU General Public License v3.0
199 stars 98 forks source link

issue with og:image logic for posts #133

Closed rawwerks closed 10 months ago

rawwerks commented 11 months ago

even when i explicitly set an image in the post front matter (ie image: "/images/postimage.jpg"), the logic in the og:image portion of meta.html doesn't seem to be operating correctly.

_note: i'm assuming that the desired functionality is for the post's Params.image to be the og:image if there is one. if not, then for it to fall back to the site's og_image, then to the brand_image._

$og_image is not returning true even when the image is defined in front matter:

{{ $og_image := .Resources.Get .Params.image }}

so then it's going into the next section of the logic loop, where you can see that Site.Params.og_image gets precedence over .Params.image - which doesn't seem to be right to me...because every post will have the site's og:image (which is how I noticed the problem when sharing a post on social media.)


{{ if not ( $og_image ) }}
  {{ $og_image_path := "" }}
  {{ if .Site.Params.og_image }}
    {{ $og_image_path = .Site.Params.og_image }}
  {{ else if .Params.image }}
    {{ $og_image_path = .Params.image }}
  {{ else }}
    {{ $og_image_path = .Site.Params.brand_image }}
  {{ end }}
  {{ if $og_image_path }}
    {{ $og_image = resources.Get $og_image_path }}
  {{ end }}
{{ end }}

the quickest fix for my site was to simply invert the order in the chain of ifstatements:

{{ if .Params.image }} 
    {{ $og_image_path = .Params.image }} 
  {{ else if .Site.Params.og_image  }}
    {{ $og_image_path = .Site.Params.og_image }}
  {{ else }}
    {{ $og_image_path = .Site.Params.brand_image }}
  {{ end }}

but in that case, you mind as well get rid of {{ $og_image := .Resources.Get .Params.image }} and the first {{ if not ( $og_image ) }} because it's never going down that path of logic.

i'll leave it to @lukeorth to decide the cleanest way to fix this.

rawwerks commented 11 months ago

(as a side note, my fix above only works if the post's Params.image file is duplicated in both assets and static, see https://github.com/lukeorth/poison/issues/131 for that issue)

rawwerks commented 11 months ago

@lukeorth - this code from another Hugo theme I use might help you with this issue, as well as #131 :


{{ "<!-- Open Graph image and Twitter Card metadata -->" | safeHTML }}
{{ $image_path := .Params.image | default .Site.Params.og_image -}}
{{ $image_path_local :=  printf "assets/%s" $image_path -}}
{{ $image_ext := trim (path.Ext $image_path | lower) "." -}}
{{ if fileExists $image_path_local -}}
{{ $image_path:= resources.Get $image_path}}
  <meta property="og:image" content="{{ $image_path.RelPermalink }}" />
  {{/* If not SVG, read image aspect ratio and define Twitter Card and Open Graph width and height  */ -}}
  {{ if ne $image_ext "svg" -}}
    {{ with (imageConfig $image_path_local) -}}
    {{ if (and (gt .Width 144) (gt .Height 144)) -}}
      <meta name="twitter:image" content="{{ $image_path.RelPermalink }}"/>
      <meta name="twitter:card" content="summary{{ if (and (gt .Width 300) (gt .Height 157) (not (eq .Width .Height))) }}_large_image{{ end }}">
    {{ end -}}
    <meta property="og:image:width" content="{{ .Width }}">
    <meta property="og:image:height" content="{{ .Height }}">
    {{ end -}}
  {{ end -}}
  <meta property="og:image:type" content="image/{{ if eq $image_ext `svg` }}svg+xml{{ else }}{{ replaceRE `^jpg$` `jpeg` $image_ext }}{{ end }}">
{{ end -}}

i tried swapping it into the theme, with working results for setting og:image . i'll let you decide how to implement within your theme.

lukeorth commented 11 months ago

Thank you, @rawwerks. I appreciate you looking so deep into this. I just created a PR that includes the code snippet you provided and removes some of the extra JS.

Would you mind taking a look and letting me know if this solves your issue before I merge?

Thanks again!

mazdakn commented 11 months ago

I am hitting the same issue. Thanks for fixing it.

lukeorth commented 10 months ago

Hi @mazdakn, would you mind checking out this pull request for me and let me know if it fixes your issue? If so, I'll go ahead and get it merged.

mazdakn commented 10 months ago

@lukeorth sorry for late reply. I am going to try it this weekend.