From 6e6e23eb61ec71c14d4f7d2ad2419bd82fa31fb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20K=C3=A4s?= Date: Thu, 9 Jan 2020 19:42:57 +0100 Subject: [PATCH 1/2] Accept null context in PaddingConverter. --- .../System/Windows/Forms/PaddingConverter.cs | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/src/System.Windows.Forms.Primitives/src/System/Windows/Forms/PaddingConverter.cs b/src/System.Windows.Forms.Primitives/src/System/Windows/Forms/PaddingConverter.cs index 376ec2283f5..7dbf266269d 100644 --- a/src/System.Windows.Forms.Primitives/src/System/Windows/Forms/PaddingConverter.cs +++ b/src/System.Windows.Forms.Primitives/src/System/Windows/Forms/PaddingConverter.cs @@ -122,32 +122,35 @@ public override object ConvertTo(ITypeDescriptorContext context, CultureInfo cul public override object CreateInstance(ITypeDescriptorContext context, IDictionary propertyValues) { - if (context == null) - { - throw new ArgumentNullException(nameof(context)); - } if (propertyValues == null) { throw new ArgumentNullException(nameof(propertyValues)); } - Padding original = (Padding)context.PropertyDescriptor.GetValue(context.Instance); + var original = context?.PropertyDescriptor?.GetValue(context.Instance); try { - int all = (int)propertyValues[nameof(Padding.All)]; - if (original.All != all) + // When incrementally changing an existing Padding instance e.g. through a PropertyGrid + // the expected behavior is that a change of Padding.All will override the now outdated + // other properties of the original padding. + // + // If we are not incrementally changing an existing instance (i.e. have no original value) + // then we can just select the individual components passed in the full set of properties + // and the Padding constructor will determine whether they are all the same or not. + + if (original is Padding originalPadding) { - return new Padding(all); - } - else - { - return new Padding( - (int)propertyValues[nameof(Padding.Left)], - (int)propertyValues[nameof(Padding.Top)], - (int)propertyValues[nameof(Padding.Right)], - (int)propertyValues[nameof(Padding.Bottom)] - ); + int all = (int)propertyValues[nameof(Padding.All)]; + if (originalPadding.All != all) + return new Padding(all); } + + return new Padding( + (int)propertyValues[nameof(Padding.Left)], + (int)propertyValues[nameof(Padding.Top)], + (int)propertyValues[nameof(Padding.Right)], + (int)propertyValues[nameof(Padding.Bottom)] + ); } catch (InvalidCastException invalidCast) { From f2c42794eac3975b62e53aab57b9cbeb96864c6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20K=C3=A4s?= Date: Fri, 17 Jan 2020 14:18:09 +0100 Subject: [PATCH 2/2] Update tests to match expectations --- .../Windows/Forms/PaddingConverterTests.cs | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/PaddingConverterTests.cs b/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/PaddingConverterTests.cs index 5afa46e9937..ec49c73d04a 100644 --- a/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/PaddingConverterTests.cs +++ b/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/PaddingConverterTests.cs @@ -157,10 +157,21 @@ public void PaddingConverter_CreateInstance_ValidPropertyValuesAll_ReturnsExpect } [Fact] - public void PaddingConverter_CreateInstance_NullContext_ThrowsArgumentNullException() + public void PaddingConverter_CreateInstance_ValidPropertyValuesNullContext_ReturnsExpected() { var converter = new PaddingConverter(); - Assert.Throws("context", () => converter.CreateInstance(null, new Dictionary())); + Padding expected = new Padding(1, 2, 3, 4); + Padding padding = Assert.IsType(converter.CreateInstance( + null, new Dictionary + { + {nameof(Padding.All), expected.All}, + {nameof(Padding.Left), expected.Left}, + {nameof(Padding.Top), expected.Top}, + {nameof(Padding.Right), expected.Right}, + {nameof(Padding.Bottom), expected.Bottom} + }) + ); + Assert.Equal(expected, padding); } [Fact] @@ -355,7 +366,7 @@ public void PaddingConverter_CreateInstance_InvalidPropertyValueType_ThrowsArgum } [Fact] - public void PaddingConverter_CreateInstance_InvalidInstanceType_ThrowsInvalidCastException() + public void PaddingConverter_CreateInstance_UnknownInstanceType_ReturnsExpected() { var converter = new PaddingConverter(); var mockContext = new Mock(MockBehavior.Strict); @@ -374,7 +385,10 @@ public void PaddingConverter_CreateInstance_InvalidInstanceType_ThrowsInvalidCas {nameof(Padding.Right), 3}, {nameof(Padding.Bottom), 4}, }; - Assert.Throws(() => converter.CreateInstance(mockContext.Object, propertyValues)); + + Padding expected = new Padding(2, 2, 3, 4); + Padding padding = Assert.IsType(converter.CreateInstance(mockContext.Object, propertyValues)); + Assert.Equal(expected, padding); } [Fact]