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

[C] support more color format in ColorTypeConverter #784

Merged
merged 3 commits into from Mar 1, 2017

Conversation

Projects
None yet
4 participants
@StephaneDelcroix
Member

StephaneDelcroix commented Feb 23, 2017

Description of Change

[C] support more color format in ColorTypeConverter

Bugs Fixed

/

API Changes

/

Behavioral Changes

/

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
case "WhiteSmoke": return Color.WhiteSmoke;
case "Yellow": return Color.Yellow;
case "YellowGreen": return Color.YellowGreen;
switch (color.ToLowerInvariant()) {

This comment has been minimized.

@samhouts

samhouts Feb 23, 2017

Member

I bet this was fun.

@samhouts

samhouts Feb 23, 2017

Member

I bet this was fun.

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Feb 24, 2017

Member

@samhouts I bet it was fun to review, trying to find the one place an uppercase was left, or the one time I mixed gray with blue, accidentally creating the glue color.

@StephaneDelcroix

StephaneDelcroix Feb 24, 2017

Member

@samhouts I bet it was fun to review, trying to find the one place an uppercase was left, or the one time I mixed gray with blue, accidentally creating the glue color.

Show outdated Hide outdated Xamarin.Forms.Core.UnitTests/ColorUnitTests.cs
Assert.AreEqual(Color.Blue.MultiplyAlpha(.8), converter.ConvertFromInvariantString("hsla(240,100%, 50%, .8)"));
Assert.AreEqual(Color.Default, converter.ConvertFromInvariantString("Color.Default"));
Assert.AreEqual(Color.Accent, converter.ConvertFromInvariantString("Accent"));
var hotpink = Color.FromHex("#FF69B4");
Color.Accent = hotpink;
Assert.AreEqual (Color.Accent, converter.ConvertFromInvariantString ("Accent"));
Assert.Throws<InvalidOperationException> (() => converter.ConvertFromInvariantString (""));

This comment has been minimized.

@samhouts

samhouts Feb 23, 2017

Member

Maybe also add some Assert.Throws for parentheses violations (e.g., ColorTypeConverter, L28).

@samhouts

samhouts Feb 23, 2017

Member

Maybe also add some Assert.Throws for parentheses violations (e.g., ColorTypeConverter, L28).

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Feb 24, 2017

Member

I'm still unsure about how this should behave on error. While parsing Xaml, we throw, but while parsing css any-other-markup, we might want to revert to default.

adding a test nonetheless

@StephaneDelcroix

StephaneDelcroix Feb 24, 2017

Member

I'm still unsure about how this should behave on error. While parsing Xaml, we throw, but while parsing css any-other-markup, we might want to revert to default.

adding a test nonetheless

Show outdated Hide outdated Xamarin.Forms.Core.UnitTests/ColorUnitTests.cs
Assert.AreEqual(Color.Blue, converter.ConvertFromInvariantString("rgb(0,0, 255)"));
Assert.AreEqual(Color.Blue, converter.ConvertFromInvariantString("rgb(0,0, 300)"));
Assert.AreEqual(Color.Blue, converter.ConvertFromInvariantString("rgb(0,0, 300)"));
Assert.AreEqual(Color.Blue.MultiplyAlpha(.8), converter.ConvertFromInvariantString("rgba(0%,0%, 100%, .8)"));

This comment has been minimized.

@samhouts

samhouts Feb 23, 2017

Member

This one is failing.. :(

@samhouts

samhouts Feb 23, 2017

Member

This one is failing.. :(

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Feb 24, 2017

Member

WORKSFORME... investigating...

@StephaneDelcroix

StephaneDelcroix Feb 24, 2017

Member

WORKSFORME... investigating...

StephaneDelcroix added some commits Feb 24, 2017

@jassmith jassmith merged commit 8379294 into master Mar 1, 2017

6 checks passed

Android-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 : Tests passed: 353, i…
Details
OSX-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: OSX Debug : Running
Details
Windows-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: Windows Debug : Tests passed: 3743, ignored: 10
Details
iOS10-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10 : Tests passed: 35…
Details
iOS8-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified IOS8 : Tests passed: 351…
Details
iOS9-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS9 : Tests passed: 353…
Details

@StephaneDelcroix StephaneDelcroix deleted the colorConversion branch Mar 3, 2017

@samhouts samhouts added D-15.4 and removed cla-not-required labels Oct 10, 2017

@samhouts samhouts modified the milestones: 2.3.0, 2.3.5 Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment