-
-
Notifications
You must be signed in to change notification settings - Fork 340
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
feat(YouTube - Settings): Add icons to the ReVanced settings #4496
base: dev
Are you sure you want to change the base?
Conversation
The YouTube brand guidelines does not mention Shorts or the Shorts logo, so it's unclear if permission is needed to use a Shorts icon. https://www.youtube.com/howyoutubeworks/resources/brand-resources |
# Conflicts: # patches/src/main/kotlin/app/revanced/patches/youtube/misc/settings/SettingsPatch.kt # patches/src/main/resources/settings/drawable/revanced_settings_screen_00_about.xml # patches/src/main/resources/settings/drawable/revanced_settings_screen_01_ads.xml # patches/src/main/resources/settings/drawable/revanced_settings_screen_02_alt_thumbnails.xml # patches/src/main/resources/settings/drawable/revanced_settings_screen_03_feed.xml # patches/src/main/resources/settings/drawable/revanced_settings_screen_04_general.xml # patches/src/main/resources/settings/drawable/revanced_settings_screen_05_player.xml # patches/src/main/resources/settings/drawable/revanced_settings_screen_06_shorts.xml # patches/src/main/resources/settings/drawable/revanced_settings_screen_07_seekbar.xml # patches/src/main/resources/settings/drawable/revanced_settings_screen_08_swipe_controls.xml # patches/src/main/resources/settings/drawable/revanced_settings_screen_11_misc.xml # patches/src/main/resources/settings/drawable/revanced_settings_screen_12_video.xml
I took the Shorts icon from some site and edited it to size. It is not in the Google icon repository. In fact, it is custom icon. |
Their brand guidelines apply to custom graphics as well. The question is if the Shorts logo needs permission to use. |
I think it's better to add Copyright than to decide if it is necessary or not. |
https://en.m.wikipedia.org/wiki/File:Youtube_shorts_icon.svg Wikipedia lists the vector icon file as public domain, but with a caution that it might be trademarked. Searching the USPTO website shows that "YouTube Shorts" is trademarked: But the trademark does not include a wordmark or logo. For reference the YouTube logo and wordmark is trademarked So, maybe it's ok to modify and use the public domain vector icon. |
Can't it depend on the app's native icon resources? |
If that built in icon matches the line width of the other icons (or the other icons can be adjusted to match the built in icon), then using that would be best. But I think using the icon already here might be ok too |
patches/src/main/kotlin/app/revanced/patches/shared/misc/settings/SettingsPatch.kt
Outdated
Show resolved
Hide resolved
03d0585
to
35bec9e
Compare
35bec9e
to
d1a4577
Compare
If necessary, it is also possible to add icons to the submenus, but it seems to me that this goes beyond the design code of the YouTube settings. Although, for example, hiding buttons for the user would be intuitive as to which button would be hidden. |
I think icons for only some submenu items but not for other items would look out of place. The hide action and navigation buttons are the only settings that an icon might help, but I think it's easy to understand what those settings do just from the setting title text. |
Looks good, but maybe remove the gear icon. The Shorts settings are less like the video settings and more like the player (which has no gear). |
Ok, it was a second variant =) |
The animated images you suggested also looks ok. |
There's multiple considerations with this PR
|
I tried to choose icons more appropriate to the context of the menu item. But IMHO, at the moment, Google has a poor taste in the design code in the YouTube settings, some icons are simply terrible. |
These icons could be default off, so users need to opt in to see them. |
After