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

Hide playlists in channel search results if "hide channel playlist" preference selected. #4454

Conversation

elshimone
Copy link
Contributor

@elshimone elshimone commented Dec 13, 2023

Hide playlists in channel search results if "hide channel playlist" preference selected.

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

closes #4453

Description

This PR hides playlists in channel search results if the "hide channel playlists" preference is selected.

Screenshots

Before:
playlist

After:
image

Testing

I tested it by searching for "playlist" in the channel page, and observing playlist search results when the "hide channel preference" was not selected. I think selected this preference and performed the same search. No playlist results were displayed.

Desktop

  • OS: PopOS
  • OS Version: 22.04
  • FreeTube version: 0.19.1

Additional context

This project is great! It makes a big difference to my son.

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 13, 2023
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) December 13, 2023 22:20
auto-merge was automatically disabled December 13, 2023 23:12

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) December 13, 2023 23:12
auto-merge was automatically disabled December 13, 2023 23:24

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) December 13, 2023 23:25
Copy link
Collaborator

@kommunarr kommunarr left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

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

Choose a reason for hiding this comment

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

Im not against the implementation of this PR but i think its getting approved a bit fast.

If hiding this on the channel page needs to be the default when hiding a tab, it should also be done for other tabs not just this one. Otherwise we'll be creating an inconsistent UX by implementing it like this. Would be bad to confuse users like this

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Dec 14, 2023
@kommunarr
Copy link
Collaborator

kommunarr commented Dec 14, 2023

Channel Podcasts are the only other hide-able items that appear in channel search results. From my testing, podcasts are glorified playlists, so enabling Hide Channel Playlists now also blocks podcasts in search results, which makes sense. Correct me if I'm wrong, but I don't think we have the ability to differentiate Channel Podcasts by their data payload. If there is no way to differentiate based on their data payload, we would have to make a request (well, multiple requests until end of continuation is hit...) to load all channel podcasts to hide only them if Hide Channel Podcasts is enabled.

@elshimone
Copy link
Contributor Author

Please advise what changes I should make to the PR to get this approved. The only other thing I can think of is adding another preference to filter the channel search results. I'm not sure that is less confusing though.

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

This comment was marked as resolved.

Copy link
Member

Choose a reason for hiding this comment

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

After internal discussing i think the requested changes arent needed to be addressed.

LGTM

@FreeTubeBot FreeTubeBot merged commit ca94bce into FreeTubeApp:development Dec 16, 2023
6 checks passed
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Dec 16, 2023
* development:
  Hide playlists in channel search results if "hide channel playlist" preference selected. (FreeTubeApp#4454)
  Fix issue with subscribe button styling in specific views (FreeTubeApp#4451)
  Translated using Weblate (Lithuanian)
  Bump electron from 27.1.3 to 28.0.0 (FreeTubeApp#4442)
  Fix pr-labeler.yml (FreeTubeApp#4449)
  Translated using Weblate (Estonian)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants