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

comment url & custom emoji on invidious API #3658

Merged
merged 2 commits into from
Jun 14, 2023

Conversation

lamemakes
Copy link
Contributor

@lamemakes lamemakes commented Jun 12, 2023

Comment URL & custom emoji on invidious API

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

#3642

Description

This PR mainly fixes the issue of custom emojis not showing up when using the invidious API, but also slightly touches the upstream issue of improper YouTube links in channels. An Invidious PR was just put up to fully mediate this issue though.

Screenshots

Comment before fix (note messed up channel URL and no custom emoji):
Screenshot from 2023-06-12 10-19-15

Comment after FreeTube fix, but without upstream fixes (current state, note emoji fix):
Screenshot from 2023-06-12 10-20-18

Comment with both FreeTube and upstream fix:
Screenshot from 2023-06-12 10-23-25

Comment with multiple custom emojis
Screenshot from 2023-06-12 12-33-31

Testing

This was tested by confirming the comment now looks as inspected utilizing both a remote invidious instance and a locally "fixed" instance of invidious which shows proper channel URL/linking.

Video to test with: https://youtu.be/LHRw3sDQH1w
Pinned comment has multiple custom emojis: https://youtu.be/v3wm83zoSSY

Desktop

Copy link
Member

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @lamemakes for your contribution. I'll hope we will see more of your contributions in the future!

Edit: inserted testing video in PR body

2nd Edit: Removed closes from the PR body because this doesn't solve the original reported issue. I noticed this when writing the issue and decided to put it in the issue as something to note.

@lamemakes
Copy link
Contributor Author

@efb4f5ff-1298-471a-8973-3d47447115dc You certainly will - I'm super appreciative for all the work you guys do to maintain this wonderful application!

@absidue
Copy link
Member

absidue commented Jun 12, 2023

Hiii
Thanks for your pull request, unfortunately when there are multiple custom emojis in the comment, only the first one works. I added a good test video to the pull request description, which I took from a previous pull request which touched the custom emojis.

Thank you again for opening this pull request ^^.

Screenshot of the issue:
missing_images

auto-merge was automatically disabled June 12, 2023 16:34

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) June 12, 2023 16:34
@lamemakes
Copy link
Contributor Author

Gahhh great catch. The replace vs replaceAll always gets me. Looks to be functional now, updated the body of the PR with screenshots from that video.

Copy link
Member

Choose a reason for hiding this comment

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

Ah good catch. Really LGTM now!

@FreeTubeBot FreeTubeBot merged commit 8281df7 into FreeTubeApp:development Jun 14, 2023
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Jun 14, 2023
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.

6 participants