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

Bugfix: Windows IconColorTintEffect HeightRequest #1271

Merged
merged 6 commits into from
Jul 5, 2023

Conversation

kperdlich
Copy link
Contributor

@kperdlich kperdlich commented Jun 28, 2023

Description of Change

  • Use AnchorPoint instead of Offset to align image correctly + apply additonal offset whenever a specific size is requested.

Linked Issues

PR Checklist

Additional information

For quick testing I adjusted IconTintColorBehaviorPage.xaml the following:

<?xml version="1.0" encoding="utf-8" ?>
<pages:BasePage
	x:Class="CommunityToolkit.Maui.Sample.Pages.Behaviors.IconTintColorBehaviorPage"
	xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
	xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
	xmlns:mct="http://schemas.microsoft.com/dotnet/2022/maui/toolkit"
	xmlns:pages="clr-namespace:CommunityToolkit.Maui.Sample.Pages"
	xmlns:vm="clr-namespace:CommunityToolkit.Maui.Sample.ViewModels.Behaviors"
	Title="IconTintColorBehaviorPage"
	x:DataType="vm:IconTintColorBehaviorViewModel"
	x:TypeArguments="vm:IconTintColorBehaviorViewModel"
	x:Name="this">
	<ScrollView>
        <VerticalStackLayout
            Padding="30,0"
            Spacing="25"
            VerticalOptions="Center">
            
            <Grid
			Padding="20" 
			ColumnDefinitions="*,*"
			RowDefinitions="Auto, Auto, 60, 60, 60, Auto"
			RowSpacing="20">

			<Label
				Margin="0,0,0,20"
				Text="With the IconTintColorBehavior you set the tint color of an image. This behavior works on the Image and ImageButton controls and is implemented for the Android, iOS, maccatalyst and Windows platforms."
				HorizontalTextAlignment="Center"
				Grid.ColumnSpan="2"/>

				<Label
					Grid.Row="1"
					FontAttributes="Bold"
					FontSize="18"
					HorizontalTextAlignment="Center"
					Text="Image control" />

				<Image Grid.Row="2" Source="shield.png" />

				<Image Grid.Row="3" Source="shield.png">
					<Image.Behaviors>
						<mct:IconTintColorBehavior TintColor="Red" />
					</Image.Behaviors>
				</Image>

				<Image Grid.Row="4" Source="shield.png">
					<Image.Behaviors>
						<mct:IconTintColorBehavior TintColor="Green" />
					</Image.Behaviors>
				</Image>

				<Label
					Grid.Column="1"
					Grid.Row="1"
					FontAttributes="Bold"
					FontSize="18"
					HorizontalTextAlignment="Center"
					Text="ImageButton control" />

				<ImageButton
					Grid.Row="2"
					Grid.Column="1"
					Source="shield.png" />

				<ImageButton
					Grid.Row="3"
					Grid.Column="1"
					Source="shield.png">
					<ImageButton.Behaviors>
						<mct:IconTintColorBehavior TintColor="Red" />
					</ImageButton.Behaviors>
				</ImageButton>

				<ImageButton
					Grid.Row="4"
					Grid.Column="1"
					Command="{Binding ChangeCommand}"
					Source="shield.png">
					<ImageButton.Behaviors>
						<mct:IconTintColorBehavior TintColor="{Binding Source={x:Reference this}, Path=BindingContext.ChangedColor}" />
					</ImageButton.Behaviors>
				</ImageButton>

			<Label
				Grid.Row="5"
				Grid.Column="1"
				Text="The ImageButton above uses Binding in order to change the TintColor. Click it!"/>
			</Grid>
      

            <Image
                HeightRequest="200"
                HorizontalOptions="Center"
                SemanticProperties.Description="Cute dot net bot waving hi to you!"
                Source="dotnet_bot.png">
                <Image.Behaviors>
                    <mct:IconTintColorBehavior TintColor="HotPink" />
                </Image.Behaviors>
            </Image>

            <Label
                FontSize="32"
                HorizontalOptions="Center"
                SemanticProperties.HeadingLevel="Level1"
                Text="Hello, World!" />

            <Label
                FontSize="18"
                HorizontalOptions="Center"
                SemanticProperties.Description="Welcome to dot net Multi platform App U I"
                SemanticProperties.HeadingLevel="Level2"
                Text="Welcome to .NET Multi-platform App UI" />

            <Button
                x:Name="CounterBtn"
                HorizontalOptions="Center"
                SemanticProperties.Hint="Counts the number of times you click"
                Text="Click me" />

        </VerticalStackLayout>
    </ScrollView>
