Skip to content

fix: Use system background colour for V-shaped outline #2635

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

Open
wants to merge 4 commits into
base: compose-dev
Choose a base branch
from

Conversation

validcube
Copy link
Member

@validcube validcube commented Jul 5, 2025

Fix one of the problem on #1191 about ReVanced logo (V-shape) is hardly seen in light mode.

@validcube validcube added the ReVanced Manager Compose Regarding the Compose rewrite of ReVanced Manager label Jul 5, 2025
@validcube

This comment has been minimized.

@validcube validcube marked this pull request as draft July 5, 2025 14:16
@validcube validcube marked this pull request as ready for review July 5, 2025 14:17
@Ushie
Copy link
Member

Ushie commented Jul 5, 2025

The resulting icon is not present here: https://github.com/ReVanced/revanced-branding/blob/main/assets/revanced-logo

Although this doesn't go against the ReVanced Branding guidelines, if ReVanced plans to officially use that variation of dark V and colored diamond, it should be added there. Otherwise it might be better fitting to use a different variation that is better fitting

@Ushie Ushie marked this pull request as draft July 5, 2025 22:46
@validcube
Copy link
Member Author

The resulting icon is not present here: https://github.com/ReVanced/revanced-branding/blob/main/assets/revanced-logo

Although this doesn't go against the ReVanced Branding guidelines, if ReVanced plans to officially use that variation of dark V and colored diamond, it should be added there. Otherwise it might be better fitting to use a different variation that is better fitting

Should that dynamic icon be in the branding guidelines? I don't see the reason to include it as this is specific to ReVanced Manager.

As for the colour, I'm not sure what colour attribution to use because using text doesn't make sense, and background colour is not inverse. But these are value made likely for the material design era so it's highly likely that the black colour is #0, and white is #f2f2f2

@Ushie
Copy link
Member

Ushie commented Jul 6, 2025

The branding icons include a light and dark variant for each design except seemingly the one with the colorful diamond, maybe that was intentional? Or it's not and it was just forgotten

cc @oSumAtrIX

@brosssh

This comment has been minimized.

@validcube

This comment has been minimized.

@validcube validcube requested a review from brosssh July 9, 2025 13:57
@validcube
Copy link
Member Author

The resulting icon is not present here: ReVanced/revanced-branding@main/assets/revanced-logo

Although this doesn't go against the ReVanced Branding guidelines, if ReVanced plans to officially use that variation of dark V and colored diamond, it should be added there. Otherwise it might be better fitting to use a different variation that is better fitting

This should be fixed now

@validcube validcube requested a review from Ushie July 9, 2025 13:59
Copy link
Member

@brosssh brosssh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, icon has been fixed as well as splash art.

@oSumAtrIX
Copy link
Member

The branding icons include a light and dark variant for each design except seemingly the one with the colorful diamond, maybe that was intentional? Or it's not and it was just forgotten

cc @oSumAtrIX

You can use the shape variant with a background just fine

@validcube validcube marked this pull request as ready for review July 9, 2025 14:38
@brosssh
Copy link
Member

brosssh commented Jul 16, 2025

Can this be merged? Should the splash art be used with a background instead?

@validcube
Copy link
Member Author

Can this be merged? Should the splash art be used with a background instead?

On second thoughts, I like the idea of having a splash art use with a background.

cc: reviewers @Axelen123 do you have any objections?

@Axelen123
Copy link
Member

I don't have any objections to that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ReVanced Manager Compose Regarding the Compose rewrite of ReVanced Manager
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants