-
-
Notifications
You must be signed in to change notification settings - Fork 417
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?
Conversation
Regarding
A rewrite to implement it via the preferences framework would be ideal. It would restore consistency with other preferences and also allow searching. May be out of scope of this PR, but would be welcome in it as well. |
@LisoUseInAIKyrios Do you think if a fuzzy search would be possible? Casing, spacing and co is something many would have to trial and error for probably. |
if (TextUtils.isEmpty(query)) { | ||
restoreOriginalPreferences(preferenceScreen, originalPreferenceScreen); | ||
} else { | ||
String queryLower = query.toLowerCase(); |
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.
I don't think the preference text has any ambiguos spacing
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
patches/src/main/kotlin/app/revanced/patches/youtube/misc/settings/SettingsPatch.kt
Outdated
Show resolved
Hide resolved
sortPreferenceListMenu(Settings.CHANGE_START_PAGE); | ||
sortPreferenceListMenu(Settings.SPOOF_VIDEO_STREAMS_LANGUAGE); |
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
...main/java/app/revanced/extension/youtube/settings/preference/ReVancedPreferenceFragment.java
Outdated
Show resolved
Hide resolved
...main/java/app/revanced/extension/youtube/settings/preference/ReVancedPreferenceFragment.java
Outdated
Show resolved
Hide resolved
...sions/youtube/src/main/java/app/revanced/extension/youtube/settings/LicenseActivityHook.java
Outdated
Show resolved
Hide resolved
Doing it for RYD would be fairly simple since it doesn't have any custom behavior except the API stats (but those could be handled using a custom preference that hides itself if debug is off). But converting all of SB would be way more substantial, since it has a lot of custom code to validate/fix user input, show custom colors/segments, user stats, and more. |
This comment was marked as resolved.
This comment was marked as resolved.
Fuzzy search would be ideal, but obviously adds complexity. Other fuzzy search ideas are if multiple words are searched for, and the search results are few (less than maybe 10), then show the combined search results of each word individually (and stop combining sub searches when 10+ results exist). But fuzzy doesn't handle if a user searches for a different wording of a preference. Preferences could declare related words that are silently searched against. So SponsorBlock could declare [ "sponsor block", "sponsors", etc ] Another example is disable resuming Shorts (Preference summary is: |
That's correct behavior. Fuzzy aids the user and prevents light spelling mistakes. It doesn't make attempts to guess what the user might search. The guess approach doesn't look ideal. It won't be useful in most or make weak guesses at what the user might or might not type. Fuzzy search is sufficient on the other hand. I don't think it adds any complexity other than a library for fuzzy searching |
This comment was marked as resolved.
This comment was marked as resolved.
If inputs needs validation a custom input component can be made. Similarly the color and popup ones. Or those can be removed and they can consistently use the same input preferences as other preferences. Consistency is more important here which would also solve side effects as seen in this PR |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I cannot reproduce either of the issues. Locking/unlocking works, and I no longer can reproduce the SB unresponsive settings. Are you sure you're on the latest commit? I forced pushed, so you'll need to force reset if you're on a divergent branch. |
This comment has been minimized.
This comment has been minimized.
Maybe related to the fragment lifecycle (onPause, onResume)? |
This comment has been minimized.
This comment has been minimized.
What if SponsorBlock is not included during patching? Is the lock/unlock issue fixed? |
This comment has been minimized.
This comment has been minimized.
The issue only shows up when locking/unlocking? I think it's caused by |
Also, when use a gesture to go to the desktop and return to the app. If do this with other words in the search, the search will be empty. |
Since the preference key is searched, it might be worth reviewing the existing setting keys and adjusting the key names if any could cause false positive/negative search results. |
…sing menu settings
I think it would be appropriate to also implement the |
The built in dependency declaration is very simple and doesn't allow the more complicated dependencies of the ReVanced settings (such as turning on either swipe volume or swipe brightness enables the same sub settings). But this functionality could be done using the existing setting availability function. |
The problem was solved. Thanks for the good work =) |
Until the SB settings is completed, the search by its categories is not yet working. |
…verything else can be completed.
… until everything else can be completed." This reverts commit f672ff4.
edit: SponsorBlock category settings now show in the search results. Overhauling the last of the SB settings will be a decent amount of work. I think for now what's here is ok. |
patches/src/main/resources/settings/layout/revanced_settings_with_toolbar.xml
Outdated
Show resolved
Hide resolved
|
I tried to implement to close the search field when pressing back, but it's difficult to do without the |
SponsorBlock is showing up with duplicates again. Needs a small adjustment to fix. |
Adds search feature to YouTube ReVanced settings.