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

Improve social media preview #3163

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

svenseeberg
Copy link
Member

@svenseeberg svenseeberg commented Oct 31, 2024

Short description

Improve the social media previews.

More info: https://chat.tuerantuer.org/digitalfabrik/pl/rnoez75xxjyhugxo97mbjuqmte & https://tasks.tuerantuer.org/projects/infra/work_packages/3734/activity?query_id=59

Previews on Signal and WhatsApp are not yet working.

Proposed changes

  • Improve edge case handling in excerpts.

Side effects

  • N/A

Pull Request Review Guidelines

@svenseeberg svenseeberg force-pushed the feature/improve-social-media-previews branch from 35489de to 83348ce Compare October 31, 2024 15:18
strip_tags(content.replace("\n", " ").replace("\r", "").replace("<br>", " "))
)
if len(stripped_content) <= 100:
return stripped_content.strip().replace(" ", " ")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this line? :)

Copy link
Member Author

@svenseeberg svenseeberg Oct 31, 2024

Choose a reason for hiding this comment

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

For example

Hallo<br>
Welt

will evaluate to Hallo Welt as we replaced newlines and br tags with empty spaces in line 51.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't re.sub(r"\s+", " ", stripped_content.strip()) work better in this case? Who says we only have two spaces?

@JoeyStk JoeyStk added the help wanted Extra attention is needed label Dec 18, 2024
@JoeyStk JoeyStk added the blocks-app Issues that block the frontend label Jan 8, 2025
@PeterNerlich
Copy link
Collaborator

PeterNerlich commented Jan 22, 2025

Just yesterday I came across a post about implementing preview cards for forgejo. Maybe it helps to cross-compare this: https://codeberg.org/forgejo/forgejo/pulls/6053/files#diff-f9ae2dae24595a0ce188650ca882e187de8cc998

Edit: No, doesn't look like they implement anything but the og:image yet

)
if len(stripped_content) <= 100:
return stripped_content.strip().replace(" ", " ")
return stripped_content[:100].strip().rsplit(' ', 1)[0]+" ..."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return stripped_content[:100].strip().rsplit(' ', 1)[0]+" ..."
return stripped_content[:100].strip().rsplit(' ', 1)[0]+" "

@PeterNerlich
Copy link
Collaborator

PeterNerlich commented Jan 22, 2025

Looking at https://ogp.me/, we might benefit from adding/improving

  • og:type (seems to already be defined elsewhere…? investigate where)
  • og:url should be a fixed (= unchanging) url of the content and functions as an ID of the node on the graph.
    Investigate whether url is a permalink or contains the slug that might change for a page.
  • og:locale Language of the content (e.g. en_US)
  • og:locale:alternate Alternate locales the content is available in (repeat this meta tag for each alternative)
  • og:site_name Name of the overall website the content is part of. I guess this would just be Integreat and should probably not be part of og:title if defined here…?
  • og:image:secure_url Described as "An alternate url to use if the webpage requires HTTPS" – Might be out of date, but won't hurt to add it. Will be the same value as og:image
  • og:image:alt Alt text for the image (not a caption). I could imagine this to be the problem on WhatsApp and/or signal, as ogp.me says "If the page specifies an og:image it should specify og:image:alt."
  • og:image:type Mime type for the image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocks-app Issues that block the frontend help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants