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

Global lint settings #44

Merged
merged 3 commits into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading