FreeTube icon indicating copy to clipboard operation
FreeTube copied to clipboard

Add feature to set a playlist as quick bookmark target

Open PikachuEXE opened this issue 1 year ago • 9 comments

Pull Request Type

  • [ ] Bugfix
  • [x] Feature Implementation
  • [ ] Documentation
  • [ ] Other

Related issue

Ability to add video to a playlist via a single click removed in multiple playlist PR 4234

Description

Add back ability to add video to a playlist set by user Even though it's possible to add multiple entries to a playlist, this feature (the quick bookmark button) can only add one entry to target playlist and remove it (behaves like the old bookmark button)

Feature can be disabled, target playlist can be changed at anytime (the bookmarked indicator would change when target playlist changed)

Screenshots

When it's disabled video list look like as if this feature not added so screenshot not included

image image image

Testing

  • Test target playlist can be set/updated to another/unset (i.e. disable the feature)
  • Test videos can be bookmarked/unbookmarked & indicator is display correctly in places
  • Test bookmark indicator is updated when target playlist updated/unset

Desktop

  • OS:
  • OS Version:
  • FreeTube version:

Additional context

UX/text/icons can be changed here or later

PikachuEXE avatar Jan 04 '24 00:01 PikachuEXE

@PikachuEXE did you see #4517?

PR differences:

  • horizontal icon alignment here vs vertical on other PR
  • slightly larger gap between icons in other PR
  • remove once here vs remove all dupes on other PR (not too relevant), and as a result, different ways we fix deleteVideoIdByPlaylistId
  • Watch Later kept here vs removed on other PR
  • Favorites playlist is unprotected here vs protected on other PR

Things my PR is missing:

  • Custom bookmarked playlist ability
  • Custom bookmarked playlist button
  • removeVideo fix

Things this PR is missing:

  • Favorites icon in watch-video-info section
  • Minor visual improvement to focus behavior
  • "Unsave" text when video is already favorited
  • Default bookmarked playlist
  • removeVideos fix

Not going to ego it out, can handle this however you prefer @PikachuEXE, as long as all "missing" items are addressed.

kommunarr avatar Jan 04 '24 01:01 kommunarr

Addressed

  • Favorites icon in watch-video-info section
  • Minor visual improvement to focus behavior
  • "Unsave" text when video is already favorited (used Remove from instead of Unsave from coz that sounds strange
  • Default bookmarked playlist
  • removeVideos fix

PikachuEXE avatar Jan 04 '24 02:01 PikachuEXE

Even though it's possible to add multiple entries to a playlist, this feature (the quick bookmark button) can only add one entry to target playlist and remove it (behaves like the old bookmark button)

I don't mind it, but just so you know, un-starring a video removes all of the dupes as well in this PR, same as mine.

kommunarr avatar Jan 04 '24 02:01 kommunarr

Looks like the deleteVideoIdsByPlaylistId function I fixed was not one you used in this PR. Please add the fix here as well, just for the future.

kommunarr avatar Jan 04 '24 02:01 kommunarr

deleteVideoIdsByPlaylistId fix added quickBookmarkIconText updated unused copyToClipboard removed Replace current quick bookmark target with undo done, different toast text though Playlist page for current quick bookmark target updated to hide star button

PikachuEXE avatar Jan 04 '24 07:01 PikachuEXE

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

github-actions[bot] avatar Jan 15 '24 04:01 github-actions[bot]

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

github-actions[bot] avatar Jan 15 '24 05:01 github-actions[bot]

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

PikachuEXE avatar Jan 17 '24 00:01 PikachuEXE

I dont see the color for the star in the code but in the past there was an issue about the star color not being bright enough. If it is not too much hassle can you maybe make change to the color of the star icon to the brightest color yellow, ffff00.

Also should we maybe disable this? Users can now set a remote playlist as quick bookmark.

https://github.com/FreeTubeApp/FreeTube/assets/73130443/01f57f67-6488-44ee-9d62-0a4ad728d6c4

I missed that entirely @efb4f5ff-1298-471a-8973-3d47447115dc - good catch. We need a infoSource === 'user' && appended to the check.

kommunarr avatar Jan 22 '24 23:01 kommunarr

@efb4f5ff-1298-471a-8973-3d47447115dc I have no idea which themes should be updated (for not bright enough)... image

Is there an existing issue/discussion about it?

PikachuEXE avatar Jan 23 '24 00:01 PikachuEXE

@efb4f5ff-1298-471a-8973-3d47447115dc I have no idea which themes should be updated (for not bright enough)... image

Is there an existing issue/discussion about it?

Issue was closed in the past by the playlist PR but probably should be reopened https://github.com/FreeTubeApp/FreeTube/issues/1762