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

[UWP] Implementation of Switch.OnColor #4883

Merged
merged 5 commits into from Jan 17, 2019
Merged

[UWP] Implementation of Switch.OnColor #4883

merged 5 commits into from Jan 17, 2019

Conversation

jfversluis
Copy link
Member

@jfversluis jfversluis commented Jan 2, 2019

Description of Change

Implements the Switch.OnColor for UWP which was reported as a bug to be not working

Issues Resolved

API Changes

None

Platforms Affected

  • UWP

Behavioral/Visual Changes

Switch.OnColor now actually works on UWP!

Before/After Screenshots

After:

screenshot 2019-01-02 15 38 41

Testing Procedure

Added a case for this in the Switch Gallery app

untitled

PR Checklist

  • Has automated tests
  • Rebased on top of the target branch at time of PR
  • Changes adhere to coding standard

@jfversluis jfversluis changed the title Implemented Switch.OnColor on UWP [UWP] Implementation of Switch.OnColor Jan 2, 2019
if (_originalOnColor == null)
_originalOnColor = frame.Value;

if (Element.IsSet(Switch.OnColorProperty) && !Element.OnColor.IsDefault)
Copy link
Member Author

Choose a reason for hiding this comment

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

The additional IsDefault check is needed (unfortunately) since just using IsSet when setting Color.Default as a value, would still return true causing the control to disappear

Copy link
Member

Choose a reason for hiding this comment

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

the color.IsDefault is probably enough

Copy link
Member Author

Choose a reason for hiding this comment

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

Then I'm confused. In another place, I had to use IsSet. What is the use-case for one or the other?

private void Control_Loaded(object sender, RoutedEventArgs e)
{
UpdateOnColor();
Control.Loaded -= Control_Loaded;
Copy link
Member

Choose a reason for hiding this comment

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

not sure this is required

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean using the Loaded event? I see in the SwitchCell (https://github.com/xamarin/Xamarin.Forms/blob/master/Xamarin.Forms.Platform.UAP/CellControl.cs#L106) that it happens in the measure override. Would you rather have it there?

Xamarin.Forms.Platform.UAP/SwitchRenderer.cs Outdated Show resolved Hide resolved
if (_originalOnColor == null)
_originalOnColor = frame.Value;

if (Element.IsSet(Switch.OnColorProperty) && !Element.OnColor.IsDefault)
Copy link
Member

Choose a reason for hiding this comment

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

the color.IsDefault is probably enough

Xamarin.Forms.Platform.UAP/SwitchRenderer.cs Outdated Show resolved Hide resolved
Xamarin.Forms.Platform.UAP/SwitchRenderer.cs Show resolved Hide resolved
Xamarin.Forms.Platform.UAP/SwitchRenderer.cs Outdated Show resolved Hide resolved
Xamarin.Forms.Platform.UAP/SwitchRenderer.cs Outdated Show resolved Hide resolved
@jfversluis
Copy link
Member Author

Seen the comments, need to find some time to process them... Will do that ASAP! Thanks!

@samhouts
Copy link
Member

@jfversluis Do you have more changes pending, or is this ready for re-review? Thanks!

@jfversluis
Copy link
Member Author

Removed hardcoded opacity, now also taken from the original color. Ready for re-review now!

Xamarin.Forms.Platform.UAP/SwitchRenderer.cs Outdated Show resolved Hide resolved
Xamarin.Forms.Platform.UAP/SwitchRenderer.cs Outdated Show resolved Hide resolved
paymicro and others added 2 commits January 15, 2019 12:52
Co-Authored-By: jfversluis <github@geraldversluis.nl>
Co-Authored-By: jfversluis <github@geraldversluis.nl>
@samhouts samhouts merged commit b7a3adf into xamarin:master Jan 17, 2019
v3.6.0 automation moved this from In Review to Done Jan 17, 2019
@jfversluis jfversluis deleted the bug/4460-uwp-switch-oncolor branch January 17, 2019 07:33
PureWeen pushed a commit that referenced this pull request Jan 24, 2019
* Implemented Switch.OnColor on UWP

* Processed first feedback

* Removed hardcoded opacity

* Update Xamarin.Forms.Platform.UAP/SwitchRenderer.cs

Co-Authored-By: jfversluis <github@geraldversluis.nl>

* Update Xamarin.Forms.Platform.UAP/SwitchRenderer.cs

Co-Authored-By: jfversluis <github@geraldversluis.nl>
andreinitescu pushed a commit to andreinitescu/Xamarin.Forms that referenced this pull request Jan 29, 2019
* Implemented Switch.OnColor on UWP

* Processed first feedback

* Removed hardcoded opacity

* Update Xamarin.Forms.Platform.UAP/SwitchRenderer.cs

Co-Authored-By: jfversluis <github@geraldversluis.nl>

* Update Xamarin.Forms.Platform.UAP/SwitchRenderer.cs

Co-Authored-By: jfversluis <github@geraldversluis.nl>
@samhouts samhouts added this to Done in v4.0.0 Feb 2, 2019
@samhouts samhouts removed this from Done in v3.6.0 Feb 2, 2019
@samhouts samhouts modified the milestones: 4.0.0, 3.6.0 Feb 6, 2019
}

protected override bool PreventGestureBubbling { get; set; } = true;

void OnControlLoaded(object sender, RoutedEventArgs e)
Copy link

@yrest yrest Mar 20, 2019

Choose a reason for hiding this comment

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

Hi guys, just wondering if somebody can help me to figure out what am I doing wrong getting below exception after update to 3.6.0.264807 published Tuesday, March 19, 2019?

image

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that Control.GetChildren<Windows.UI.Xaml.Controls.Grid>().FirstOrDefault() returns null for some reason.

Copy link

Choose a reason for hiding this comment

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

@paymicro thanks for prompt reply! This is a piece of xaml I'm using (first Switch is used to activate the second)
< StackLayout Orientation="Horizontal" >
< Switch x:Name="FirstSwitch" IsToggled="{Binding FirstToggled}" / >
< /StackLayout >
< StackLayout Orientation="Horizontal" IsVisible="{Binding FirstToggled}" >
< Switch x:Name="SecondSwitch" IsToggled="{Binding SecondToggled}" / >
< /StackLayout >

It seems if I use the same variable, used as IsToggled binding, somewhere else, in this case in the IsVisible binding, it breaks...
Have just created brand new XF App1, updated from nuget to the latest version from yesterday, added same xaml and it crashes.
Actually only rolling back to 3.5.0.169047 works for me, for the two 3.6 in nuget it seems to crash for such cases.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
v3.6.0
  
Done
Development

Successfully merging this pull request may close these issues.

[UWP] Switch color is not set
6 participants