Skip to content

Commit

Permalink
Global lint settings (#44)
Browse files Browse the repository at this point in the history
* Global lint settings

* fix some bugs identified by the global lint thing

* fix test
  • Loading branch information
mcnuttandrew authored Feb 26, 2024
1 parent 6fc70ab commit 718c65d
Show file tree
Hide file tree
Showing 11 changed files with 124 additions and 21 deletions.
20 changes: 17 additions & 3 deletions src/App.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,29 @@
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
let 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),
},
};
lint(outPal, true).then((res): void => {
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);
Expand Down
9 changes: 9 additions & 0 deletions src/components/Tooltip.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,18 @@

{#if tooltipOpen && boundingBox}
<Portal target="body">
<!-- svelte-ignore a11y-click-events-have-key-events -->
<!-- svelte-ignore a11y-no-static-element-interactions -->
<div
class="absolute min-w-10"
id="tooltip"
style={`left: ${leftString}; top: ${topString}; z-index: 1000`}
on:click|stopPropagation={(e) => {
const id = e.target.id;
if (id === "tooltip") {
onClick(e);
}
}}
>
<div
class="relative"
Expand Down
3 changes: 1 addition & 2 deletions src/lib/CustomLint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -65,7 +64,7 @@ export function CreateCustomLint(props: CustomLint) {
.join(", ");
}

return props.failMessage.replace("{{blame}}", blame);
return props.failMessage.replaceAll("{{blame}}", blame);
}
};
}
4 changes: 2 additions & 2 deletions src/lib/__snapshots__/ColorLint.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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"`;

Expand Down
2 changes: 1 addition & 1 deletion src/lib/api-calls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ function workerDispatch() {
}
const dispatch = workerDispatch();

export function lint(pal: Palette, computeMessage?: boolean) {
export function lint(pal: Palette, computeMessage: boolean) {
// this may be too deep a copy?
return dispatch({
type: computeMessage ? "run-lint" : "run-lint-no-message",
Expand Down
2 changes: 2 additions & 0 deletions src/lib/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,15 @@ export function runLintChecks(
const ignoreList = palette.evalConfig;
const affects = palette.intendedAffects;
const contexts = palette.intendedContexts;
const globallyIgnoredLints = palette.evalConfig?.globallyIgnoredLints || [];
const lints = [
DivergingOrder,
...customLints.map((x) => CreateCustomLint(x)),
] as (typeof ColorLint)[];
return (
lints
.map((x) => new x(palette))
.filter((x) => !globallyIgnoredLints.includes(x.isCustom))
// task type
.filter((x) => x.taskTypes.includes(palette.type))
// affect type
Expand Down
2 changes: 1 addition & 1 deletion src/lib/lints/name-discrim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.`,
Expand Down
21 changes: 13 additions & 8 deletions src/lib/lints/size-discrim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand All @@ -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,
},
},
Expand All @@ -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",
};
Expand Down
8 changes: 5 additions & 3 deletions src/linting/Eval.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -52,7 +53,7 @@
}
</script>

<div class="bg-stone-300 w-full">
<div class="bg-stone-300 w-full flex">
<Nav
tabs={["regular", "compact"]}
isTabSelected={(x) => x === $configStore.evalDisplayMode}
Expand All @@ -61,6 +62,7 @@
configStore.setEvalDisplayMode(x);
}}
/>
<GlobalLintConfig />
</div>
<div class="flex h-full bg-stone-100">
{#if showEvalColumn}
Expand Down Expand Up @@ -122,7 +124,7 @@
<LintDisplay {check} />
{/if}
{/each}
{#if checkGroup[1].length === 0}
{#if checkGroup[1].length === 0 && $lintStore.loadState === "loading"}
<div class="text-sm animate-pulse italic font-bold">Loading</div>
{/if}
{/each}
Expand All @@ -133,7 +135,7 @@
onClose={() => {
setTimeout(() => {
loadLints()
.then(() => lint(currentPal))
.then(() => lint(currentPal, false))
.then((res) => {
checks = res;
});
Expand Down
60 changes: 60 additions & 0 deletions src/linting/GlobalLintConfig.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<script lang="ts">
import lintStore from "../stores/lint-store";
import Tooltip from "../components/Tooltip.svelte";
import { buttonStyle } from "../lib/styles";
$: lints = [...$lintStore.lints].sort((a, b) => a.name.localeCompare(b.name));
$: ignoreList = $lintStore.globallyIgnoredLints;
$: ignoredSet = new Set(ignoreList);
</script>

<Tooltip positionAlongRightEdge={true}>
<div slot="content" class="flex flex-col">
<div class="font-bold">Global Lint Ignore List</div>
<div class="italic text-sm">Checked lints will be ignored</div>
<div class="flex">
<button
class={buttonStyle}
on:click={() =>
lintStore.setGloballyIgnoredLints(lints.map((x) => x.id))}
>
Ignore All
</button>
<button
class={buttonStyle}
on:click={() => lintStore.setGloballyIgnoredLints([])}
>
Ignore none
</button>
</div>
<div class="flex flex-col">
{#each lints as lint}
<label class="flex">
<input
type="checkbox"
checked={ignoredSet.has(lint.id)}
class="mr-2"
on:change={() => {
const newLints = [...ignoreList];
if (ignoredSet.has(lint.id)) {
newLints.filter((l) => l !== lint.id);
} else {
newLints.push(lint.id);
}
lintStore.setGloballyIgnoredLints(newLints);
}}
/>
<div class="whitespace-nowrap mr-2">
{lint.name}
{#if lint.taskTypes.length && lint.taskTypes.length !== 3}
({lint.taskTypes.join(", ")})
{/if}
</div>
</label>
{/each}
</div>
</div>
<button slot="target" let:toggle on:click={toggle} class={buttonStyle}>
SETTINGS
</button>
</Tooltip>
14 changes: 13 additions & 1 deletion src/stores/lint-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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 })),
};
}

Expand Down

0 comments on commit 718c65d

Please sign in to comment.