Skip to content
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 Music - AMOLED black theme): Add AMOLED black theme patch #4258

Draft
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

ILoveOpenSourceApplications
Copy link
Contributor

@ILoveOpenSourceApplications ILoveOpenSourceApplications commented Jan 3, 2025

Closes #609, #614 (partially), #1391(partially), and #1491. (I know, all of them are my issues, crazy right.)

I have no idea what I've done here. But, since there's no one else to do this (at least for now), I shall be the Guinea pig. Open to changes (and many of that shall come).

Credits: @inotia00

@ILoveOpenSourceApplications
Copy link
Contributor Author

Is it any better now or just worse?

@oSumAtrIX
Copy link
Member

You're getting there; however, there's missing/incorrect abstraction, which isn't as easy to pull off as copying or moving files. I think it's better leaving this to us.

@oSumAtrIX oSumAtrIX marked this pull request as draft January 5, 2025 01:05
@ILoveOpenSourceApplications
Copy link
Contributor Author

@oSumAtrIX , do you think navigation bar theme is defined in the xml files so that this can work ?

document("res/values/themes.xml").use { document -> val resourcesNode = document.getElementsByTagName("resources").item(0) as Element for (i in 0 until resourcesNode.childNodes.length) { val node = resourcesNode.childNodes.item(i) as? Element ?: continue if (node.getAttribute("name") == "navigation_bar_color") { node.textContent = amoledBlackColor

@ILoveOpenSourceApplications
Copy link
Contributor Author

The reason I asked is mentioned in this #609 (comment). As currently, ReVanced YouTube Music settings page implementation is in works, only features that can be achieved while patching can be added, which is the theme patch.

Theming should all fall under one patch and the nav bar theme being separate felt off to me and I would like it to come under this patch as well but I don't know how to achieve that. Will you be able to look into that as well or should I do something about that?

@oSumAtrIX
Copy link
Member

I don't know what you're referring to with the settings, but theming for YT doesn't rely on the settings patch.

@ILoveOpenSourceApplications
Copy link
Contributor Author

I don't know what you're referring to with the settings, but theming for YT doesn't rely on the settings patch.

I don't think you read my comment completely.

@ILoveOpenSourceApplications
Copy link
Contributor Author

Please refer patches/src/main/kotlin/app/revanced/patches/music/layout/theme/DarkThemePatch.kt as he has made some changes and is now much more similar to our YouTube Theme patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants