revanced-patches
revanced-patches copied to clipboard
feat(YouTube): Show applied patches version in Misc settings menu
Adds a simple 'About' preference to the settings menu.
Also adds a link to the website.
The version displayed is pulled from the patches.jar manifest, so this needs no special changes to the build for this to work.
@LisoUseInAIKyrios Would be nice if it also showing applied integrations version. :)
Listing the integrations version is not difficult (do the same manifest lookup, but use an integrations class instead of a patch class).
But ReVanced manager does not mention the integrations version, so the information might be confusing for users. Those who patch with dev release probably know what they have.
Maybe if it was patched with dev it can show integrations version. Otherwise just show the patch release version.
Listing the integrations version is not difficult (do the same manifest lookup, but use an integrations class instead of a patch class).
But ReVanced manager does not mention the integrations version, so the information might be confusing for users. Those who patch with dev release probably know what they have.
Maybe if it was patched with dev it can show integrations version. Otherwise just show the patch release version.
However, sometimes there is an integration version update, without a patches version update. For that case, it would be handy to also show the integrations version.
Integrations aren't meant to be separate to begin with. This is a remnant back from how Vanced dealt with integrations and writing a system around it and supporting that design is not the way to go from here onwards.
With this, the call to the Announcements API now includes the patches version. So the announcements can provide more specific notifications on first launch, such as showing the old "You will see announcements here" message if their patch version does not have the playback history issue since their version is fixed.
I have conflicting opinions about including the version of the patches in the patched app. Showing which version was used when patching the app should be saved outside of the patched app. For example, ReVanced Manager could store the patches used for every patched app in the app info page. The "Settings"patch is also being abused for that purpose, whereas its purpose is merely to add settings to apps.
A proper approach
A proper approach would be to have the "Settings" patch have an abstract "BasePreference" class, which other patches can implement. The class would have an abstract "serialize" method, which the patches would implement for their "BasePreference" implementation. In this current case, it would mean, adding a "PatchesInformation" patch which would embed the patches used via an implementation of the abstract "BasePreference" class. Since the "Settings" patch only knows "BasePreference" and the "BasePreference" implementation in our "PatchesInformation" patch would extend the "BasePreference" class, the "Settings" patch would call the custom implementation of the "serialize" method.
This design maintains the single responsibility of the "Settings" patch. The problem would be that the patch would be deselectable, whereas we want to brand the "Settings" patch with our link.
Instead of embedding the patches version, it would be better to make use of https://api.revanced.app/v2/socials and add the links of ReVanced to the settings instead. This would also get rid of the hacky approach of extracting the patches version.
Adding the social links can be done, but the preference would need to open a new dialog or view to display that.
A preference can only have one tap action, so that action would need to open the social links view.
Would the patches version be useful for the announcement API call? Looking into the jar and reading the manifest is not that hacky, the manifest has a known format and Java has built in tools to read manifest entries.
A preference can only have one tap action, so that action would need to open the social links view.
A dialog can have a custom view, so that should be possible.
Would the patches version be useful for the announcement API call?
Generally speaking, yes. For example, if the app was patched with a problematic version of the patches.
Instead of calling https://api.revanced.app/v2/socials
and parsing the results to then display something fairly rigid (with probably no graphics or images), it would be simpler and way more flexible if the client opened an embedded webview that has the social links in a pretty html format. Then tapping on any link in that then opens the link in the external browser.
A dialog can have a custom view, so that should be possible.
Oh yes, I forgot about that. If it's just going to be a list of links in the preference then this would be simpler (but obviously less flexible) compared to opening an intermediate embedded browser for the social links.
The webview would need to call https://api.revanced.app/v2/socials and parse the results instead anyways. We would need a place to host it specifically for the purpose of YouTube, or statically embed it into the app. I think its fine to just show the links similar to how our announcements do it.
I'm trying to get multiple links working for the summary of a preference, and it's not working out well.
To get multiple touch listeners working, it requires either an xml layout file (which cannot be loaded from integrations, because layout files are declared in patches). Or it requires mucking about with removing all the existing summary views and adding your own programmatically (and I cannot get the layout to look right, and the padding and everything is different/wrong).
Even if the layout was figured out, it feels weird and unintuitive touching and tapping on parts of the summary to open different links, and the summary is huge with all the links. Then there's the question of how does it fetch the social links, and how frequently does it fetch. I didn't even get to the asynchronous fetch code needed for this, I just got tied up in the layout issues.
For the sake of usability and less mess in implementing this, I think this simple preference should behave as:
- One tap on the preference and it opens a website with the social links (either an embedded web view inside the app, or open straight to an external browser).
Or
- One tap on the preferences and it opens the ReVanced.app website (basically what is already here in this PR and ready to go).
If you check the announcements dialog, you will see that it parses the response as HTML and also renders clickable links correctly. Perhaps that can be used. No additional view or layout is necessary.
If you check the announcements dialog, you will see that it parses the response as HTML and also renders clickable links correctly. Perhaps that can be used. No additional view or layout is necessary.
That could work for a fancy popup dialog that opens when the preference is tapped (and would be minimal and look great). But that requires a new backend end point or it requires modifying the social API endpoint and add a 'fancyhtml' field containing the html for this dialog.
For now can we use something similar to what's here, and make this smart and fancy later? I wanted to merge this before merging the GmsCore dialog improvement.
That could work for a fancy popup dialog that opens when the preference is tapped (and would be minimal and look great). But that requires a new backend end point or it requires modifying the social API endpoint and add a 'fancyhtml' field containing the html for this dialog.
The client would download the JSON from the API, build a small HTML string with tags and display it in the dialog. Moving this to API would be way too low.
For now can we use something similar to what's here, and make this smart and fancy later? I wanted to merge this before merging the GmsCore dialog improvement.
I still have conflicted opinions on reading the patches version. I need a little more time to think about this.
The client would download the JSON from the API, build a small HTML string with tags and display it in the dialog. Moving this to API would be way too low.
Sticking some basic html in the API call isn't much different from what the announcements call is already doing.
I'll try doing a popup dialog with the social links similar to how the announcements are shown.
I changed it to show a dialog with links from the social api call.
The icons are using the favicon.ico
for each of the links. But since the discord link is a link to revanced.app, it would be best if https://api.revanced.app/v2/socials
was modified to include an icon url so then discord can have it's own appropriate icon (although, discord.com does not have a standard 'favicon.ico' so this would also require hosting an icon somewhere else). Setting the fav icon in the API would also allow using higher resolution icons (such as reddit here)
The social links might benefit from changing the revanced.app
name from "Website" to "ReVanced.app", since users constantly mix up what is the website. And the word 'website' is not localized (using 'ReVanced.app' requires no localization)
I have pushed some changes that change the appearance of the About dialog and add dark theme if the dark theme is enabled:
Unfortunately, dark icons like that of GitHub will blend with the dark background color, but that's better than being flash-banged at night.
What can be considered to change is adding a notice about counterfeits in the body of the dialog and moving the About settings preference to the top settings page, out of the misc sub-menu, since it is about ReVanced, which would fit in the injected ReVanced preference screen.
Currently, this preference is only added to YouTube, although it would make sense to have this same menu in every app with settings.
I have pushed some changes that change the appearance of the About dialog and add dark theme if the dark theme is enabled:
Looks awful should be arranged like revanced manager
I think we both pushed dark mode code. It could use hard coded dark values, but the code I added uses the resource colors so it should use any custom theme colors the user may have applied.
Looks awful should be arranged like revanced manager
Manager has a completely different UI look compared to YT (or any other target app for that). Mimicking the look and feel of Manager in just one app menu is not a requirement.
The github logo can be fixed by changing the social api to include both light and dark icon urls.
GitHub uses these, and they appear correct in app: https://github.githubassets.com/favicons/favicon.svg https://github.githubassets.com/favicons/favicon-dark.svg
Other better icons that can be used (same icon is for light/dark mode): Discord: https://discord.com/assets/images/favicon.ico Reddit: https://www.redditstatic.com/shreddit/assets/favicon/192x192.png Telegram: https://web.telegram.org/k/assets/img/android-chrome-192x192.png Twitter: https://abs.twimg.com/responsive-web/client-web/icon-default-large.9ab12c3a.png YouTube: https://www.gstatic.com/youtube/img/branding/favicon/favicon_192x192.png
I am not sure if it's allowed to use trademark assets freely like its done in this PR. I know that Vanced got into trouble due to trademark violations in a related situation with Google. This also applies to all the other icons we plan to show. YouTube for example, requires explicit approval, as stated on https://www.youtube.com/howyoutubeworks/resources/brand-resources. Similarly, we may need to ask for approval from other sites. Do you have any idea regarding this?
Can check the brand guidelines for each company and see what it says. These icons are not hosted by ReVanced and come straight from the websites favicon. RV Manager is already using the icons for some of these sites
For YouTube can remove the social links (or make it not preferred and not display in app). There is no content on the YT channel anyways.
RV Manager is already using the icons for some of these sites
We have once asked for approval for the YouTube trademark logo, but I think the rest of the icons we have not yet.
For YouTube can remove the social links (or make it not preferred and not display in app). There is no content on the YT channel anyways.
It's more of a placeholder for future content and a verification method since counterfeit sites can't show this kind of reach.
Alternatively, we can just not show any icons at all and only the links. This spares us from having to make changes in the API to suite this PR and other matters such as approvals to use the logos.
Looking at a few of the brand guidelines, it appears using their own fav icon has no issue (it's straight from them after all).
The only issue I see is the complex guideline for how the text should appear with the icon, such as the Reddit guideline stating orange text is preferred when the background is white, and white text if the background is black. Plus other preferences of the font and padding around the logo/name.
The branding kits for each of these sites provide banners where the logo and text is one part and it has the preferred font, spacing, color, etc. Those could be used as-is without modification, but it would require hosting the banner files and then this about screen might as well be a regular html file hosted on ReVanced.app so the programmatic html styling can be removed from this about preference code.
Maybe these sites don't care if their own favicon are shown beside regular text links to their websites, but maybe they do care.
For now, the icons can be removed until someone gets some kind of confirmation that "showing our favicon with a regular text link is ok".
Some of these logos are on the website, such as https://revanced.app/socials/reddit.svg
Another alternative, is to add the remaining icons to the website (YouTube, Twitter, GitHub), and then this about screen can show just the icon and not any text. Then there's no issues of what text color, spacing, or any of that. It would also look consistent with the website and more consistent with Manager.
For now, the icons are removed. But it looks a lot less exciting.
Can figure out how to make this pretty later.
Because the icon urls were not added to the api, this PR is no longer dependent on the small changes in https://github.com/ReVanced/revanced-api/pull/173
@oSumAtrIX Does this PR need any other changes?