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

Use addVideo instead of addVideos for quick bookmark button #5168

Merged

Conversation

absidue
Copy link
Member

@absidue absidue commented May 24, 2024

Use addVideo instead of addVideos for quick bookmark button

Pull Request Type

  • Performance improvement

Description

As we already have a dedicated process for adding just one video to the playlist, we might as well use it for the quick bookmark button, as that always just adds one video. Another advantage is that the addVideo store action, doesn't need to clone the video object, because we know it's not used anywhere else as we just created it.

I'm not expecting this improvement to be drastic enough to be noticeable without measuring tools, but it's still worth it as we are doing less work and using less memory.

Testing

Check that adding a video to the quick bookmark playlist through the dedicated button on a video thumbnail and on the watch page, still works.

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: f7cadd1

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label May 24, 2024
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) May 24, 2024 17:38
@efb4f5ff-1298-471a-8973-3d47447115dc

as that always just adds one video

Umm just curious what happens with this if the Batch actions PR is merged

@absidue
Copy link
Member Author

absidue commented May 24, 2024

as that always just adds one video

Umm just curious what happens with this if the Batch actions PR is merged

I haven't looked at that pull request, however if it just fakes a click on the quick bookmark button then it won't make a difference, but if that is the case then that pr should be redesigned, to call addVideos once with all videos, instead of for every video alone.

@FreeTubeBot FreeTubeBot merged commit 2e43b09 into FreeTubeApp:development May 25, 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 May 25, 2024
@absidue absidue deleted the quick-bookmark-add-video branch May 25, 2024 10:40
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request May 26, 2024
* development:
  Fix channel sort values to show the values they are (FreeTubeApp#5162)
  Translated using Weblate (Russian)
  Translated using Weblate (French)
  Quick bookmark button RTL & hover fixes (FreeTubeApp#5157)
  Use addVideo instead of addVideos for quick bookmark button (FreeTubeApp#5168)
  Cache quick bookmark playlist to reduce the amount of lookups (FreeTubeApp#5169)
  Translated using Weblate (Arabic)
  Translated using Weblate (Indonesian)
  Fix hide/show channel in ft-list-video (FreeTubeApp#5149)

# Conflicts:
#	src/renderer/components/ft-list-video/ft-list-video.js
#	src/renderer/components/watch-video-info/watch-video-info.js
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