Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
feat(YouTube - Settings): Add ability to search in settings #4881
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
base: dev
Are you sure you want to change the base?
feat(YouTube - Settings): Add ability to search in settings #4881
Changes from 1 commit
073e99b
f616b7f
af49e1e
e4e6927
768e54c
80ccc8a
9315832
e723218
e98a6d5
2da1a90
eef25f9
75fd5e6
35c794c
f910780
31b1c1c
82da35a
fe6b963
c96fa38
82f488c
6b60d9b
8ca7973
8e6c2ed
e8ee20a
30355d6
2cb1118
f4d9a01
00f4944
98e8993
3a0602a
e3de443
ed38d91
f6db6c5
db626ea
1d42739
0fd733d
04c8572
be884f3
7079e62
06769f4
212b946
eec41da
f672ff4
2ea2214
ffc7318
93e35f1
65c20bf
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also strip spaces? "sponsorblock" and "sponsor block" would match.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or things like ', " etc. Lowercase alphabet would be ideal probably. @LisoUseInAIKyrios what do you say?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the exception of "SponsorBlock" and "Sponsor Block", I don't think the preference text has any ambiguos spacing or punctuation that would affect searching. Even if searching for "sponsor block", the SponsorBlock settings will show before they get to "block". Using a combination of individual word searches (if little to no results exist) I mentioned would also fix this since "sponsor block" would show the combined search of each word "sponsor" and "block".
Since the results show up as the user is typing, I think they may stop when the result they want is shown.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's just an assumption. There's no benefit in including spaces in search. It's only beneficial for the statistics to omit them from the search as it's more unlikely to be useful in search than not useful
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LisoUseInAIKyrios Searching should be possible via the base implementation of the settings patch, so when we add the patch to other apps, its there as well. Isn't this implemented in the wrong place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only simple solution is to make a self sorting preference list, but there's nuances to that since for this use case the first list element should always be first (so 'default' or 'disabled' is always first).
The biggest headache of doing that is the way preferences are programmatically created during patching, then serialized to xml, then de-serialized during runtime. Because of the xml serialization it makes adding extra custom parameters very difficult or overly complex (such as do not sort the first N preference list elements).
The best solution is to not do the xml serialization/deserialization and instead create all preferences at runtime, similar to how ReVanced TikTok does it. Then all kinds of extra data and behavior could easily be added to the preference objects and not what RV YouTube does with clunky static methods modifying stock Android Preferences.
Making that change is way outside the scope of this PR and would be a lot of work to accomplish, but it would both simplify things and open up a lot of use cases that are difficult or almost impossible to do right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR would break compilation on the attempt to separate the shared patches to their own extension as the youtube unspecific patch references youtube specific settings. For that the code needs to be restructured