-
-
Notifications
You must be signed in to change notification settings - Fork 248
feat(YouTube - Alternative Thumbnails): Add option to use DeArrow #534
feat(YouTube - Alternative Thumbnails): Add option to use DeArrow #534
Conversation
Alternative Thumbnails
Patch
@LisoUseInAIKyrios I have moved lines of code for review; look into the commits separately for simplicity |
app/src/main/java/app/revanced/integrations/patches/AlternativeThumbnailsPatch.java
Outdated
Show resolved
Hide resolved
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 original reason why DeArrow was not used for this patch, was that the patch blocks the UI thread. This means, if the API times out, the UI will be blocked for the timeout duration. Does this PR consider this in any way?
This patch actually does not block the UI, everything happens off the main thread. The only reason DeArrow was not added was because nobody took the time to finish adding it until now. |
It would be fantastic if DeArrow added a parameter to only provide user selected thumbnails. But that's something DeArrow would need to implement. |
app/src/main/java/app/revanced/integrations/patches/AlternativeThumbnailsPatch.java
Outdated
Show resolved
Hide resolved
Because the settings menu is not dynamically changed the same way SponsorBlock or RYD is, the settings menu for this is now a bit confusing as to what the end user will get. This was the largest reason I put off adding DeArrow to the first iteration of this. I think this might be better if it had it's own settings UI fragment, as then it can give a single 'about' entry and explain what the end result is for the user based on the current setting values. It can also disable settings that don't apply (such as fast still images, when the currently selected settings do not use still images). Edit: Added an 'about' preference that is updated based on the current settings. |
- Fix validation of API url.
app/src/main/java/app/revanced/integrations/patches/AlternativeThumbnailsPatch.java
Outdated
Show resolved
Hide resolved
app/src/main/java/app/revanced/integrations/patches/AlternativeThumbnailsPatch.java
Outdated
Show resolved
Hide resolved
… not work correctly because YouTube includes many device and user parameters in the request headers. Committing this code for any future reference.
…This may not work correctly because YouTube includes many device and user parameters in the request headers. Committing this code for any future reference." This reverts commit bd51f08.
I think this PR is nearly ready. On a related topic, navigating to the alt thumbnails menus is quite a journey that needs a lot of scrolling. Not for this PR, but perhaps the ReVanced/RYD/SB menu can be moved to the top of the settings menu. So they are first and above the stock YouTube general settings. Stock YT settings are rarely changed, but ReVanced settings can often be changed and each time it requires scrolling past a lot of mostly unused stock settings. And similarly, maybe move the ReVanced sub menus to the top of each list. So Alt thumbnails, Action bar settings, etc, all appear first and before all the single item preferences. |
@oSumAtrIX When you have time look this over and add your review. Otherwise I think it's ready to merge. |
app/src/main/java/app/revanced/integrations/patches/AlternativeThumbnailsPatch.java
Outdated
Show resolved
Hide resolved
app/src/main/java/app/revanced/integrations/patches/AlternativeThumbnailsPatch.java
Show resolved
Hide resolved
...java/app/revanced/integrations/settingsmenu/AlternativeThumbnailsAboutDeArrowPreference.java
Outdated
Show resolved
Hide resolved
If they're moved to a submenu, then the base menu would be only a status with more menus under that. It's already a long navigation journey to reach the menu (every time I scroll thru the enormous layout menu to reach the thumbnail menu i can almost imagine Lord of the Rings memes of Frodo's journey) I agree the two sections can be more segregated. I think two PreferenceCategorys on the same screen might work to visually separate them. I'll give this a try and see how it looks. |
Currently, categories do not exist as components in the SettingsPatch. They would be highly welcome as they will be useful to separate settings sections. The SettingsPatch would need a new interface, for example, SettingsComponent, which Preference and Category could implement. A category can then have additional settings components as its children. |
Here is the diff in case a submenu fits more into this PR Open>
|
In that case settings categories can be a different PR. |
# [1.0.0-dev.9](v1.0.0-dev.8...v1.0.0-dev.9) (2023-12-11) ### Features * **YouTube - Alternative Thumbnails:** Add option to use DeArrow ([#534](#534)) ([c4ee6ca](c4ee6ca))
# [1.0.0](v0.125.0...v1.0.0) (2023-12-12) ### Bug Fixes * **YouTube - Announcements:** Don't show error toast if there is no internet connection ([#537](#537)) ([0ce92c2](0ce92c2)) * **YouTube - Client spoof:** Do not break clips ([f9102fa](f9102fa)) * **YouTube - Minimized playback:** Fix PIP incorrectly shown for some Shorts playback ([#533](#533)) ([fb433da](fb433da)) * **YouTube - Return YouTube Dislike:** Fix dislikes sometimes not showing for non English language ([5d4c8b0](5d4c8b0)) * **YouTube - Return YouTube Dislike:** Prevent the first Short opened from freezing the UI ([#532](#532)) ([0bb8669](0bb8669)) * **YouTube - Return YouTube Dislike:** Wait until fetch is complete before allowing the first Short to start playback ([#538](#538)) ([1c9c51c](1c9c51c)) * **YouTube - SponsorBlock:** Allow autoplay when skipping to the end of the video ([3d660e1](3d660e1)) * **YouTube - SponsorBlock:** Prevent autoplay from stopping to work ([f4e2d56](f4e2d56)) * **YouTube - Spoof signature:** Wait until storyboard fetch is done ([#535](#535)) ([92e8619](92e8619)) ### Features * Allow choosing the vendor of GmsCore via patch options ([#529](#529)) ([fba7181](fba7181)) * **Tiktok:** Bump compatibility to `32.5.3` ([#536](#536)) ([10a1e16](10a1e16)) * **YouTube - Alternative Thumbnails:** Add option to use DeArrow ([#534](#534)) ([c4ee6ca](c4ee6ca)) * **YouTube:** Add `Change start page` patch ([792dc0c](792dc0c)) ### BREAKING CHANGES * The class `MicroGSupport` has been renamed to `GmsCoreSupport`
adds the option to use thumbnails provided by DeArrow to the existing alternative thumbnails patch
requires ReVanced/revanced-patches#3378