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

Added buttons to hide hidden channels/text #6156

Open
wants to merge 11 commits into
base: development
Choose a base branch
from

Conversation

JL0000
Copy link

@JL0000 JL0000 commented Nov 12, 2024

Added buttons to hide hidden channels/text

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

closes #5652

Description

  • Added toggle switches to hide/show the channels and text hidden by filters set by user
  • Grouped the toggles together with the filtering into a new section

Screenshots

Before
before
After
after

Testing

Settings persist after closing the application.

Desktop

  • OS: MacOs
  • OS Version: 15.1
  • FreeTube version: v0.22.0 Beta

Additional context

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 12, 2024
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 12, 2024 23:53
auto-merge was automatically disabled November 13, 2024 00:25

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 13, 2024 00:25
@efb4f5ff-1298-471a-8973-3d47447115dc

Errors when entering the settings page and using the toggles

VirtualBoxVM_civ1heeKvZ.mp4

@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 Nov 15, 2024
auto-merge was automatically disabled November 15, 2024 12:08

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 15, 2024 12:08
@JL0000
Copy link
Author

JL0000 commented Nov 15, 2024

Fixed it.

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: changes requested labels Nov 15, 2024
@PikachuEXE
Copy link
Collaborator

I think the label reads a bit confusing to me
Show Hidden Channels at first I thought it was disabling hiding channels then I read Show Hidden Text.
I think it would be better to label them as something like "show all already entered list of channels/text" (Not sure what exact words to use)
Or move the controls below each text input field (though not the best position I guess) so users can relate that control to the list of videos/texts

@absidue absidue added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Nov 21, 2024
@JL0000
Copy link
Author

JL0000 commented Nov 21, 2024

What do you think about List Hidden Channels and List Hidden Words?

@PikachuEXE
Copy link
Collaborator

Show Stored/Saved Hidden Channels & Show Stored/Saved Hidden Keywords?
Weird to see List...

@JL0000
Copy link
Author

JL0000 commented Nov 22, 2024

Because I name this section Content Filter I think Show Channels Added to Content Filter and Show Keywords Added to Content Filter works well. If content filter is vague, then Show Stored Hidden Channels and Show Stored Hidden Keywords works well too.

@PikachuEXE
Copy link
Collaborator

Show Added Channels + Show Added Keywords?
Assuming they are under heading Content Filter

auto-merge was automatically disabled November 22, 2024 15:14

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 22, 2024 15:15
@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: changes requested labels Nov 23, 2024
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.

suggestion: I hate to be the guy who comes in late with new things to say. This feature does work well, but I am still skeptical of the toggle setup. Couldn't we do a Show / Hide button where the content is supposed to appear / disappear (e.g., like our recent Hide/Show description feature) instead of toggles above the whole container? That way the positional and logical relationship of the control to the corresponding section is much more clear. I ask particularly because I thought I was in the wrong branch for a couple minutes when trying to test this.

@JL0000
Copy link
Author

JL0000 commented Nov 23, 2024

suggestion: I hate to be the guy who comes in late with new things to say. This feature does work well, but I am still skeptical of the toggle setup. Couldn't we do a Show / Hide button where the content is supposed to appear / disappear (e.g., like our recent Hide/Show description feature) instead of toggles above the whole container? That way the positional and logical relationship of the control to the corresponding section is much more clear. I ask particularly because I thought I was in the wrong branch for a couple minutes when trying to test this.

I think it does make it more coherent. Maybe to the right of each tooltip?
Also, can you link me the PR for the "recent Hide/Show description feature" please?

@kommunarr
Copy link
Collaborator

Ah, I didn't see that wasn't merged yet. Here's that implementation I'm thinking of for reference!

auto-merge was automatically disabled November 25, 2024 17:47

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 25, 2024 17:47
auto-merge was automatically disabled November 25, 2024 17:48

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 25, 2024 17:48
auto-merge was automatically disabled November 25, 2024 20:50

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 25, 2024 20:50
Copy link
Member

Choose a reason for hiding this comment

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

I would like the showTags to be true by default so that this doesnt visually change anything for our users in the next version. If someone decided to uncheck it than that should be remembered within the session but also on next startup. This is how it works with all the toggles in the settings

Settings dont get remembered when navigating away and back to the settings page

VirtualBoxVM_An3To9T6Qf.mp4

@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 Nov 25, 2024
auto-merge was automatically disabled November 25, 2024 23:22

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 25, 2024 23:22
auto-merge was automatically disabled November 25, 2024 23:31

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 25, 2024 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: "Hide Videos From Channels" List As Submenu
5 participants