Skip to content

feat: add optional color modify mathFractionDigits and color precision properties #49

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

Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/types/Modifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ interface Modifier<T extends string, V> {
interface ColorGenericModifier<T extends ColorModifierTypes, V> extends Modifier<T, V> {
space: ColorSpaceTypes;
format: string;
mathFractionDigits?: number;
precision?: number;
Comment on lines +11 to +12
Copy link
Member

@jorenbroekema jorenbroekema Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
mathFractionDigits?: number;
precision?: number;
decimals?: number;

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.

Copy link
Author

@DarioSoller DarioSoller Mar 31, 2025

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, and precision 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

Copy link
Member

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.

Copy link
Author

@DarioSoller DarioSoller Mar 31, 2025

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!?

Copy link
Author

@DarioSoller DarioSoller Mar 31, 2025

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?

}

export type LightenModifier = ColorGenericModifier<ColorModifierTypes.LIGHTEN, string>;
Expand Down