Skip to content
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

[WIP] [DRAFT] Typescript conversion + misc changes #17

Open
wants to merge 44 commits into
base: main
Choose a base branch
from

Conversation

rvion
Copy link

@rvion rvion commented Jun 16, 2024

I'll update the readme with soon, opening the PR in the meantime so progress is public.

Temporary summary: I'm building a whole bunch of stuff, including a component library + a design system where everything is dynamically themed based on its surroundings / context. I'm planning to use apcach heavilly.

Background: I discovered OKLCH a month ago => then learnt more about colors => then built some custom naive contrast algorithm => then upgraded my component system to be fully dynamically themed => then struggled with some contrast not always beeing good => then discovered WCAG, APCA => then decided to add some code to find colors automatically => then discovered your project

Planned work: typescript conversion, possibly some perf tweaking, some caching utilities, some more utilities, more tests, etc.

If you want to merge this branch some day, I'm happy to adapt to your preferences

2024-06-16_09-15-23.mp4

@rvion
Copy link
Author

rvion commented Jun 16, 2024

@antiflasher point of attention 1:

function signOf(number: number): 1 | -1 {
  return number >= 0 ? 1 : -1;
  // 2024-06-16: this seems to be dangerous (divide by 0?)
  // return number / Math.abs(number);
}

@rvion
Copy link
Author

rvion commented Jun 16, 2024

@antiflasher point of attention 2:

function apcach(
  //
  contrast: number,
  chroma: number,
  hue?: Maybe<number | string>,
  alpha: number = 100,
  colorSpace: ColorSpace = "p3"
) {
  // 💬 2024-06-16 rvion:
  // | checking if something is either null or undefined should be done
  // | in one go by doing `hue == null` (two equal sign).
  // | (almost the only case when one want to use `==` instead of `===`).

  // 💬 2024-06-16 rvion:
  // | not sure if parseFloat is guaranteed to accept number
  // | probably better to only pass strings to it

  // Check for hue
  hue = hue == null ? 0 : typeof hue === "number" ? hue : parseFloat(hue);

@rvion
Copy link
Author

rvion commented Jun 16, 2024

@antiflasher would splitting the code into multiple file be ok with you ? (while still having the final build a single standalone js file as it is now)

edit: I went for a split; not sure I can extend /document things much without splitting things a bit

@rvion
Copy link
Author

rvion commented Jun 16, 2024

I'm also inlining documentation for easy access without relying on the readme

/**
 * The apcach format can be restored from color in CSS format
 * using the function cssToApcach():
 */
function cssToApcach(
  /** color in CSS format that you want to convert to apcach format */
  color: string,

  /** comparing color
   * if it's on the background position: { bg : comparingColor }
   * if it's in the foreground position: { fg : comparingColor }
   *
   * (supported formats: oklch, oklab, display-p3, lch, lab, hex, rgb, hsl, p3)
   */
  antagonist: { bg?: string; fg?: string },
  colorSpace: ColorSpace = "p3",
  contrastModel: ContrastModel = "apca"
) {
  if (color == null) throw new Error("Color is unde

@rvion
Copy link
Author

rvion commented Jun 16, 2024

I'll probably also add a class for Apcach, so I can scope the setChroma, setHue, and other methods

@rvion
Copy link
Author

rvion commented Jun 16, 2024

So far so good ! I'm not done splitting index.ts yet, but it's progressing quite well

@rvion
Copy link
Author

rvion commented Jun 17, 2024

okay, since I'm about to add some more code, like utilities to make palettes or new class wrapper with both mutable and immutable methods (clone), I'll also bring some prettier config, to keep formatting consistent.

I'll bring something I personally like, since I anyway plan to support my fork, but happy to find some common ground if you want this merge that some day

{
    "singleQuote": true,
    "semi": false,
    "trailingComma": "all",
    "printWidth": 110,
    "jsxSingleQuote": true,
    "tabWidth": 4
}

@rvion
Copy link
Author

rvion commented Jun 17, 2024

okay, there are some unnecessary conversions going on between string format and parsed culori formats; I'll probaly rewrite things a bit more deeply than initially planed, so things get normalized earlier

@rvion
Copy link
Author

rvion commented Jun 17, 2024

I've converted tests to bun:test no dependency when bun locally installed, better runtime, better CLI, and work with typescript directly (as simple as can be)

@rvion
Copy link
Author

rvion commented Jun 17, 2024

new build with esbuild => non minified / without any cleanup => sill < 25kb, done in 4ms

➜  apcach git:(rvion/typescript-conversion) ✗ esbuild src/index.ts --bundle --outfile=index.js  --external:culori --external:apca-w3 --external:wcag-contrast 

  index.js  22.4kb

⚡ Done in 4ms

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant