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

Comments: Show @-mentions in line with reply #1137

Merged
merged 4 commits into from
Jan 10, 2025
Merged

Conversation

obenland
Copy link
Member

@obenland obenland commented Jan 8, 2025

Fixes #1135.

@pfefferle Any reason the mention handling shouldn't be part of get_content()? Is there a use case for it that requires it to not contain Fediverse mentions?

Proposed changes:

  • Prepends mentions to comment content without wrapping paragraph tags.
  • Moves @-mention handling into the comment content getter to avoid grepping HTML tags to insert them in the right place.
  • Updates unit tests to test for the new expected output.

Other information:

  • Have you written new tests for your changes, if applicable?

Testing instructions:

  • Publish a post.
  • Have another comment reply to the post on the Fediverse.
  • On your site, reply to the comment via Federation.
  • View the reply in the Fediverse client and make sure the reply is inline with the mentions.

@obenland obenland requested review from jeherve and a team January 8, 2025 15:36
@obenland obenland self-assigned this Jan 8, 2025
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

I'll say that this tests well on my site, but I'll let @pfefferle weigh in for your question. :)

Copy link
Member

@pfefferle pfefferle left a comment

Choose a reason for hiding this comment

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

@obenland thanks a lot, this should work totally fine! I have simplified the code a bit more and changed the HTML to unify with the one from the mention class.

@obenland obenland merged commit 370d8c8 into trunk Jan 10, 2025
24 checks passed
@obenland obenland deleted the fix/mentions-linebreak branch January 10, 2025 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Comment replies via Federation: @mention in Federated reply is wrapped in a paragraph
3 participants