Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[iOS] Fix Background issue in Button CustomRenderer #12395

Merged
merged 6 commits into from
Dec 15, 2020
Merged

[iOS] Fix Background issue in Button CustomRenderer #12395

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
80000f0
Added repro sample
jsuarezruiz Oct 7, 2020
d9ed727
Updated the sample
jsuarezruiz Oct 7, 2020
28bddfb
Fix the issue
jsuarezruiz Oct 7, 2020
a0c8220
Merge branch '5.0.0' into fix-12372
PureWeen Nov 10, 2020
8ff015a
Merge branch '5.0.0' into fix-12372
jsuarezruiz Nov 24, 2020
14888f4
Improved the fix with another approach
jsuarezruiz Dec 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 89 additions & 0 deletions Xamarin.Forms.ControlGallery.iOS/CustomRenderers/_12372CustomRenderer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
using CoreGraphics;
using System.ComponentModel;
using UIKit;
using Xamarin.Forms;
using Xamarin.Forms.ControlGallery.iOS.CustomRenderers;
using Xamarin.Forms.Controls.Issues;
using Xamarin.Forms.Platform.iOS;

[assembly: ExportRenderer(typeof(Issue12372Button), typeof(_12372CustomRenderer))]
namespace Xamarin.Forms.ControlGallery.iOS.CustomRenderers
{
public class _12372CustomRenderer : ButtonRenderer
{
protected override void OnElementChanged(ElementChangedEventArgs<Button> args)
{
base.OnElementChanged(args);

if (Control != null && Element != null)
{
SetColors();
Control.TitleLabel.LineBreakMode = UILineBreakMode.WordWrap;
Control.TitleLabel.TextAlignment = UITextAlignment.Center;
}
}

protected override void OnElementPropertyChanged(object sender, PropertyChangedEventArgs args)
{
base.OnElementPropertyChanged(sender, args);

if (Control == null || Element == null || args == null)
return;

if (args.PropertyName == nameof(Button.IsEnabled) ||
args.PropertyName == nameof(Button.IsPressed) ||
args.PropertyName == nameof(Button.IsVisible))
{
SetColors();
}
}

private void SetColors()
{
if (Element is Issue12372Button nymblButton)
{
Control.SetTitleColor(nymblButton.NymblTextColor.ToUIColor(), UIControlState.Normal);
Control.SetTitleColor(nymblButton.NymblPressedColor.ToUIColor(), UIControlState.Selected);
Control.SetTitleColor(nymblButton.NymblDisabledTextColor.ToUIColor(), UIControlState.Disabled);

if (nymblButton.IsEnabled && !nymblButton.IsPressed)
{
Control.BackgroundColor = nymblButton.NymblDefaultColor.ToUIColor();
CreateShadow();
}
else if (nymblButton.IsEnabled && nymblButton.IsPressed)
{
Control.BackgroundColor = nymblButton.NymblPressedColor.ToUIColor();
RemoveShadow();
}
else if (!nymblButton.IsEnabled)
{
Control.BackgroundColor = nymblButton.NymblDisabledColor.ToUIColor();
RemoveShadow();
}
else
{
// Any other state?
Control.BackgroundColor = nymblButton.NymblDefaultColor.ToUIColor();
RemoveShadow();
}
}
}

private void CreateShadow()
{
Layer.CornerRadius = 20;
Layer.ShadowRadius = 2;
Layer.ShadowColor = UIColor.Black.CGColor;
Layer.ShadowOffset = new CGSize(4, 4);
Layer.ShadowOpacity = 0.80f;
}

private void RemoveShadow()
{
Layer.CornerRadius = 20;
Layer.ShadowOpacity = 0f;
}
}
}

1 change: 1 addition & 0 deletions Xamarin.Forms.ControlGallery.iOS/Xamarin.Forms.ControlGallery.iOS.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@
<Compile Include="CustomEffects\FooEffect.cs" />
<Compile Include="_10337CustomRenderer.cs" />
<Compile Include="_11132CustomRenderer.cs" />
<Compile Include="CustomRenderers\_12372CustomRenderer.cs" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\Xamarin.Forms.Controls\Xamarin.Forms.Controls.csproj">
Expand Down
149 changes: 149 additions & 0 deletions Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue12372.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
using Xamarin.Forms.CustomAttributes;
using Xamarin.Forms.Internals;

