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.

Sign up for GitHub

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

Jump to bottom

Update playlist page to add remove duplicate videos button for user playlists #5191

Merged
merged 5 commits into from
Jun 12, 2024
Merged

Update playlist page to add remove duplicate videos button for user playlists #5191

merged 5 commits into from
Jun 12, 2024

Conversation

PikachuEXE
Copy link
Collaborator

@PikachuEXE PikachuEXE commented May 30, 2024 •
edited
Loading

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

#5044 (comment)

Description

  • Only on user playlists with duplicate videos (by ID)
  • Add button for removing duplicate videos

Screenshots

image
image

Testing

  • Ensure button invisible in remote playlist, edit mode, playlist with no video, no duplicate video
  • Ensure button works like remove watched videos except it targets duplicate videos (including toast message)
  • Ensure button disappear if duplicate videos removed another way

Desktop

  • OS:
  • OS Version:
  • FreeTube version:

Additional context

If you got a better icon let me know~

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label May 30, 2024
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) May 30, 2024 01:44
@absidue absidue changed the title $ Remove duplicate line in a method setting a property to the same value * Update playlist page to add remove duplicate videos button for user playlists May 30, 2024
@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

Ugh finding an appropriate icon for this sucks

efb4f5ff-1298-471a-8973-3d47447115dc
efb4f5ff-1298-471a-8973-3d47447115dc reviewed May 30, 2024
View reviewed changes
Copy link
Member

Choose a reason for hiding this comment

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

Everything LGTM but im not so sure about the icon that is being used but also couldnt find a better one that we arent already using.

Ensure button disappear if duplicate videos removed another way

Ugh we might also need to do this for the remove watched videos. No point in having it there when there arent any videos in the playlists. That would also make the UX consistent which wont confuse users

@PikachuEXE
Copy link
Collaborator Author

Updated to make remove watched videos button visible only when any video watched

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

Thanks Pika i really appreciate the effort. I'll try to find a better icon. If anyone found an icon that represents this better pls share :)

@kommunarr
Copy link
Collaborator

users-slash almost works because it has many of one thing being slashed. This is why I want #4259 to be a thing, because I don't think anything we could choose here would fully suffice in conveying the meaning well enough on its own.

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

Lets change it to that one as that is indeed the only slash kind of icon that involves multiple things

@PikachuEXE
Copy link
Collaborator Author

icon updated
image

kommunarr
kommunarr reviewed Jun 6, 2024
View reviewed changes
Copy link
Collaborator

@kommunarr kommunarr left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good!

suggestion (non-blocking): This is not strictly within the original scope, but I would suggest that the 2 prompts actually showed the number of duplicate videos / watched videos that will be removed. They currently only do so in the toasts after the deletions are complete. As it stands, it's unclear what the magnitude of the action will be.

@PikachuEXE PikachuEXE changed the title * Update playlist page to add remove duplicate videos button for user playlists Update playlist page to add remove duplicate videos button for user playlists Jun 11, 2024
@PikachuEXE
Copy link
Collaborator Author

the 2 prompts actually showed the number of duplicate videos / watched videos that will be removed

Updated
image
image

@FreeTubeBot FreeTubeBot merged commit 7688f81 into FreeTubeApp:development Jun 12, 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 Jun 12, 2024
@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc deleted the feature/playlist/remove-duplicate-button branch June 12, 2024 16:53
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Jun 13, 2024
@PikachuEXE
Merge branch 'development' into custom-builds/current
c6720d2 
* development: (60 commits)
  check for 204 error (FreeTubeApp#5259)
  Translated using Weblate (Turkish)
  ! Fix error when fetching deleted comment replies in local API (FreeTubeApp#5255)
  Optical enhancement - Improved spacing on the about page FreeTubeApp#5210 (FreeTubeApp#5257)
  Translated using Weblate (Czech)
  Translated using Weblate (Hungarian)
  Translated using Weblate (Estonian)
  Translated using Weblate (German)
  Update playlist page to add remove duplicate videos button for user playlists (FreeTubeApp#5191)
  Respect playlist sort order in `watch-video-playlist` (FreeTubeApp#5013)
  Translated using Weblate (Serbian)
  Translated using Weblate (Chinese (Simplified))
  Translated using Weblate (Chinese (Traditional))
  Translated using Weblate (Arabic)
  Translated using Weblate (Polish)
  Translated using Weblate (Italian)
  Translated using Weblate (Turkish)
  Translated using Weblate (Croatian)
  Translated using Weblate (Dutch)
  Translated using Weblate (Spanish)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kommunarr kommunarr kommunarr approved these changes

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc approved these changes

@absidue absidue absidue approved these changes

@ChunkyProgrammer ChunkyProgrammer Awaiting requested review from ChunkyProgrammer

@MarmadileManteater MarmadileManteater Awaiting requested review from MarmadileManteater

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@PikachuEXE @efb4f5ff-1298-471a-8973-3d47447115dc @kommunarr @absidue @FreeTubeBot