-
Notifications
You must be signed in to change notification settings - Fork 82
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 a PlainColorObject type for functions that return color objects #291
Conversation
Hey, thanks for the contribution! I've requested review from some more TS-savvy folks than me. Meanwhile, you could rebase this PR, as it now shows changes from your other, merged, PR. |
e79e03c
to
d2b72d8
Compare
I managed to remove the extra commit from my original pull request but I think I ended up creating a different mess. I have no idea what I'm doing when it comes to git commands. For now I'll just wait for the review and if everything is ok we can figure out what to do next. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this! I think there may be a few more places where we can replace ColorObject
with PlainColorObject
(e.g. Display.color
in display.d.ts
). I haven't looked through all of them, but I expect there might be other places too?
I tried to find all of the functions that could be switched but it's quite possible I missed a few. The |
I meant the return value |
The ColorObject type has optional properties and doesn't accurately reflect the color object type returned by functions such as getColor() which ensures that the object returned has space, coords and alpha properties that are not undefined. A new interface called PlainColorObject has been introduced to represent fully formed color objects. The Color class implements the PlainColorObject interface so the Color class was removed from list of types in the ColorTypes type.
c3d87bf
to
8421e62
Compare
Sorry, misread your comment. You're correct and I've made the appropriate change to the |
The function represented by Range should return a Color, not a number. Fixes #298.
Just realized this PR had not been merged because it was referenced by another issue. Sorry @lloydk! FWIW @jgerigmeyer @MysteryBlokHed if I tag you for review, I fully trust you to merge the PR when/if ready. 😊 |
The
ColorObject
type has optional properties and doesn't accurately reflect the color object type returned by functions such asgetColor()
which ensures that the object returned has space, coords and alpha properties that are not undefined.A new interface called
PlainColorObject
has been introduced to represent fully formed color objects.The
Color
class implements thePlainColorObject
interface so theColor
class was removed from list of types in theColorTypes
type.This PR also fixes the return type for functions in the interpolation module that were returning the incorrect type.