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

merged 6 commits into from
Dec 15, 2020

Conversation

jsuarezruiz
Copy link
Contributor

Description of Change

Fix Background issue in Button CustomRenderer in iOS.

Issues Resolved

API Changes

None

Platforms Affected

  • iOS

Behavioral/Visual Changes

None

Before/After Screenshots

fix12372

Testing Procedure

Launch Core Gallery and navigate to the issue 12372. If you can see the background color of the custom Button below, the test has passed.

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@samhouts samhouts added this to In Review in vNext+1 (5.0.0) Nov 2, 2020
@PureWeen PureWeen added this to the 5.0.0 milestone Nov 5, 2020
@samhouts samhouts added blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. i/regression labels Nov 5, 2020
}

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

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.

@rachelkang rachelkang self-requested a review November 25, 2020 16:42
@rmarinho rmarinho assigned PureWeen and unassigned jsuarezruiz Dec 3, 2020
@PureWeen PureWeen added the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Dec 3, 2020
@jsuarezruiz jsuarezruiz removed the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Dec 4, 2020
@rmarinho rmarinho merged commit 8b10f11 into 5.0.0 Dec 15, 2020
vNext+1 (5.0.0) automation moved this from In Review to Done Dec 15, 2020
@rmarinho rmarinho deleted the fix-12372 branch December 15, 2020 23:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

[Bug] XF 4.8 breaks custom renderer (Button) background color on iOS
5 participants