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

Order watched recommended videos last #4394

Merged

Conversation

kommunarr
Copy link
Collaborator

Order watched recommended videos last

Pull Request Type

  • Feature Implementation

Related issue

closes #397

Description

Orders watched videos last in the recommended videos list. Only sets ordering once when route is first loaded, not live during any "Mark as Watched" / "Mark as Unwatched" events (because this would be undesirable). This should functionally make infinite autoplay loops impossible.

Testing

  • Enable autoplay and mark videos at the top of the rec list as watched
  • Reload route and see that those watched videos are at the bottom of the rec list
  • Let video autoplay and see that the video visibly at the top of the rec list is played

Desktop

  • OS: OpenSUSE Tumbleweed
  • OS Version: 2023xxxx
  • FreeTube version: 0.19.1

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 27, 2023
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 27, 2023 02:50
PikachuEXE
PikachuEXE previously approved these changes Nov 27, 2023
@efb4f5ff-1298-471a-8973-3d47447115dc

not live during any "Mark as Watched" / "Mark as Unwatched" events (because this would be undesirable)

Why would this be undesirable? Wouldnt it be a good thing that i can mark something as watched and it will not autoplay as the next video?

@kommunarr
Copy link
Collaborator Author

I was more referring to the physical ordering aspect, which would be jarring to have it jump out of view if you marked it as watched. Detaching the visible display order of the videos from how they would be autoplayed seems unintuitive. I think there's an interesting use case for manipulating the autoplay order on the go, but that would fall in line with either a Queue feature (#547) and/or hiding videos (#1362).

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

To me it feels a bit like unexpexted behavior when i mark the first video in the list as watched and still get it when the autoplay kicks in. I feeling could be wrong though.

@kommunarr
Copy link
Collaborator Author

To me it feels a bit like unexpexted behavior when i mark the first video in the list as watched and still get it when the autoplay kicks in

I think that would make sense if it was a labeled feature like "Keep watched videos last," but as it stands, I think reordering the recommended videos list based on Mark as Watched is more jarring & not in line with user expectations of how things would work. Open to hearing other perspectives on this though!

@PikachuEXE
Copy link
Collaborator

Right now I would say it's unnecessary to move entries marked as watched to the bottom
If they want to play non 1st item, just click on it

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

@PikachuEXE just so we are one the same page, Im not talking about it moving visually just moving it behind the scenes so the user doesnt get to see that vid

@PikachuEXE
Copy link
Collaborator

So more like on video end
Pick 1st recommended not marked as watched video to autoplay? (fallback to 1st video if all watched)

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

Yup exactly like that. If for some reason i see a video on top of the recommended videos list but i already seen it. The user marks it as watched. When its time for the autoplay to go to the next video it should skip that one on the top i just marked as watched and pick the 2nd one in line

@kommunarr
Copy link
Collaborator Author

kommunarr commented Nov 29, 2023

I don't think that's intuitive behavior unless you're expecting that to happen & behave like that already, which most people wouldn't be. You would want & expect that behavior to happen for something like video-hiding or a watch queue. This current feature is more of a subtle convenience feature for regulating how we order recommended videos for preventing circular autoplay loops.

@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

efb4f5ff-1298-471a-8973-3d47447115dc commented Nov 29, 2023

Good point you made!

@absidue
Copy link
Member

absidue commented Dec 1, 2023

Could you please use a local/temporary variable and only assign the final value to this, instead of assigning and then overwriting the value in this? (should be slightly more efficient as it will only trigger the watcher once, although hopefully Vue is clever enough to detect those back to back changes and only update the watch-video-recommendations component once).

@FreeTubeBot FreeTubeBot merged commit fc210aa into FreeTubeApp:development Dec 2, 2023
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 Dec 2, 2023
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Dec 5, 2023
* development:
  Translated using Weblate (Serbian)
  Translated using Weblate (Croatian)
  Translated using Weblate (Croatian)
  Bump electron from 27.1.2 to 27.1.3 (FreeTubeApp#4420)
  Bump the stylelint group with 1 update (FreeTubeApp#4416)
  Bump the fortawesome group with 3 updates (FreeTubeApp#4417)
  Bump marked from 10.0.0 to 11.0.0 (FreeTubeApp#4419)
  Bump the babel group with 2 updates (FreeTubeApp#4414)
  Bump the eslint group with 3 updates (FreeTubeApp#4415)
  Bump lefthook from 1.5.4 to 1.5.5 (FreeTubeApp#4418)
  Translated using Weblate (Greek)
  Order watched recommended videos last (FreeTubeApp#4394)
  IV: check for 404 and 500 for subscriptions (FreeTubeApp#4410)
  Translated using Weblate (Hungarian)
  Add homebrew to README.md (FreeTubeApp#4274)
  Translated using Weblate (Polish)
  Translated using Weblate (Icelandic)
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.

Enhancement : Skip Play Next Video when already in History
5 participants