#if UITEST
using Xamarin.Forms.Core.UITests;
using Xamarin.UITest;
using NUnit.Framework;
#endif

namespace Xamarin.Forms.Controls.Issues
{
#if UITEST
[Category(UITestCategories.ManualReview)]
#endif
[Preserve(AllMembers = true)]
[Issue(IssueTracker.Github, 12372, "[Bug] XF 4.8 breaks custom renderer (Button) background color on iOS", PlatformAffected.iOS)]
public class Issue12372 : TestContentPage
{
public Issue12372()
{

}

protected override void Init()
{
Title = "Issue 12372";

var layout = new StackLayout();

var instructions = new Label
{
Padding = 12,
BackgroundColor = Color.Black,
TextColor = Color.White,
Text = "If you can see the background color of the custom Button below, the test has passed."
};

var button = new Issue12372Button
{
NymblDefaultColor = Color.Blue,
NymblPressedColor = Color.Red,
NymblTextColor = Color.White,
Text = "Issue12372"
};

layout.Children.Add(instructions);
layout.Children.Add(button);

Content = layout;
}
}

public class Issue12372Button : Button
{
public readonly static BindableProperty NymblDefaultColorProperty = BindableProperty.Create(
nameof(NymblDefaultColor),
typeof(Color),
typeof(Issue12372Button),
Color.Blue);

public Color NymblDefaultColor
{
get => (Color)GetValue(NymblDefaultColorProperty);
set => SetValue(NymblDefaultColorProperty, value);
}

public readonly static BindableProperty NymblPressedColorProperty = BindableProperty.Create(
nameof(NymblPressedColor),
typeof(Color),
typeof(Issue12372Button),
Color.Blue);

public Color NymblPressedColor
{
get => (Color)GetValue(NymblPressedColorProperty);
set => SetValue(NymblPressedColorProperty, value);
}

public readonly static BindableProperty NymblDisabledColorProperty = BindableProperty.Create(
nameof(NymblDisabledColor),
typeof(Color),
typeof(Issue12372Button),
Color.Blue);

public Color NymblDisabledColor
{
get => (Color)GetValue(NymblDisabledColorProperty);
set => SetValue(NymblDisabledColorProperty, value);
}

public readonly static BindableProperty NymblDisabledTextColorProperty = BindableProperty.Create(
nameof(NymblDisabledTextColor),
typeof(Color),
typeof(Issue12372Button),
Color.LightSlateGray);

public Color NymblDisabledTextColor
{
get => (Color)GetValue(NymblDisabledTextColorProperty);
set => SetValue(NymblDisabledTextColorProperty, value);
}

public readonly static BindableProperty NymblBorderColorProperty = BindableProperty.Create(
nameof(NymblBorderColor),
typeof(Color),
typeof(Issue12372Button),
Color.White);

public Color NymblBorderColor
{
get => (Color)GetValue(NymblBorderColorProperty);
set => SetValue(NymblBorderColorProperty, value);
}

public readonly static BindableProperty NymblBorderWidthProperty = BindableProperty.Create(
nameof(NymblBorderWidth),
typeof(int),
typeof(Issue12372Button));

public int NymblBorderWidth
{
get => (int)GetValue(NymblBorderWidthProperty);
set => SetValue(NymblBorderWidthProperty, value);
}

public readonly static BindableProperty NymblTextColorProperty = BindableProperty.Create(
nameof(NymblTextColor),
typeof(Color),
typeof(Issue12372Button),
Color.White);

public Color NymblTextColor
{
get => (Color)GetValue(NymblTextColorProperty);
set => SetValue(NymblTextColorProperty, value);
}

public readonly static BindableProperty NymblTextAllCapsProperty = BindableProperty.Create(
nameof(NymblTextAllCaps),
typeof(bool),
typeof(Issue12372Button));

public bool NymblTextAllCaps
{
get => (bool)GetValue(NymblTextAllCapsProperty);
set => SetValue(NymblTextAllCapsProperty, value);
}
}
}
1 change: 1 addition & 0 deletions ...ssues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1623,6 +1623,7 @@
<Compile Include="$(MSBuildThisFileDirectory)Issue11081.xaml.cs">
<DependentUpon>Issue11081.xaml</DependentUpon>
</Compile>
<Compile Include="$(MSBuildThisFileDirectory)Issue12372.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue12222.cs" />
</ItemGroup>
<ItemGroup>
Expand Down
13 changes: 6 additions & 7 deletions Xamarin.Forms.Platform.iOS/Renderers/ButtonRenderer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
using System;
using System.ComponentModel;
using System.Diagnostics;
using System.Linq;
using Foundation;
using UIKit;
using Xamarin.Forms.Internals;
using Xamarin.Forms.PlatformConfiguration.iOSSpecific;
using Specifics = Xamarin.Forms.PlatformConfiguration.iOSSpecific;
using SizeF = CoreGraphics.CGSize;
using PreserveAttribute = Foundation.PreserveAttribute;
using CoreGraphics;
Expand All @@ -21,6 +16,7 @@ public class ButtonRenderer : ViewRenderer<Button, UIButton>, IImageVisualElemen
UIColor _buttonTextColorDefaultHighlighted;
UIColor _buttonTextColorDefaultNormal;
bool _useLegacyColorManagement;
bool _useBackgroundBrush;

