Make lints have configurable parameters

mcnuttandrew · Jan 16, 2024 · d0fb7bb · d0fb7bb
1 parent c2b0dda
commit d0fb7bb
Show file tree

Hide file tree

Showing 13 changed files with 196 additions and 73 deletions.
diff --git a/README.md b/README.md
@@ -51,13 +51,14 @@ To raise the sliders, you need to click on one of the examples that are displaye
 
 ## TODOS
 
-- [ ] Make keyboard short cut (option+up/down) for the z-direction
+- [ ] Minor: Make keyboard short cut (option+up/down) for the z-direction
 - [ ] Polar stuff
 - [ ] Chore: Extract some common types into a top level location (like types.ts type thing)
 - [?] Rearrange some of the colors in the color area eg make rg on xy and b on z etc
 - [ ] Insert color theory options, eg insert opposing, inserting analogous color, etc, mine from the adobe picker
 - [ ] Labels, tooltips, etc
 - [ ] Nice to have: Rest of basic geometry manipulations: flip (horizontal, vertical), scale, Distribute radially
+- [x] Make lints have configurable parameters
 - [x] Examples: A (default) example showing annotated math stuff
 - [x] Examples: SVG Upload
 - [x] Examples: Vega/Vega-Lite Specification

diff --git a/src/components/ColorScatterPlot.svelte b/src/components/ColorScatterPlot.svelte
@@ -6,12 +6,7 @@
     toColorSpace,
     colorPickerConfig,
   } from "../lib/Color";
-  import {
-    makePosAndSizes,
-    deDup,
-    toggleElement,
-    clampToRange,
-  } from "../lib/utils";
+  import { makePosAndSizes, toggleElement, clampToRange } from "../lib/utils";
   import configStore from "../stores/config-store";
   import { scaleLinear } from "d3-scale";
   import simulate_cvd from "../lib/blindness";
@@ -282,7 +277,7 @@
 
 <!-- svelte-ignore a11y-no-static-element-interactions -->
 <!-- svelte-ignore a11y-no-static-element-interactions -->
-<div class="flex" style="background: {bg.toDisplay()}">
+<div class="flex pb-2" style="background: {bg.toDisplay()}">
   <div class="flex flex-col items-center">
     <span class="text-2xl" style="color: {textColor}">
       {config.title}
@@ -431,7 +426,7 @@
         <svg
           {height}
           width={80 + margin.left + margin.right}
-          class="mt-3"
+          class="mt-1"
           on:mouseleave={stopDrag}
           on:mouseup={stopDrag}
           on:touchend={stopDrag}
@@ -442,14 +437,15 @@
             {textColor}
             {colorSpace}
             {axisColor}
+            {margin}
           />
           <g transform={`translate(${margin.left}, ${margin.top})`}>
             <!-- svelte-ignore a11y-no-static-element-interactions -->
             <rect
               x={0}
-              y={0}
+              y={-20}
               width={80}
-              height={yScale.range()[1]}
+              height={yScale.range()[1] + 40}
               fill="white"
               opacity="0"
               class:cursor-pointer={dragging}

diff --git a/src/components/ColorScatterPlotZGuide.svelte b/src/components/ColorScatterPlotZGuide.svelte
@@ -6,18 +6,24 @@
   export let textColor: string;
   export let colorSpace: string;
   export let axisColor: string;
+  export let margin: {
+    left: number;
+    right: number;
+    top: number;
+    bottom: number;
+  };
 
   $: config = colorPickerConfig[colorSpace];
 
   $: zPoints = {
     top: {
-      y: zScale.range()[0] + 0,
+      y: zScale.range()[0] + 8,
       label: `${config.zChannel.toUpperCase()}: ${zScale
         .domain()[0]
         .toFixed(1)}`,
     },
     bottom: {
-      y: zScale.range()[1] + 15,
+      y: zScale.range()[1] + 30,
       label: zScale.domain()[1].toFixed(1),
     },
   };
@@ -26,9 +32,9 @@
 
 <line
   x1={10}
-  y1={0}
+  y1={margin.top}
   x2={10}
-  y2={plotHeight}
+  y2={plotHeight + margin.top}
   stroke={axisColor}
   stroke-width="1"
 />

diff --git a/src/components/Tooltip.svelte b/src/components/Tooltip.svelte
@@ -37,20 +37,23 @@
       x.addEventListener("click", onClick);
     });
   }
