Skip to content
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

Add Orange color to Color #290

Merged
merged 4 commits into from Aug 11, 2016

Conversation

rookiejava
Copy link
Collaborator

@rookiejava rookiejava commented Aug 10, 2016

Description of Change

Add Orange color to Xamarin.Forms.Color.

Bugs Fixed

None

API Changes

Added:

  • Color.Orange // The color that is represented by the RGB value #FFA500. (255, 165, 0)

Behavioral Changes

None

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

@dnfclas
Copy link

dnfclas commented Aug 10, 2016

Hi @rookiejava, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, DNFBOT;

@StephaneDelcroix
Copy link
Member

I'm expecting to see a veeeeery long discussion on this PR. Should I start ?

@StephaneDelcroix
Copy link
Member

So I start...

FFA500 is yellowish. According to wikipedia, Orange is FF7F00. and this color profiled monitor tend to agree...

There's also a ColorTypeConverter that speeds up xaml parsing of Colors. Orange should be in there too.
Also, it misses the documentation. I'd define it as:

ORANGE. This is Orange, according to me. Some says it's a bit yellowish, but who cares?

@tpetrina
Copy link
Contributor

Well, this is Orange color that we already have in .NET IIRC.

@ghuntley
Copy link
Contributor

🍿

On Wed, 10 Aug 2016, 7:39 PM Stephane Delcroix notifications@github.com
wrote:

So I start...

FFA500 is yellowish. According to wikipedia, Orange is FF7F00. and this
color profiled monitor tend to agree...

There's also a ColorTypeConverter that speeds up xaml parsing of Colors.
Orange should be in there too.
Also, it misses the documentation. I'd define it as:

ORANGE. This is Orange, according to me. Some says it's a bit yellowish,
but who cares?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#290 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAHxeRoUyNv_Ag1SAKS1WvLagd3DY8Noks5qeZxUgaJpZM4Jgz0x
.

@rookiejava
Copy link
Collaborator Author

@StephaneDelcroix Thank you for your time and effort to this matter. Wikipedia uses a normalized set of color coordinate systems in the color Information Box. It means that color coordinate ranges are a matter of designer choice and there is no universal standard range for many color spaces (though some ranges may be prevalent).

In this PR, I referred to Web color table. (All predefined color values in Xamarin.Forms are exactly same with Web color table.)

What is your reference color coordinate system? If you just want to follow Wikipedia, please make new PR to fix current Color.Lime value to #BFFF00 from #00FF00. - https://en.wikipedia.org/wiki/Lime_(color)
Also, please don't forget modify documentation as below:

LIME. This is Lime, according to me. Some says it's a bit reddish, but who cares?

I also thank you for your comment about ColorTypeConverter. I've updated patch.

@adrianknight89
Copy link
Contributor

Can the colors be aligned with .NET Framework colors? In this case, #FFA500 is the correct value.

https://msdn.microsoft.com/en-us/library/system.drawing.color.orange(v=vs.110).aspx

Since Xamarin is very deeply integrated with Microsoft, as a programmer, I'd expect .NET colors.

@rookiejava
Copy link
Collaborator Author

@adrianknight89 Thanks for your comment :-).

@StephaneDelcroix
Copy link
Member

thanks for the fix in the converter, and the doc addition
👍

@rmarinho
Copy link
Member

Hi, can you rebase in a single commit ?

Thanks

@StephaneDelcroix StephaneDelcroix merged commit ba29e59 into xamarin:master Aug 11, 2016
@StephaneDelcroix
Copy link
Member

no need

@StephaneDelcroix
Copy link
Member

this has been merged. Thanks for this @rookiejava. Sorry for the trolling, that was too tempting, I wasn't able to resists 😃

@rookiejava rookiejava deleted the add-color.orange branch August 11, 2016 14:05
jimmgarrido pushed a commit to jimmgarrido/Xamarin.Forms that referenced this pull request Aug 11, 2016
* Add Orange color to Color
@samhouts samhouts added this to the 2.3.3 milestone Jun 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants