Skip to content

Color: add overload to accept hex string#4170

Merged
swharden merged 8 commits intoScottPlot:mainfrom
aespitia:feature/aespitia/color-new-overload
Aug 26, 2024
Merged

Color: add overload to accept hex string#4170
swharden merged 8 commits intoScottPlot:mainfrom
aespitia:feature/aespitia/color-new-overload

Conversation

@aespitia
Copy link
Contributor

@aespitia aespitia commented Aug 21, 2024

Small update to add an new Color overload described in this ticket

#4165

sample usage:

var clr = new ScottPlot.Color("#FF0000");

@aespitia aespitia marked this pull request as ready for review August 21, 2024 21:20
@aespitia aespitia changed the title adding new overload adding new overload for Color to use hex string Aug 21, 2024
@KroMignon
Copy link
Contributor

KroMignon commented Aug 22, 2024

@aespitia IMHO, this is not the best way to create this constructor. Create a first instance of the class to copy it to create the class itself is not very clean for me.

Perhaps you could made a small change to this PR:

  • split static method Color.FromHex() into 2 parts:
    • create a private static method Hex2Uint to convert hex string into an unsigned int value
    • create a Color constructor from unsigned int Color (uint value)
  • call this constructor from hex string constructor public Color(string hex):this(Hex2Uint(hex)) {}
  • also use this constructor in static method Color.FromHex() ==> public static Color FromHex(string hex) => new Color(hex);

my 2 cts.

@aespitia
Copy link
Contributor Author

@aespitia IMHO, this is not the best way to create this constructor. Create a first instance of the class to copy it to create the class itself is not very clean for me.

Perhaps you could made a small change to this PR:

* split static method `Color.FromHex()` into 2 parts:
  
  * create a private static method `Hex2Uint` to convert hex string into an unsigned int value
  * create a `Color` constructor from unsigned int `Color (uint value)`

* call this constructor from hex string constructor `public Color(string hex):this(Hex2Uint(hex)) {}`

* also use this constructor in static method `Color.FromHex()` ==> `public static Color FromHex(string hex) => new Color(hex);`

my 2 cts.

i like it, can make updates

@aespitia
Copy link
Contributor Author

@KroMignon updated PR with your suggestions, lmk if you had other thoughts.

@swharden
Copy link
Member

This looks great! Thanks @aespitia @KroMignon and @kebox7 - I look forward to using this myself after the next release! 🚀

@swharden swharden enabled auto-merge (squash) August 26, 2024 01:48
@swharden swharden changed the title adding new overload for Color to use hex string Color: add overload to accept hex string Aug 26, 2024
@swharden swharden merged commit d660eb0 into ScottPlot:main Aug 26, 2024
@swharden swharden linked an issue Aug 26, 2024 that may be closed by this pull request
@aespitia aespitia deleted the feature/aespitia/color-new-overload branch August 27, 2024 14:52
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.

Color.New() overload for accepting hex string

4 participants