fix: make format* take s/l/t on the 0–1 scale - #94
Merged
Conversation
The format* writers took s/l/t on a 0-100 percentage scale while every producer in the library — resolve(), variantToOkhsl, srgbToOkhsl, oklabToOkhsl, okhslToSrgb — returns them on 0-1. Composing the two was off by 100x and failed silently, since 0.7 is a legal percentage: the result was a valid CSS string naming a near-black color. The writers now take the factors and scale to percent themselves, so the library speaks one scale end to end. A value above 1 can only be old percentage-scale input, so warn once per writer instead of emitting a plausible wrong color. Also documents okhslToSrgb's scale, the one converter that stated no units. Closes #93 Claude-Session: https://claude.ai/code/session_01JgZkbB2yZjca1WMVpsHjTq
Contributor
📦 Snapshot releasePublished |
- The api.md composition snippet spread `Object.values(variantToOkhsl(v))` into `formatRgb`, which does not typecheck (TS2556) and would bind a future `alpha` to the `pastel` parameter. Destructure instead, in the doc, the `variantToOkhsl` JSDoc, and the test that asserted it. - "the scale every converter returns" overstated it: `toTone` / `fromTone` still speak the 0-100 tone axis the authoring API takes. Say so where the claim is made, and mark the scale on both rows. - Fix the `variantToOkhsl` row, which claimed an `alpha` it never returns. - Drop the copied 32-entry cap on the scale-warning cache: it is keyed by writer name, so it tops out at five entries and the branch was dead. Claude-Session: https://claude.ai/code/session_01JgZkbB2yZjca1WMVpsHjTq
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #93.
Takes suggestion 1 from the issue — the stated first preference: make the writers accept 0–1 so the library is self-consistent — plus suggestion 4's doc fixes.
What changed
formatOkhsl/formatOkhst/formatRgb/formatHsl/formatOklchnow reads/l/ton the 0–1 factor scale every producer in the library returns (resolve(),variantToOkhsl,srgbToOkhsl,oklabToOkhsl,okhslToSrgb). The percentages are an output detail: the writers scale by 100 themselves where the CSS syntax asks for one. So the compositionvariantToOkhsl's doc points at is now correct with nothing in between:and it round-trips:
glaze.color(formatOkhst(v.h, v.s, v.t))comes back with the sames/t.The old call shape no longer fails silently either. A value above 1 cannot be a factor, so it can only be pre-2.0 percentage-scale input — the writers
console.warnonce per writer (deduped for the process) and emit an obviously-broken7040.68%rather than a plausible near-black.Docs, per suggestion 4:
okhslToSrgbnow states its scale (the one converter that stated no units), andvariantToOkhsl's comment spells out thats/lcome back on the same 0–1 scale the writers take, so it no longer points at a bug.Compatibility
Breaking for direct callers of the writers, so the changeset is
major. The fix is to drop the* 100:Nothing else moves —
hwas always 0–360,alphaalways 0–1 — and no export method changes its output:css()/tokens()/tasty()/json()/dtcg()/tailwind()/glaze.format()were compensating internally, and now just stop.I diffed the full export surface (4 theme seeds × every format × css/json/tasty/tailwind/dtcg/splitHue, before vs. after) to confirm. One string differs anywhere: a fully-desaturated shadow emits
hsl(0 0% 18.07% / 0.3554)where it used to emithsl(340 0% 18.07% / 0.3554). Dropping the redundant×100 ÷100round-trip moves the sRGB channels by ~1.7e-16, which flips which channelformatHslpicks asmaxwhen deriving hue — and at0%saturation the hue term names no color either way. Same color, one fewer float round-trip.If you'd rather ship this as a
minor(these are the advanced-use re-exports, not the main surface), the only edit needed is the bump in.changeset/plain-scales-align.md.Tests
New
src/okhsl-color-math.test.ts(12 tests) covers the scale contract, agreement with the converters each writer wraps, the pastel recompute on the new scale, the percentage-scale guard (warns on 0–100 input, once per writer, silent at the1endpoint), and theresolve()→ writer → parser round-trip from the issue.pnpm typecheck,pnpm lint,pnpm format:check,pnpm build, andpnpm test(386 tests) all pass; the pre-existing 374 are untouched.Generated by Claude Code