Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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 think I want to think a bit further ahead and make these a bit more brief in naming.
We'll map it to the appropriate values in sd-transforms if needed, but I think decimals / precision are the best names for these.
In the end, it comes down to that decimals gets passed to here https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toFixed#digits
They refer to it as "digits" but I think this is a poor name as well, it's specifically the digits after the decimal point so I think "decimals" is the best name.
I think I'm going to plan to break this in sd-transforms next major to also use decimals but for now we'll map it to mathFractionDigits there.
As for precision, I feel like we should have a single property. They both define the amount of digits after decimal point for number values. Having a different amount for math expressions in color definitions vs the output color numbers after conversion doesn't make much sense to me, I think they would always be the same.
Uh oh!
There was an error while loading. Please reload this page.
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.
Just be aware that
mathFractionDigits
currently define how precise the calculations are done, andprecision
right now sets how many decimals the color values have in the output files. I am open for renaming, but I fear your suggestion still leaves room for confusion!?How about renaming:
mathFractionDigits
-->calcPrecision
precision
-->colorDecimals
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.
Let's discuss this one on Slack maybe, I wanna see if we can actually bring this to a single property by making some (imo sensible) assumptions.
Uh oh!
There was an error while loading. Please reload this page.
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.
The tricky part is that for ColorJS.io precision 3 means a number like 89,4%, so ColorJS.io precision 3 turns out to be a color like rgb(44.3% 59.5% 89.9%). So if you reuse decimals on the ColorJS.io precision, this is misleading, because for a color like rgb(44.325% 59.545% 89.992%) you would need a color precision of 5.
I mean we could maybe just always add 2 to the decimal setting!?
Uh oh!
There was an error while loading. Please reload this page.
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.
For example, let's assume I have a mix color calculation, that with precision 4 is
#c2c5d0
, for precision 2 this will be#c2c4cf
and for precision 1 this will be#ccc
and for precision 0 this even is#fff
, that's how this works in ColorJS.io.So this makes quite a big difference for colors, maybe having bigger impact, than for other spacing calculations?