From 7237043b64afdeb201d046bd5eb980b2e2897b84 Mon Sep 17 00:00:00 2001 From: Andrew Michael McNutt Date: Sun, 25 Feb 2024 15:42:54 -0800 Subject: [PATCH 1/3] Global lint settings --- src/App.svelte | 22 +++++++++-- src/components/Tooltip.svelte | 9 +++++ src/lib/api-calls.ts | 2 +- src/lib/linter.ts | 2 + src/linting/Eval.svelte | 8 ++-- src/linting/GlobalLintConfig.svelte | 59 +++++++++++++++++++++++++++++ src/stores/lint-store.ts | 14 ++++++- 7 files changed, 108 insertions(+), 8 deletions(-) create mode 100644 src/linting/GlobalLintConfig.svelte diff --git a/src/App.svelte b/src/App.svelte index 29b17825..6c21f096 100644 --- a/src/App.svelte +++ b/src/App.svelte @@ -36,15 +36,31 @@ import { debounce } from "vega"; $: selectedLint = $lintStore.focusedLint; - $: updateSearchDebounced = debounce(10, (pal: any) => { + const bindStr = "!!"; + // it appears that there is a bug in the vega debounce implementation, that causes the second argument to not fire + $: updateSearchDebounced = debounce(10, (x: [any, string]) => { + const [pal, ignoreString] = x; // keep the noise down on the console if (!selectedLint && pal) { - lint(pal, true).then((res) => { + lintStore.setLoadState("loading"); + + const outPal = { + ...pal, + evalConfig: { + ...pal.evalConfig, + globallyIgnoredLints: ignoreString.split(bindStr), + }, + }; + console.log("linting", ignoreString); + lint(outPal, true).then((res): void => { + console.log("lint result", res); lintStore.postCurrentChecks(res); }); } }); - $: updateSearchDebounced(currentPal); + // this weird foot work is to circumvent the svelte reactivity which is weird aggressive in this one specific case + $: globalString = $lintStore.globallyIgnoredLints.join(bindStr); + $: globalString, updateSearchDebounced([currentPal, globalString]); let innerWidth = window.innerWidth; $: scatterSize = Math.max(Math.min(innerWidth * 0.3, 450), 350); diff --git a/src/components/Tooltip.svelte b/src/components/Tooltip.svelte index 2564c6b9..3ed733e1 100644 --- a/src/components/Tooltip.svelte +++ b/src/components/Tooltip.svelte @@ -70,9 +70,18 @@ {#if tooltipOpen && boundingBox} + +
{ + const id = e.target.id; + if (id === "tooltip") { + onClick(e); + } + }} >
CreateCustomLint(x)), @@ -53,6 +54,7 @@ export function runLintChecks( return ( lints .map((x) => new x(palette)) + .filter((x) => !globallyIgnoredLints.includes(x.isCustom)) // task type .filter((x) => x.taskTypes.includes(palette.type)) // affect type diff --git a/src/linting/Eval.svelte b/src/linting/Eval.svelte index fa4eaf80..a07c8e09 100644 --- a/src/linting/Eval.svelte +++ b/src/linting/Eval.svelte @@ -11,6 +11,7 @@ import NewLintSuggestion from "./NewLintSuggestion.svelte"; import { titleCase } from "../lib/utils"; import EvalColorColumn from "./EvalColorColumn.svelte"; + import GlobalLintConfig from "./GlobalLintConfig.svelte"; import type { LintResult } from "../lib/ColorLint"; @@ -52,7 +53,7 @@ } -
+
{#if showEvalColumn} @@ -122,7 +124,7 @@ {/if} {/each} - {#if checkGroup[1].length === 0} + {#if checkGroup[1].length === 0 && $lintStore.loadState === "loading"}
Loading
{/if} {/each} @@ -133,7 +135,7 @@ onClose={() => { setTimeout(() => { loadLints() - .then(() => lint(currentPal)) + .then(() => lint(currentPal, false)) .then((res) => { checks = res; }); diff --git a/src/linting/GlobalLintConfig.svelte b/src/linting/GlobalLintConfig.svelte new file mode 100644 index 00000000..5c44e20e --- /dev/null +++ b/src/linting/GlobalLintConfig.svelte @@ -0,0 +1,59 @@ + + + +
+
Global Lint Ignore List
+
Checked lints will be ignored
+
+ + +
+
+ {#each lints as lint} + + {/each} +
+
+ +
diff --git a/src/stores/lint-store.ts b/src/stores/lint-store.ts index 94a34291..4415058c 100644 --- a/src/stores/lint-store.ts +++ b/src/stores/lint-store.ts @@ -11,12 +11,16 @@ interface StoreData { lints: CustomLint[]; focusedLint: string | false; currentChecks: LintResult[]; + globallyIgnoredLints: string[]; + loadState?: "loading" | "idle"; } const InitialStore: StoreData = { lints: [], focusedLint: false, currentChecks: [], + globallyIgnoredLints: [], + loadState: "idle", }; const builtInIndex = BUILT_INS.reduce((acc, x) => { @@ -113,7 +117,15 @@ function createStore() { }; }), postCurrentChecks: (checks: LintResult[]) => - persistUpdate((old) => ({ ...old, currentChecks: checks })), + persistUpdate((old) => ({ + ...old, + currentChecks: checks, + loadState: "idle", + })), + setGloballyIgnoredLints: (lints: string[]) => + persistUpdate((old) => ({ ...old, globallyIgnoredLints: [...lints] })), + setLoadState: (state: StoreData["loadState"]) => + persistUpdate((old) => ({ ...old, loadState: state })), }; } From c50aaa11941014b7e8e40c27625f4936df60ea95 Mon Sep 17 00:00:00 2001 From: Andrew Michael McNutt Date: Sun, 25 Feb 2024 15:55:44 -0800 Subject: [PATCH 2/3] fix some bugs identified by the global lint thing --- src/App.svelte | 4 +--- src/lib/lints/name-discrim.ts | 2 +- src/lib/lints/size-discrim.ts | 21 +++++++++++++-------- src/linting/GlobalLintConfig.svelte | 1 + 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/App.svelte b/src/App.svelte index 6c21f096..0761d7bb 100644 --- a/src/App.svelte +++ b/src/App.svelte @@ -38,7 +38,7 @@ $: selectedLint = $lintStore.focusedLint; const bindStr = "!!"; // it appears that there is a bug in the vega debounce implementation, that causes the second argument to not fire - $: updateSearchDebounced = debounce(10, (x: [any, string]) => { + let updateSearchDebounced = debounce(10, (x: [any, string]) => { const [pal, ignoreString] = x; // keep the noise down on the console if (!selectedLint && pal) { @@ -51,9 +51,7 @@ globallyIgnoredLints: ignoreString.split(bindStr), }, }; - console.log("linting", ignoreString); lint(outPal, true).then((res): void => { - console.log("lint result", res); lintStore.postCurrentChecks(res); }); } diff --git a/src/lib/lints/name-discrim.ts b/src/lib/lints/name-discrim.ts index e28c6982..4d02cd52 100644 --- a/src/lib/lints/name-discrim.ts +++ b/src/lib/lints/name-discrim.ts @@ -56,7 +56,7 @@ const lint: CustomLint = { }, }), name: "Color Name Discriminability", - taskTypes: ["sequential"] as const, + taskTypes: ["sequential", "categorical", "diverging"] as const, level: "error", group: "usability", description: `Being able to identify colors by name is important for usability and for memorability.`, diff --git a/src/lib/lints/size-discrim.ts b/src/lib/lints/size-discrim.ts index c31bf550..b3a13cf6 100644 --- a/src/lib/lints/size-discrim.ts +++ b/src/lib/lints/size-discrim.ts @@ -29,12 +29,18 @@ function jndLabInterval(p: pType, s: sType) { return nd(pVal, sVal); } -const lints: CustomLint[] = ["Thin", "Medium", "Wide"].map((key) => { +const itemSizeDescriptions = { + Thin: "small blocks such as small circles or lines", + Medium: "medium blocks such as bars in a bar chart or small graphics", + Wide: "large blocks of color such as backgrounds or countries on a map", +} as const; +const keys = ["Thin", "Medium", "Wide"] as const; +const lints: CustomLint[] = keys.map((key) => { const p = "default"; const s = key as keyof typeof sMap; const jnd = jndLabInterval(p, s); return { - name: `Works for ${key} marks`, + name: `Mark size legibility: ${key}`, program: JSONToPrettyString({ // @ts-ignore $schema: `${location.href}lint-schema.json`, @@ -47,10 +53,9 @@ const lints: CustomLint[] = ["Thin", "Medium", "Wide"].map((key) => { or: [ { ">": { - // left: { - // absDiff: { left: { "lab.l": "x" }, right: { "lab.l": "y" } }, - // }, - left: { absDiff: { left: 0, right: 1 } }, + left: { + absDiff: { left: { "lab.l": "x" }, right: { "lab.l": "y" } }, + }, right: jnd.l, }, }, @@ -77,8 +82,8 @@ const lints: CustomLint[] = ["Thin", "Medium", "Wide"].map((key) => { taskTypes: ["sequential", "diverging", "categorical"] as const, level: "warning", group: "usability", - description: `Pairs of colors in a palette should be differentiable from each other in ${key} lines. `, - failMessage: `This palette has some colors ({{blame}}) that are close to each other in perceptual space and will not be resolvable for ${key} areas.`, + description: `Pairs of colors in a palette should be differentiable from each other in ${key} marks. `, + failMessage: `This palette has some colors ({{blame}}) that are close to each other in perceptual space and will not be resolvable for ${key} areas. This involves elements like ${itemSizeDescriptions[key]}`, id: `${key}-discrim-built-in`, blameMode: "pair", }; diff --git a/src/linting/GlobalLintConfig.svelte b/src/linting/GlobalLintConfig.svelte index 5c44e20e..14f18eab 100644 --- a/src/linting/GlobalLintConfig.svelte +++ b/src/linting/GlobalLintConfig.svelte @@ -33,6 +33,7 @@ { const newLints = [...ignoreList]; if (ignoredSet.has(lint.id)) { From 4a8f5b0655eaaa225a27522d134b229084ecc5fb Mon Sep 17 00:00:00 2001 From: Andrew Michael McNutt Date: Sun, 25 Feb 2024 16:05:09 -0800 Subject: [PATCH 3/3] fix test --- src/lib/CustomLint.ts | 3 +-- src/lib/__snapshots__/ColorLint.test.ts.snap | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/lib/CustomLint.ts b/src/lib/CustomLint.ts index 07dc2be6..789f8052 100644 --- a/src/lib/CustomLint.ts +++ b/src/lib/CustomLint.ts @@ -42,7 +42,6 @@ export function CreateCustomLint(props: CustomLint) { ...options, }); if (result) return { passCheck: true, data: blame }; - let newBlame: number[] | number[][] = []; if (this.blameMode !== "none") { newBlame = permutativeBlame(prog, this.palette, this.blameMode); @@ -65,7 +64,7 @@ export function CreateCustomLint(props: CustomLint) { .join(", "); } - return props.failMessage.replace("{{blame}}", blame); + return props.failMessage.replaceAll("{{blame}}", blame); } }; } diff --git a/src/lib/__snapshots__/ColorLint.test.ts.snap b/src/lib/__snapshots__/ColorLint.test.ts.snap index ac8d412e..c57d910f 100644 --- a/src/lib/__snapshots__/ColorLint.test.ts.snap +++ b/src/lib/__snapshots__/ColorLint.test.ts.snap @@ -56,9 +56,9 @@ exports[`ColorLint - SequentialOrder 1`] = `"This pal should be ordered by light exports[`ColorLint - SequentialOrder 2`] = `"This pal should be ordered by lightness if being used as a sequential palette. #ecddff, #bbc3ff may be to blame."`; -exports[`ColorLint - SizeDiscrim (Thin) 1`] = `"This palette has some colors () that are close to each other in perceptual space and will not be resolvable for Thin areas."`; +exports[`ColorLint - SizeDiscrim (Thin) 1`] = `"This palette has some colors () that are close to each other in perceptual space and will not be resolvable for Thin areas. This involves elements like small blocks such as small circles or lines"`; -exports[`ColorLint - SizeDiscrim (Thin) 2`] = `"This palette has some colors (#0084a9 and #009de5) that are close to each other in perceptual space and will not be resolvable for Thin areas."`; +exports[`ColorLint - SizeDiscrim (Thin) 2`] = `"This palette has some colors (#0084a9 and #009de5) that are close to each other in perceptual space and will not be resolvable for Thin areas. This involves elements like small blocks such as small circles or lines"`; exports[`ColorLint - UglyColors 1`] = `"This palette has some colors (specifically ) that are close to what are known as ugly colors"`;