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

fix: android maincommand tint #1339

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rajamatt
Copy link
Contributor

@rajamatt rajamatt commented Feb 11, 2025

GitHub Issue (If applicable): #closes https://github.com/unoplatform/add-private/issues/30

PR Type

What kind of change does this PR introduce?

  • Bugfix

What is the current behavior?

The MainCommand on Android is being set a tint on-top, making it appear monochrome.

What is the new behavior?

The MainCommand will appear monochrome if specified in the BitmapIcon.ShowAsMonochrome property.

PR Checklist

Please check if your PR fulfills the following requirements:

Other information

Internal Issue (If applicable):

@rajamatt rajamatt self-assigned this Feb 11, 2025
@rajamatt rajamatt marked this pull request as draft February 11, 2025 14:34
@rajamatt rajamatt force-pushed the dev/mara/fix-android-maincommand branch from 3dfbb05 to dd00fd9 Compare February 11, 2025 15:21
@rajamatt rajamatt force-pushed the dev/mara/fix-android-maincommand branch from 875ba08 to fc7b45a Compare February 11, 2025 15:29
@@ -84,7 +84,7 @@ protected override void Render()

// Foreground
var hasIconColor = element.TryGetIconColor(out var foregroundColor);
var foregroundOpacity = hasIconColor ? foregroundColor.A / 255d : 0d;
var foregroundOpacity = hasIconColor ? foregroundColor.A / 255d : 1.0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this fix is better than the last (see first commit). We can remove the TryGetColorWithOpacity() check behind the scenes. We are already getting and setting the opacity here so

(1) If the user really wants a lower opacity they can just specify it in the AppBarButton.Opacity property.
(2) If its really just the BitmapIcon's opacity that the user wants to lower it will work as before because we have this

@rajamatt rajamatt requested a review from kazo0 February 11, 2025 22:06
@rajamatt rajamatt marked this pull request as ready for review February 11, 2025 22:06
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.

1 participant