-  $: topString =
-    allowDrag && $configStore.tooltipXY
-      ? $configStore.tooltipXY[1]
-      : boundingBox
-        ? top
-          ? `calc(${boundingBox.y}px + ${top})`
-          : `${boundingBox.y}px`
-        : "0";
-  $: leftString =
-    allowDrag && $configStore.tooltipXY
-      ? $configStore.tooltipXY[0]
-      : boundingBox
-        ? `${boundingBox.x}px`
-        : "0";
+  $: topString = boundingBox
+    ? top
+      ? `calc(${boundingBox.y}px + ${top})`
+      : `${boundingBox.y}px`
+    : "0";
+  $: leftString = boundingBox ? `${boundingBox.x}px` : "0";
+  $: {
+    if (boundingBox.y + 300 > window.screen.height) {
+      topString = `${window.screen.height - 300}px`;
+    }
+  }
+  $: {
+    if (allowDrag && $configStore.tooltipXY) {
+      topString = $configStore.tooltipXY[1];
+      leftString = $configStore.tooltipXY[0];
+    }
+  }
 </script>
 
 {#if tooltipOpen && boundingBox}

diff --git a/src/content-modules/Eval.svelte b/src/content-modules/Eval.svelte
@@ -153,6 +153,16 @@
       </select>
       Palette
     </div>
+    {#if Object.keys($colorStore.currentPal.evalConfig)}
+      <button
+        class={buttonStyle}
+        on:click={() => {
+          colorStore.setCurrentPalEvalConfig({});
+        }}
+      >
+        Restore Defaults
+      </button>
+    {/if}
     <div>Checks</div>
     <div class="overflow-auto h-full max-w-md">
       {#each checks as check}

diff --git a/src/content-modules/contextual-tools/EvalResponse.svelte b/src/content-modules/contextual-tools/EvalResponse.svelte
@@ -33,10 +33,27 @@
     });
   }
   $: evalConfig = $colorStore.currentPal.evalConfig;
+  function updateEvalConfig(
+    checkName: string,
+    value: any,
+    formatter: any = (x) => x
+  ) {
+    let val = value;
+    if (val.target.value) {
+      val = formatter(val.target.value);
+    }
+    colorStore.setCurrentPalEvalConfig({
+      ...evalConfig,
+      [checkName]: { ...evalConfig[checkName], val },
+    });
+  }
 </script>
 
 <Tooltip positionAlongRightEdge={true}>
   <div slot="content" let:onClick>
