-
-
Notifications
You must be signed in to change notification settings - Fork 962
fix: Adjust ReVanced icons #2751
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
Conversation
These never did anything and were removed from the figma a while ago.
I can't believe this happened
Co-authored-by: Ax333l <[email protected]>
|
Also fix #1124 |
validcube
left a comment
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.
🥞💄 Approved, see files section above for preview.
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.
Is it possible to use the logo with the border for rounded icons?
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.
Currently, the logo with the frame has a rounded V and is different from the logo without the frame.
We need to come to a consensus on which logo to use, I asked this question before (ReVanced/revanced-patches#6007 (comment) and ReVanced/revanced-patches#6007 (comment)), but there was no answer, so I used the latest modified logo with "sharp" edges
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 conversation here about rounded icons is lagged and was resolved in the patches PR.
For which logo to use? (Sharp inner V, or rounded inner V)? It should be whatever is in the branding guidelines repo.
It's odd that Manager has been using a slightly different (and slightly wrong?) logo this whole time.
|
Hello! Please rebase your change on top of the latest Also, I forgot about this but don't you need to specify Thanks! |
The old round and square non adaptive icon can still be used for pre Android 8.0 devices, or for third party launchers that have forced off adaptive icons. It's an unusual edge case, but it's possible they could still be used. I think Google suggested ignoring updating the old icons to reduce work load for rare use cases, but they didn't say to remove the usage of them. |
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.
Need to rebase again.
Lots of random and unrelated commits are here.
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 can't do a rebase, please tell me how to do it correctly.
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.
If using command line:
-
add upstream remote (if you haven't already):
git remote add upstream https://github.com/ReVanced/revanced-manager.git -
Fetch upstream:
git fetch upstream -
Rebase onto upstream:
git rebase upstream/compose-dev -
Force push your branch to GitHub.
Or can do all this inside Intellij or Android Studio using menu commands.
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 if all that fails, basically delete the branches and start over with a new branch.
Delete this branch, delete your compose-dev branch, checkout compose-dev again, make a new branch from compose-dev, apply icon changes, commit & push.
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 think the merge from remote before rebasing is what got this into a mixed up state.
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 isn't needed. Just checkout the new compose branch and cherry pick your pr commits on top then push
# Conflicts: # .github/workflows/release.yml # api/build.gradle.kts # app/.releaserc # app/build.gradle.kts # app/src/main/res/drawable/ic_launcher_round.png # app/src/main/res/mipmap-hdpi/ic_launcher.png # app/src/main/res/mipmap-mdpi/ic_launcher.png # app/src/main/res/mipmap-xhdpi/ic_launcher.png # app/src/main/res/mipmap-xxhdpi/ic_launcher.png # app/src/main/res/mipmap-xxxhdpi/ic_launcher.png # docs/2_3_managing_patches.md # gradle.properties # gradle/libs.versions.toml # package-lock.json # package.json
4ecc8d3 to
77722b3
Compare
77722b3 to
26a1f2a
Compare
|
Moved to #2762 |
Synchronize changes with ReVanced/revanced-patches#6029