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

Rename light/dark functions as they return boolean value #101

Closed
felixsanz opened this issue Dec 16, 2016 · 2 comments
Closed

Rename light/dark functions as they return boolean value #101

felixsanz opened this issue Dec 16, 2016 · 2 comments

Comments

@felixsanz
Copy link
Contributor

This functions:

color.light();  // true
color.dark();   // false

They return a boolean, so the API should be color.isLight() and color.isDark(). :(

Being just light() it seems like it returns a light version of the color, or something like that.

It is too late to push it into 1.x? Maybe add this methods and move light() and dark() as an alias?

@felixsanz
Copy link
Contributor Author

I've created PR #102

@felixsanz felixsanz changed the title Too late to rename light/dark functions? Rename light/dark functions as they return boolean value Dec 16, 2016
@Qix-
Copy link
Owner

Qix- commented Jan 25, 2017

So I agree with the spirit of these comments, though your solution I feel is just another bandaid patch (especially the PR).

There's this creeping problem of the library outgrowing its flat API style now that many different color models are included.

There needs to be some more thought put into the overall API and how we can silo things out a bit more to include the vast amounts of color operations that can occur between all of the different color models. This also includes the fact I want to support color space manipulation in the future, as well as support color schemes (e.g. specify multiple colors and perform batch operations on them).

The API is quickly growing a bit out of control, so the light/dark descriptiveness problems will be resolved when the solution to the above mentioned problem is found.

@Qix- Qix- closed this as completed in 792a69f Jan 25, 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

No branches or pull requests

2 participants