+    <button class={buttonStyle} on:click={() => proposeFix()}>
+      Try to fix
+    </button>
     <button
       class={buttonStyle}
       on:click={() => {
@@ -48,12 +65,32 @@
     >
       Ignore for this palette
     </button>
-    {#if check.hasParam}
-      <button class={buttonStyle}>This is too restrictive</button>
+    {#if check.paramOptions.type !== "none"}
+      <div>
+        <div>Adjust check parameter</div>
+        {#if check.paramOptions.type === "number"}
+          <input
+            min={check.paramOptions.min}
+            max={check.paramOptions.max}
+            type="number"
+            step={check.paramOptions.step}
+            value={check.config.val}
+            on:change={(e) => updateEvalConfig(check.name, e, (x) => Number(x))}
+          />
+        {/if}
+        {#if check.paramOptions.type === "enum"}
+          <select
+            value={check.config.val}
+            on:change={(e) => updateEvalConfig(check.name, e)}
+          >
+            {#each check.paramOptions.options as option}
+              <option value={option}>{option}</option>
+            {/each}
+          </select>
+        {/if}
+      </div>
     {/if}
-    <button class={buttonStyle} on:click={() => proposeFix()}>
-      Try to fix
-    </button>
+
     {#if requestState === "loading"}
       <div>Loading...</div>
     {:else if requestState === "failed"}

diff --git a/src/lib/ColorLint.test.ts b/src/lib/ColorLint.test.ts
@@ -23,7 +23,7 @@ const unique = <T>(arr: T[]): T[] => [...new Set(arr)];
 
 test("ColorLint - ColorNameDiscriminability", async () => {
   const examplePal = makePalFromHexes(["#5260d1", "#005ebe"]);
-  const exampleLint = new ColorNameDiscriminability(examplePal);
+  const exampleLint = new ColorNameDiscriminability(examplePal).run();
   expect(exampleLint.passes).toBe(false);
   expect(exampleLint.message).toBe(
     "Color Name discriminability check failed. The following color names are repeated: Royalblue (#5260d1, #005ebe)"
@@ -62,23 +62,23 @@ test("ColorLint - ColorBlind", async () => {
     "#00becf",
   ];
   const examplePal = makePalFromHexes(tableau10);
-  const exampleLint1 = new ColorBlind[0](examplePal);
+  const exampleLint1 = new ColorBlind[0](examplePal).run();
   expect(exampleLint1.passes).toBe(false);
   expect(exampleLint1.message).toMatchSnapshot();
 
-  const exampleLint2 = new ColorBlind[1](examplePal);
+  const exampleLint2 = new ColorBlind[1](examplePal).run();
   expect(exampleLint2.passes).toBe(false);
   expect(exampleLint2.message).toMatchSnapshot();
 
-  const exampleLint3 = new ColorBlind[2](examplePal);
+  const exampleLint3 = new ColorBlind[2](examplePal).run();
   expect(exampleLint3.passes).toBe(false);
   expect(exampleLint3.message).toMatchSnapshot();
 });
 
 const ughWhat = ["#00ffff", "#00faff", "#00e4ff", "#fdfdfc", "#00ffff"];
 test("ColorLint - BackgroundDifferentiability", async () => {
   const examplePal = makePalFromHexes(ughWhat);
-  const exampleLint = new BackgroundDifferentiability(examplePal);
+  const exampleLint = new BackgroundDifferentiability(examplePal).run();
   expect(exampleLint.passes).toBe(false);
   expect(exampleLint.message).toBe(
     "This palette has some colors (#fdfdfc) that are close to the background color"
@@ -87,7 +87,7 @@ test("ColorLint - BackgroundDifferentiability", async () => {
   expect(fix.colors.map((x) => x.toHex())).toMatchSnapshot();
 
   examplePal.background = colorFromHex("#00e4ff", "lab");
-  const exampleLint2 = new BackgroundDifferentiability(examplePal);
+  const exampleLint2 = new BackgroundDifferentiability(examplePal).run();
   expect(exampleLint2.passes).toBe(false);
   expect(exampleLint2.message).toBe(
     "This palette has some colors (#0ff, #00faff, #00e4ff, #0ff) that are close to the background color"

diff --git a/src/lib/linter.ts b/src/lib/linter.ts
@@ -11,6 +11,7 @@ import UglyColors from "./lints/ugly-colors";
 import SequentialOrder from "./lints/sequential-order";
 import AvoidExtremes from "./lints/avoid-extremes";
 import DivergingOrder from "./lints/diverging-order";
+import BackgroundContrast from "./lints/contrast";
 
 export function runLintChecks(palette: Palette): ColorLint<any, any>[] {
   return [
@@ -24,5 +25,6 @@ export function runLintChecks(palette: Palette): ColorLint<any, any>[] {
     new SequentialOrder(palette),
     new AvoidExtremes(palette),
     new DivergingOrder(palette),
-  ];
+    new BackgroundContrast(palette),
+  ].map((x) => x.run());
 }
diff --git a/src/lib/lints/ColorLint.ts b/src/lib/lints/ColorLint.ts
@@ -3,20 +3,13 @@ import { suggestFix } from "../api-calls";
 import { colorFromHex } from "../Color";
 
 export type TaskType = "sequential" | "diverging" | "categorical";
-type Annotation =
-  | { type: "line"; points: { x: number; y: number }[] }
-  | { type: "point"; x: number; y: number; color: string };
-
-// example usage
-// const checks = [..., new ColorNameDiscriminability(pal), ...];
-// const suggestions = checks.filter((check) => !check.passCheck).map((check) => check.suggestFix());
 
 function AIFix(palette: Palette, message: string, engine: string) {
+  const colorSpace = palette.colorSpace;
   return suggestFix(palette, message, engine as any).then((x) => {
     if (x.length === 0) {
       throw new Error("No suggestions");
     }
-    const colorSpace = palette.colors[0].spaceName;
     return {
       ...palette,
       colors: x[0].colors.map((x) => colorFromHex(x, colorSpace)),
@@ -30,17 +23,48 @@ export class ColorLint<CheckData, ParamType> {
   passes: boolean;
   checkData: CheckData;
   palette: Palette;
-  message: string;
+  message: string = "";
   hasParam: boolean = false;
-  param?: ParamType;
+  config: { val?: ParamType } = {};
+  defaultParam: ParamType = false as any;
+  paramOptions:
+    | { type: "number"; min: number; max: number; step: number }
+    | { type: "enum"; options: string[] }
+    | { type: "none" } = { type: "none" };
   level: "error" | "warning" = "error";
 
   constructor(Palette: Palette) {
     this.palette = Palette;
+    this.checkData = undefined as CheckData;
+    this.passes = false;
+  }
+
+  copy() {
+    const copy = new ColorLint<CheckData, ParamType>(this.palette);
+    copy.name = this.name;
+    copy.taskTypes = this.taskTypes;
+    copy.passes = this.passes;
+    copy.checkData = this.checkData;
+    copy.message = this.message;
+    copy.hasParam = this.hasParam;
+    copy.config = this.config;
+    copy.defaultParam = this.defaultParam;
+    copy.level = this.level;
+    return copy;
+  }
+
+  run() {
+    const { evalConfig } = this.palette;
+    this.config = {
+      ...evalConfig[this.name],
+      val: evalConfig[this.name]?.val || this.defaultParam,
+    };
+
     const { passCheck, data } = this._runCheck();
     this.passes = passCheck;
     this.checkData = data as CheckData;
     this.message = this.buildMessage();
+    return this;
   }
 
   _runCheck(): { passCheck: boolean; data: CheckData } {
@@ -51,12 +75,6 @@ export class ColorLint<CheckData, ParamType> {
     return "";
   }
 
-  increaseParam() {
-    if (!this.hasParam) {
-      throw new Error("Cannot increase param on lint without param");
-    }
-  }
-
   async suggestFix(engine?: string) {
     return AIFix(this.palette, this.message, engine || "openai");
   }