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

Switch comments to calculatePublishedDate and getRelativeTimeFromDate #6086

Merged

Conversation

absidue
Copy link
Member

@absidue absidue commented Nov 3, 2024

Switch comments to calculatePublishedDate and getRelativeTimeFromDate

Pull Request Type

  • Refactor

Description

Published dates for videos and community posts were already using the calculatePublishedDate and getRelativeTimeFromDate functions, comments however were still using the old toLocalePublicationString function. This pull request switches the comments over to the newer functions too, which means that the x unit ago published date display should be the same again across the app. As the old function is now unused we can remove it as well as the translation strings that it was using (it also removes the other unused strings under the Video.Published.* key), because the newer functions use the built-in Intl.RelativeTimeFormat class.

@ChunkyProgrammer You'll need to make the same adjustment in your Piped pull request as that currently uses the function that this pull reqeust removes.

This pull request also fixes the storyboard issue that was introduced in the jsdoc pull request so that this pull request can actually be tested.

Testing

Check that the published dates show up on comments with both API backends

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 94fe62a

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 3, 2024 17:37
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 3, 2024
PikachuEXE
PikachuEXE previously approved these changes Nov 4, 2024
ChunkyProgrammer added a commit to ChunkyProgrammer/FreeTube that referenced this pull request Nov 4, 2024
Copy link
Contributor

github-actions bot commented Nov 6, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added PR: merge conflicts / rebase needed and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Nov 6, 2024
@efb4f5ff-1298-471a-8973-3d47447115dc

oh didnt saw that pr had to be rebased :(

@PikachuEXE
Copy link
Collaborator

@ChunkyProgrammer Now you understand why I opened
#6097 just in case...

@absidue
Copy link
Member Author

absidue commented Nov 7, 2024

Just in case what? Just because I didn't instantly solve the conflicts and can't work on FreeTube 24/7 doesn't mean I'm abandoning my pull requests. I also spent a bunch of time that I could have worked on FreeTube investigating the slow webpack build times and eslint linting times, in an attempt to improve our development experience, but I guess it doesn't count unless I am working directly on FreeTube...

Copy link
Contributor

github-actions bot commented Nov 7, 2024

Conflicts have been resolved. A maintainer will review the pull request shortly.

@absidue absidue added the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 7, 2024
Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Just in case PRs got merge conflicts and making fix delivered slower

oh didnt saw that pr had to be rebased :(

@FreeTubeBot FreeTubeBot merged commit b51d97c into FreeTubeApp:development Nov 8, 2024
5 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 8, 2024
@absidue absidue deleted the comments-published-dates branch November 8, 2024 06:33
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.

5 participants