color.js
color.js copied to clipboard
color-js
[types] Fix type errors in codebase
Closes #572
Fixes type errors originating from the codebase itself.
In some instances, the logic was changed where it seemed like the type error reflected an actual problem. For example, adding color = Color.get(color) where the signature made it seem as though a function could take ColorTypes, but actually couldn't.
Deploy Preview for colorjs ready!
| Name | Link |
|---|---|
| Latest commit | 365f07f8d25c6062f04efa4303c30c10d33dea9a |
| Latest deploy log | https://app.netlify.com/sites/colorjs/deploys/66956f6820ab7400087d97f9 |
| Deploy Preview | https://deploy-preview-574--colorjs.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
Not 100% sure that the commit I just pushed for Format is entirely correct, so any comments would be appreciated
Not 100% sure that the commit I just pushed for
Formatis entirely correct, so any comments would be appreciated
The Format stuff is complicated, I took a look at it last week and gave up after a few hours.
I'll take a closer look after I take the dog for a walk but some stuff that's missing:
ColorSpace.getFormatshould return theFormatclass not theFormatinterface.- The
Formatclass should have almost every property from theFormatinterface. see https://github.com/color-js/color.js/blob/c0a48bf283f7858cf589e81f5566a485299aa344/src/Format.js#L36
In the Format class I didn't like how the coords property changed from an array of strings to Type[][] so in the PR that I abandoned I renamed Format.coords to coordTypes.
The Format changes look good although I'm not 100% sure about the type for Format.coords. If format.type === "custom" it's [string, string, string] otherwise it's Type[][]. I think it would be better to have two different properties.
If
format.type === "custom"it's[string, string, string]otherwise it'sType[][]. I think it would be better to have two different properties.
Ah, I didn't even notice that. I don't think there's a great way to type it without separating the two properties. We could ask Lea for her thoughts in regards to making a new property, since she wrote the class?
If
format.type === "custom"it's[string, string, string]otherwise it'sType[][]. I think it would be better to have two different properties.Ah, I didn't even notice that. I don't think there's a great way to type it without separating the two properties. We could ask Lea for her thoughts in regards to making a new property, since she wrote the class?
We could just leave it as Type[][] since you probably won't have a coords property for a custom format.
Sorry I haven't worked on this for a while, I've been busier than usual. If anybody else wants to push commits to fix errors then feel free—this branch is on the main repository instead of a fork so permissions shouldn't be an issue.
Sorry I’ve been absent as well, my thesis defense is in a month so I've been directing all my resources towards it. Should this PR block releasing v0.6.0?
Sorry I’ve been absent as well, my thesis defense is in a month so I've been directing all my resources towards it. Should this PR block releasing v0.6.0
At the very least we should merge what we have so far in this PR before releasing v0.6.0. I had to switch my project to this branch because I needed some of the fixes in this PR.
At the very least we should merge what we have so far in this PR before releasing v0.6.0.
I agree. We might as well merge what we have so far and worry about the rest of the errors some other time, since most errors don't actually seem to indicate any logical problems in the code.
@MysteryBlokHed should I just merge this then? It's still marked as draft.