Skip to content

Commit

Permalink
Address some MCS requests (#101)
Browse files Browse the repository at this point in the history
* Fix some browse/manage bugs

* close color space picker on change

* dont revert to lab from browse

* add color test

* add labels to fixes

* add an all colors div

* fix test
  • Loading branch information
mcnuttandrew authored Sep 12, 2024
1 parent 5d2bf42 commit f3bc596
Show file tree
Hide file tree
Showing 14 changed files with 1,239 additions and 853 deletions.
15 changes: 15 additions & 0 deletions apps/color-buddy/public/tableau-colors.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,19 @@
[
{
"name": "Color Test",
"type": "categorical",
"colors": [
"#f00",
"#ff0",
"#0f0",
"#0ff",
"#00f",
"#f0f",
"#fff",
"#888",
"#000"
]
},
{
"name": "FL10.0",
"type": "categorical",
Expand Down
9 changes: 6 additions & 3 deletions apps/color-buddy/src/content-modules/Browse.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@
import PreviewSelector from "../example/PreviewSelector.svelte";
import NewExampleModal from "../example/NewExampleModal.svelte";
import { convertPalToSpace } from "../lib/utils";
$: familiarPals = $examplePalStore.palettes.map((x) => x.palette);
$: currentPal = $colorStore.palettes[$colorStore.currentPal];
$: colorSpace = currentPal.colorSpace;
let searchString = "";
$: filteredPals = familiarPals
Expand All @@ -24,9 +28,8 @@
] as any;
function usePal(palette: Palette) {
colorStore.createNewPal(palette);
colorStore.createNewPal(convertPalToSpace(palette, colorSpace));
focusStore.clearColors();
configStore.setRoute("examples");
}
</script>

Expand All @@ -46,7 +49,7 @@
These are pre-created palettes that you can use as a starting point.
</div>
</div>
<div class="overflow-y-scroll h-full p-2 bg-stone-100">
<div class="overflow-y-scroll h-full p-2 bg-stone-100 content-baseline">
<div class="flex flex-wrap">
{#each filteredPals as palette}
<BrowseCard
Expand Down
6 changes: 3 additions & 3 deletions apps/color-buddy/src/content-modules/Manage.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
name: "Use",
action: () => {
colorStore.startUsingPal(paletteIdx);
configStore.setRoute("examples");
},
},
{
Expand Down Expand Up @@ -106,7 +105,9 @@
</div>
</div>

<div class="flex flex-wrap bg-stone-100 h-full overflow-auto p-4">
<div
class="flex flex-wrap bg-stone-100 h-full overflow-auto p-4 content-baseline"
>
{#each $colorStore.palettes as pal, paletteIdx}
<BrowseCard
markAsCurrent={$colorStore.currentPal === paletteIdx}
Expand All @@ -120,7 +121,6 @@
palette={pal}
titleClick={() => {
colorStore.startUsingPal(paletteIdx);
configStore.setRoute("examples");
}}
title={pal.name}
previewIndex={$configStore.manageBrowsePreviewIdx}
Expand Down
18 changes: 12 additions & 6 deletions apps/color-buddy/src/controls/SetColorSpace.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
return a.localeCompare(b);
});
$: advancedSpaceOptions = Object.keys(colorPickerConfig).filter(
(x) => !notAllowed.has(x) && colorPickerConfig[x].advancedSpace
);
$: advancedSpaceOptions = Object.keys(colorPickerConfig)
.filter((x) => !notAllowed.has(x) && colorPickerConfig[x].advancedSpace)
.filter((x) => x.toLowerCase() !== "srgb");
</script>

<Tooltip>
<div slot="content" class="flex flex-col max-w-md">
<div slot="content" class="flex flex-col max-w-md" let:onClick>
<div class="font-bold">Set Color Space</div>
<div class="text-sm">
Select the color space to use for the color picker.
Expand All @@ -29,7 +29,10 @@
<button
class={`${buttonStyle} justify-self-start`}
class:font-bold={space === colorSpace}
on:click={() => onChange(space)}
on:click={() => {
onChange(space);
onClick();
}}
>
{space.toUpperCase()}
</button>
Expand All @@ -49,7 +52,10 @@
<button
class={`${buttonStyle} justify-self-start`}
class:font-bold={space === colorSpace}
on:click={() => onChange(space)}
on:click={() => {
onChange(space);
onClick();
}}
>
{space.toUpperCase()}
</button>
Expand Down
4 changes: 2 additions & 2 deletions apps/color-buddy/src/example/BrowseCard.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
</script>

<div
class="flex flex-col border-2 rounded w-min min-w-fit mr-4 mb-2 browse-card"
class="flex flex-col border-2 rounded w-min min-w-fit mr-4 mb-2 browse-card shrink self-start"
style={`background: ${palette.background.toHex()}; min-height: ${minHeight}px;`}
>
<div class="bg-stone-300 w-full flex justify-between p-1">
Expand Down Expand Up @@ -68,7 +68,7 @@
</div>
</Tooltip>
</div>
<div class="h-full flex justify-center items-center p-4">
<div class="flex shrink justify-center items-center p-4">
{#if previewIndex === -1}
<PalPreview pal={palette} allowModification={false} />
{:else if !example}
Expand Down
10 changes: 10 additions & 0 deletions apps/color-buddy/src/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -504,3 +504,13 @@ export function processBodyTextToColors(body: string, colorSpace: string) {
.filter((x) => x.length > 0)
.map((x) => Color.colorFromString(x, colorSpace as any, true));
}

export let convertPalToSpace = (
pal: Palette,
colorSpace: ColorSpace
): Palette => ({
...pal,
colorSpace,
background: Color.toColorSpace(pal.background, colorSpace),
colors: pal.colors.map((x) => Color.toColorSpace(x, colorSpace)),
});
82 changes: 45 additions & 37 deletions apps/color-buddy/src/linting/EvalResponse.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@
$: palette = $colorStore.palettes[$colorStore.currentPal];
$: engine = $configStore.engine;
$: suggestions = [] as Palette[];
$: suggestions = [] as { pal: Palette; label: string }[];
$: colorSpace = palette.colorSpace;
$: lintProgram = lintResult.lintProgram;
function proposeFix(fixType: "ai" | "monte" | "heuristic") {
function proposeFix(fixType: "ai" | "monte" | "heuristic", label: string) {
requestState = "loading";
let hasRetried = false;
const getFix = () => {
Expand All @@ -40,7 +40,7 @@
fix = suggestLintFix(palette, lintResult);
}
return fix.then((x) => {
suggestions = [...suggestions, ...x];
suggestions = [...suggestions, ...x.map((pal) => ({ pal, label }))];
requestState = "loaded";
logEvent(
"lint-fix",
Expand Down Expand Up @@ -117,16 +117,19 @@

{#if lintResult.kind === "success" && !lintResult.passes}
<!-- hiding the LLM based solution, bc it works poorly -->
<button class={buttonStyle} on:click={() => proposeFix("ai")}>
<button class={buttonStyle} on:click={() => proposeFix("ai", "LLM")}>
Try to fix (LLM)
</button>
{#if lintProgram && lintProgram.program.length}
<button class={buttonStyle} on:click={() => proposeFix("monte")}>
<button class={buttonStyle} on:click={() => proposeFix("monte", "AI")}>
Try to fix (AI)
</button>
{/if}
{#if lintProgram.subscribedFix && lintProgram.subscribedFix !== "none"}
<button class={buttonStyle} on:click={() => proposeFix("heuristic")}>
<button
class={buttonStyle}
on:click={() => proposeFix("heuristic", "ColorBuddy")}
>
Try to fix (ColorBuddy)
</button>
{/if}
Expand Down Expand Up @@ -174,38 +177,43 @@
{:else if requestState === "failed"}
<div>Failed to generate suggestions</div>
{/if}
{#each suggestions as suggestion}
<div class="flex">
<PalDiff beforePal={currentPal} afterPal={suggestion} />
<div class="flex flex-col justify-between">
<button
class={buttonStyle}
on:click={() => {
if (suggestion) {
colorStore.setCurrentPal(suggestion);
focusStore.clearColors();
requestState = "idle";
suggestions = [];
onClick();
}
}}
>
Use
</button>
<button
class={buttonStyle}
on:click={() => {
suggestions = suggestions.filter((x) => x !== suggestion);
if (suggestions.length === 0) {
requestState = "idle";
}
}}
>
Reject
</button>
<div class="max-h-40 overflow-auto">
{#each suggestions as suggestion, idx}
<div class="flex relative mb-4">
<PalDiff beforePal={currentPal} afterPal={suggestion.pal} />
<div class="flex flex-col justify-between items-baseline">
<div class="font-bold pl-2 mb-0">
{suggestion.label} fix
</div>
<button
class={buttonStyle}
on:click={() => {
if (suggestion) {
colorStore.setCurrentPal(suggestion.pal);
focusStore.clearColors();
requestState = "idle";
suggestions = [];
onClick();
}
}}
>
Use
</button>
<button
class={buttonStyle}
on:click={() => {
suggestions = suggestions.filter((_, jdx) => jdx !== idx);
if (suggestions.length === 0) {
requestState = "idle";
}
}}
>
Reject
</button>
</div>
</div>
</div>
{/each}
{/each}
</div>
</div>
<button
slot="target"
Expand Down
9 changes: 2 additions & 7 deletions apps/color-buddy/src/stores/color-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { writable } from "svelte/store";
import { Color } from "color-buddy-palette";
import type { Palette, StringPalette, ColorSpace } from "color-buddy-palette";

import { deDup, newGenericPal } from "../lib/utils";
import { deDup, newGenericPal, convertPalToSpace } from "../lib/utils";

interface StoreData {
palettes: Palette[];
Expand Down Expand Up @@ -213,12 +213,7 @@ function createStore() {
reset: () => set({ ...convertStoreHexToColor(InitialStore) }),

setColorSpace: (colorSpace: ColorSpace) =>
palUp((n) => ({
...n,
colorSpace,
background: Color.toColorSpace(n.background, colorSpace),
colors: n.colors.map((x) => Color.toColorSpace(x, colorSpace)),
})),
palUp((n) => convertPalToSpace(n, colorSpace)),
clearPalettes: () =>
persistUpdate(() => ({
currentPal: 0,
Expand Down
7 changes: 4 additions & 3 deletions apps/color-buddy/src/stores/lint-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,10 @@ function createStore() {
let storeBase = deserializeStore({ ...InitialStore, ...x });
let lints = (storeBase.lints || []) as LintProgram[];
// force-ably set lints to defaults
// .map(
// (x: LintProgram) => builtInIndex[x.id] || x
// ) as LintProgram[];
// TODO turn off for release...
lints = lints.map(
(x: LintProgram) => builtInIndex[x.id] || x
) as LintProgram[];
const missingBuiltIns = PREBUILT_LINTS.filter(
(x) => !lints.find((y) => y.id === x.id)
).map((x) => ({
Expand Down
64 changes: 33 additions & 31 deletions apps/docs/docs/buglist.md
Original file line number Diff line number Diff line change
@@ -1,45 +1,47 @@
# Bugs and improvements

## Browse/Manage
- After clicking on a palette, the palette updates and the tab switches to samples. Leave the tab on the view of the palettes
- In the manage pane, make the windows tight on the palettes, as in Browse

- [x] After clicking on a palette, the palette updates and the tab switches to samples. Leave the tab on the view of the palettes
- [x] In the manage pane, make the windows tight on the palettes, as in Browse

## Editing and color spaces
- when you change the color space, dismiss the menu once you select one.
- Selecting a new palette changes the color space back to LAB. It shouldn't
- The color space specific metrics in the lower left corner aren't tracking selection. Only the hex updates.
- Update the color space descriptions (details TBD from Maureen)
- JZ values are all in the range 0-0.1, think you need to scale by 10. Comparing JAJBJZ to LAB, I think there's also a scaling problem in the A and B values as well.
- No color space background for JAJBJZ. That's two issues in JAJBJZ. Do we really want to include it?
- I'd remove the SRGB editing space. Even if it's working correctly (which I think it isn't), it's going to be very confusing.
- Possible additional color spaces
- XYZ If you do this one, put Y on the single axis. Could be useful for evaluating the simulations.
- xyY While not supported natively in the Color package, the conversion between this and XYZ is trivial: x = X/(X+Y+Z), y = Y/(X+Y+Z); Plot xy on the plane, Y on the lightness scale. This will show the colors on the CIE Chromaticity diagram.
- (#f00, #ff0, #0f0, #0ff, #00f, #f0f, #fff, #888, #000) is a useful palette for evaluting colorspaces
- HSL, #fff should be in the middle
- #0ff is displayed in LAB as out of gamut. It shouldn't be. Clip to gamut doesn't fix it, similar for LCH. Except sometimes it's fine (argh)
- when a color (nearly) matches the background, it would be nice to outline it so it doesn't vanish. Use a slightly darker/lighter version of the color, depending on whether it's a light or dark background. Most obvious in this palette for black and white.

## Comparison
- The dotted line connections are useful if the colors are in the same order and have only changed slightly, but confusing if the palette order has changed (for example, Tableau 10 Classic and Tableau 10). Since small changes are easy to see with just the rings vs disks, I'd leave the lines out for now.
- To get a palette to compare, it has to be in the Manage set. From the Change Compared Palette dropdown, I'd add a "browse for more palettes" affordance that switched you to the Browse pane.
- [x] when you change the color space, dismiss the menu once you select one.
- [x] Selecting a new palette changes the color space back to LAB. It shouldn't
- [ ] The color space specific metrics in the lower left corner aren't tracking selection. Only the hex updates. AM: can't reproduce
- [ ] Update the color space descriptions (details TBD from Maureen)
- [ ] JZ values are all in the range 0-0.1, think you need to scale by 10. Comparing JAJBJZ to LAB, I think there's also a scaling problem in the A and B values as well. AM: nope that's how this space is
- [ ] No color space background for JAJBJZ. That's two issues in JAJBJZ. Do we really want to include it?. AM: can't reproduce.
- [x] I'd remove the SRGB editing space. Even if it's working correctly (which I think it isn't), it's going to be very confusing.

## Eval
- When you ask for a fix and it displays the before and after palette, the vertical lines seem to connect the colors that didn't change. I would expect to have them on the colors that did change. Ideally, they'd be arrows pointing from/to.
- These palettes are labeled Use and Reject. I think it would be useful if they included what algorithm was used. For example, Use(ColorBuddy), Reject(LLM)
- In Gamut should be a Usability Check, not a Design Check.
- Settings raises a long list of lints, which really needs a scroll bar or some other way of managing the list on a smaller screen. Also, it would be more conventional to make this a list of Selected lints rather than a List of Ignored lints.
- the New lint text box doesn't seem to do anything. I tried several types of requests, got nothing back.
- Being able to copy and modify an existing lint seems very powerful, but I see no way to do that.
- I would like a way to test all colors for text legibility, no matter what their tags are. Maybe this is just a new lint?
- I would like to improve the appearance of the red X's on the colors. Ideally, we'd have some tasteful icons, one per major category (Usability, Accessibility, Design, Custom) that were legible on all colors.
- I'd like to see more metrics in the lint descriptions and errors. For example, what contrast do we need for text legibility, and what is the contrast for those colors that failed?
- [x] (#f00, #ff0, #0f0, #0ff, #00f, #f0f, #fff, #888, #000) is a useful palette for evaluating colorspaces. AM: added as "Color Test"
- [ ] HSL, #fff should be in the middle
- [ ] #0ff is displayed in LAB as out of gamut. It shouldn't be. Clip to gamut doesn't fix it, similar for LCH. Except sometimes it's fine (argh)
- [ ] when a color (nearly) matches the background, it would be nice to outline it so it doesn't vanish. Use a slightly darker/lighter version of the color, depending on whether it's a light or dark background. Most obvious in this palette for black and white.

# Possible additional color spaces

## Simulations
- As noted before, the simulated L* values are noticably different than the original, which is surprising. This is true even for grayscale, so I really wonder if there's some bug in the L* plotting pipeline, rather than in the algorithms.
- [ ] XYZ If you do this one, put Y on the single axis. Could be useful for evaluating the simulations.
- [ ] xyY While not supported natively in the Color package, the conversion between this and XYZ is trivial: x = X/(X+Y+Z), y = Y/(X+Y+Z); Plot xy on the plane, Y on the lightness scale. This will show the colors on the CIE Chromaticity diagram.

## Comparison

- [ ] The dotted line connections are useful if the colors are in the same order and have only changed slightly, but confusing if the palette order has changed (for example, Tableau 10 Classic and Tableau 10). Since small changes are easy to see with just the rings vs disks, I'd leave the lines out for now.
- [ ] To get a palette to compare, it has to be in the Manage set. From the Change Compared Palette dropdown, I'd add a "browse for more palettes" affordance that switched you to the Browse pane.

## Eval

- [ ] When you ask for a fix and it displays the before and after palette, the vertical lines seem to connect the colors that didn't change. I would expect to have them on the colors that did change. Ideally, they'd be arrows pointing from/to.
- [x] These palettes are labeled Use and Reject. I think it would be useful if they included what algorithm was used. For example, Use(ColorBuddy), Reject(LLM)
- [x] In Gamut should be a Usability Check, not a Design Check.
- [ ] Settings raises a long list of lints, which really needs a scroll bar or some other way of managing the list on a smaller screen. Also, it would be more conventional to make this a list of Selected lints rather than a List of Ignored lints.
- [ ] the New lint text box doesn't seem to do anything. I tried several types of requests, got nothing back.
- [ ] Being able to copy and modify an existing lint seems very powerful, but I see no way to do that. AM: click lint info / click customize / click clone
- [ ] I would like a way to test all colors for text legibility, no matter what their tags are. Maybe this is just a new lint?
- [ ] I would like to improve the appearance of the red X's on the colors. Ideally, we'd have some tasteful icons, one per major category (Usability, Accessibility, Design, Custom) that were legible on all colors.
- [ ] I'd like to see more metrics in the lint descriptions and errors. For example, what contrast do we need for text legibility, and what is the contrast for those colors that failed?

## Simulations

- [ ] As noted before, the simulated L* values are noticably different than the original, which is surprising. This is true even for grayscale, so I really wonder if there's some bug in the L* plotting pipeline, rather than in the algorithms.
Loading

0 comments on commit f3bc596

Please sign in to comment.