This repository was archived by the owner on May 1, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix UWP Toolbar icons #8147
Merged
Fix UWP Toolbar icons #8147
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hartez
suggested changes
Oct 22, 2019
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.
Just need to deal with the TODO
samhouts
approved these changes
Oct 24, 2019
I think if we really want to (in a later stage) there are some APIs available within UWP to make an image monochromatic. Which comes of course at a performance cost. I've looked into making this possible with this change immediately, but using those APIs seemed to take another good but of effort which I'm not sure is worth it at this point. The other way around is also true: I discovered that there is a property on the icons (that are used now) which lets you make them non-monochromatic. Maybe something we could expose as a platform specific or something. |
hartez
approved these changes
Nov 4, 2019
Merged
2 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
approved
Has two approvals, no pending reviews, and no changes requested
breaking
Changes behavior or appearance
p/UWP
partner/cat 😻
t/bug 🐛
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
Change the icons on UWP ToolbarItem to map to
AppBarButton.Content
and add anImage
in there instead of using theAppBarButton.Icon
directly because of a bug in UWP.Issues Resolved
API Changes
None
Platforms Affected
Behavioral/Visual Changes
The toolbar icons will look a bit different and less "iconized". You can now show full colored icons.
If users are not doing so already, they can create monochromatic images to mimic the old behavior themselves.
Before/After Screenshots
As you can see in the screenshots below there is a slight difference in the look whenever the toolbar item is primary. This change also allows full color icons to be used.
You can also see a black square, that is the full color Xamarin logo. In the old way, this was not supported, in the new way it is. You can see while using the new way for a secondary item the full colored Xamarin logo is (still) a black square. To make an icon still show up when a toolbar item is secondary, we still need to assign it directly to
AppBarButton.Icon
. I have tested if the bug with the secondary monitor also affects items that are secondary, but from what I can tell it does not.Original icons used in both screenshots below
Old
New
Testing Procedure
Go to Issue7505 and inspect the toolbar icons. It will take a dual monitor setup (with different resolutions) to test if the icons are not disappearing anymore. To see the old vs new appearance you would have to comment some lines in the
Platform.cs
file. Or you can compare the looks from the secondary item icons and the primary ones.PR Checklist