</pages:BasePage>

image

@brminnick
Copy link
Collaborator

Thanks @kperdlich!!

@brminnick brminnick enabled auto-merge (squash) July 5, 2023 17:01
@brminnick brminnick merged commit bb31c87 into CommunityToolkit:main Jul 5, 2023
7 checks passed
@stbau04
Copy link

stbau04 commented Jul 27, 2023

FYI
In the current Version (5.2.0) there is a bug in the IconTintColorBehavior (if using on windows):
If you are reusing a page (navigating back and forth again without reinstancing it; e.g. using DI and registering the screens as singleton), the icon tint will be removed while closing the page (RemoveTintColor is called in OnDetachedFrom which is called on closing) and won't be reapplied on opening it again (it would be applied, if the ImageOpened event would be thrown again, but this event is not thrown, because the image is already loaded). This causes, that the image uses it's color and just gets back to the default it was before applying the tint.

In the commit text for this bugfix, there is a change "* Load image immediately when available" which checks, if the image is already loaded, and if so, the tint will be applied. This does not only cause the image to be colored immeadiately, this fixes a bug, that the image wouldnt be at all.

So if you are having this issue (and you are testing on windows), the problem is already solved; it is just nowhere documented, bc i think the problem wasn't found at all

Thanks a lot!

@brminnick
Copy link
Collaborator

brminnick commented Jul 27, 2023

Could you please open a new Issue to document this? It'd be great if you could provide a PR to fix it as well!

If you are reusing a page (navigating back and forth again without reinstancing [sic] it; e.g. using DI and registering the screens as singleton)

Just FYI - it is not recommended to register ContentPages and ViewModels as singletons. Best practice is to register them as Transient.

@stbau04
Copy link

stbau04 commented Jul 27, 2023

It's not necessary to provide a PR, the bug is already fixed, but it isn't mentioned anywhere that it is fixed/that it existed. I guess it was fixed unintentionally

Should i still create an issue to document it?

@brminnick
Copy link
Collaborator

In the current Version (5.2.0) there is a bug in the IconTintColorBehavior (if using on windows)

It's not necessary to provide a PR, the bug is already fixed

Apologies, I'm confused. Is there a bug in v5.2.0?

@stbau04
Copy link

stbau04 commented Jul 28, 2023

There is a bug in v5.2.0, but is already fixed on the current master branch. It just isn't documented/mentioned anywhere that the bug exists/existed

Sorry, i think i didn't say that clearly in my description

@TravisRo
Copy link

TravisRo commented Aug 3, 2023

This is not fixed in the main branch as of 08/2/2023.

Steps to reproduce:

  1. Use Visual Studio MAUI application template to create a new project.
  2. Clone MAUI Toolkit main branch and add project references.
  3. Add the IconTintColorBehavior to the first image in MainPage.xaml
  4. Run app (Windows Machine) and the image is not displayed at all!

IconTintColorBehavior_Offset_Incorrect

            <Image
                Source="dotnet_bot.png"
                SemanticProperties.Description="Cute dot net bot waving hi to you!"
                HeightRequest="200"
                HorizontalOptions="Center">
                <Image.Behaviors>
                    <toolkit:IconTintColorBehavior TintColor="Aqua"/>
                </Image.Behaviors>
            </Image>

If you also set WidthRequest to 200, you get this:

IconTintColorBehavior_Offset_Incorrect2

            <Image
                Source="dotnet_bot.png"
                SemanticProperties.Description="Cute dot net bot waving hi to you!"
                HeightRequest="200"
                WidthRequest="200"
                HorizontalOptions="Center">
                <Image.Behaviors>
                    <toolkit:IconTintColorBehavior TintColor="Aqua"/>
                </Image.Behaviors>
            </Image>

I have fixed this in my fork here:
https://github.com/TravisRo/Maui

Should a submit a pull request?

@brminnick
Copy link
Collaborator

Should a submit a pull request?

Yes! Please open an Issue first to document the bug, demonstrating it with this reproduction sample.

Then submit the fix in a PR and we'll merge it in!

@Mataboge
Copy link

Mataboge commented Oct 3, 2023

I am using 6.0.0, but the offset bug problem still exists on Windows, the workaround was to use ImageButton instead of Image. I will attach a sample to reproduce soon

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.

[BUG] IconTintColorBehavior misaligns tinted image on Windows
5 participants