ButtonLayoutManager _buttonLayoutManager;

Expand Down Expand Up @@ -159,7 +155,7 @@ protected override void SetBackground(Brush brush)
if (Control == null)
return;

UIColor backgroundColor = Element.BackgroundColor == Color.Default ? null : Element.BackgroundColor.ToUIColor();
UIColor backgroundColor = null;

if (!Brush.IsNullOrEmpty(brush))
{
Expand All @@ -170,9 +166,12 @@ protected override void SetBackground(Brush brush)
var backgroundImage = this.GetBackgroundImage(brush);
backgroundColor = backgroundImage != null ? UIColor.FromPatternImage(backgroundImage) : UIColor.Clear;
}

_useBackgroundBrush = true;
}

Control.BackgroundColor = backgroundColor;
if (_useBackgroundBrush)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be using a locally stored _defaultBackgroundColor instead of a bool check?

Basically how SetBackgroundColor is setup?

		protected override void SetBackgroundColor(Color color)
		{
			if (IsElementOrControlEmpty)
				return;
#if __MOBILE__
			if (color == Color.Default)
				Control.BackgroundColor = _defaultColor;
			else
				Control.BackgroundColor = color.ToUIColor();
#else
			Control.Layer.BackgroundColor = color == Color.Default ? _defaultColor : color.ToCGColor();
#endif
		}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we already have a variable with the default color, the new bool variable only indicates if a brush is being used or not. We have two properties Background and BackgroundColor and this variable just indicate us when we can update the background using a color.

Copy link
Contributor

@PureWeen PureWeen Dec 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This still feels a little bit off to me. _useBackgroundBrush can only ever be toggled one direction so once it's true it's always true

So, after you set the Background to something it becomes impossible to use the BackgroundColor. If you set the Background back to "Default" then use the BackgroundColor property it'll just set the Control.BackgroundColor to null

        private void Button_Clicked(object sender, EventArgs e)
        {
           // this code stops working after you've set the Background to a brush. 
            btnMe.Background = SolidColorBrush.Default;
            btnMe.BackgroundColor = Color.Green;
        }

        private void Button_Clicked2(object sender, EventArgs e)
        {
            btnMe.Background = SolidColorBrush.Purple;
        }

Is this expected behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shane, I totally see your feedback and I understand it. I have applied changes with a different approach with more sense.
Now, the custom renderer works and also your case works without using any internal variable etc.

Control.BackgroundColor = backgroundColor;
}

void SetControlPropertiesFromProxy()
Expand Down