From 242d572da4b7d65824e85ad7e7431890d40658c7 Mon Sep 17 00:00:00 2001 From: Andrew Michael McNutt Date: Thu, 11 Jan 2024 12:49:03 -0800 Subject: [PATCH] reduce undo granularity --- README.md | 39 +++++++++++++++++++++ src/App.svelte | 2 ++ src/components/ColorScatterPlot.svelte | 5 +++ src/stores/color-store.ts | 47 ++++++++++++++++++++++---- 4 files changed, 87 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 1c9b9e25..29390e28 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,45 @@ Use: should point to localhost:8888 if all is well First time you start it up you should also run `yarn prep data` +## Direct manipulation bugs/improvement + +The basic UX for editing has the following components + +## Two graphs, one 2D and one 1D. + +The 2D graph displays hue/chroma graph and the 1D graph displays lightness. You can map any of a number of colorspaces onto this pair of graphs. (in the code, the lightness graph is "Z"). I propose the following changes + +- [x] Reduce the visual impact of the axes and labels by making them transparent gray. Set the colors, made them adaptive, set the luminance flip to .3 (50% visually) +- [ ] Flip the Y axis (zero at the bottom) +- [ ] Make the labels integers for CIELAB +- [x] Make the axis scale sliders less visualy prominent. +- [ ] Consider removing the axis sliders, replace them with zoom controls in the same panel as the background color selection. Changing these values is rare, they don't need to take up so much UX space. +- [ ] The 2D graph should always be a centered hue/chroma graph. CIELAB and CIELCH would therefore use the same graph. Leave the LAB vs LCH distinction for slider based editing. +- [ ] I'd consider creating an rectangle that enclosed both graphs and filling it with the background color. I might then also pull the axis labels outside bounding box for each graph. + +## Single and multiple selection, drag and drop editing. + +Click to select, drag or shift click to multi-select + +- [x] Show the selection bounding box only on multi-select. Make it thinner and a lighter +- [ ] **Bug** a single click in the 1D graph selects and immediately deselects the color. +- [ ] **Bug** shift click doesn't do multi-select for me. +- [ ] I would add a deselect when you click in the white space, either a single click or the start of a new area select (this may also be a bug) +- [ ] There needs to be feedback when you drag out of gamut +- [x] Undo for dragging is too granular. Undo should undo the entire drag. + +## Color editing with sliders. + +To raise the sliders, you need to click on one of the examples that are displayed below the graphs. You can then manipulate 2 colorspaces simultaneously. + +- [ ] I'd move the examples into the example pane, use the space to permanently display the sliders, make them longer +- [ ] Always display the hex values of the state of the sliders. +- [ ] Once this is done, it make sense to show a tooltip that displays a pair of values as you hover over the graph. (maybe also the hex?) +- [ ] There should be an add color button that adds the current state of the sliders. +- [ ] A single selection sets the sliders. Multi-select does not +- [ ] Might then make sense to move the graph colorspace, range and background color controls into this area. +- [ ] As a short term improvement, raise the sliders when you click on a color in the graphs. + ## TODOS - [ ] A (default) example showing annotated math stuff diff --git a/src/App.svelte b/src/App.svelte index 9168b631..89353f92 100644 --- a/src/App.svelte +++ b/src/App.svelte @@ -29,6 +29,8 @@ width={450} onColorsChange={(x) => colorStore.setCurrentPalColors(x)} onFocusedColorsChange={(x) => focusStore.setColors(x)} + startDragging={() => colorStore.pausePersistance()} + stopDragging={() => colorStore.resumePersistance()} /> diff --git a/src/components/ColorScatterPlot.svelte b/src/components/ColorScatterPlot.svelte index d3274c77..4dfd3a8e 100644 --- a/src/components/ColorScatterPlot.svelte +++ b/src/components/ColorScatterPlot.svelte @@ -22,6 +22,8 @@ export let height = 256; export let onColorsChange: (color: Color[]) => void; export let onFocusedColorsChange: (color: number[]) => void; + export let startDragging: () => void; + export let stopDragging: () => void; export let colorSpace: any; $: selectedBlindType = $navStore.colorSim; @@ -126,6 +128,8 @@ let isPointDrag = false; const startDrag = (isXYDrag: boolean, idx?: number) => (e: any) => { if (scatterPlotMode !== "moving") return; + + startDragging(); const targetIsPoint = typeof idx === "number"; let target = e.target; isPointDrag = false; @@ -174,6 +178,7 @@ }; const rectMoveEnd = (isZ: boolean) => (e: any) => { + stopDragging(); if (scatterPlotMode !== "moving") return; if (!isPointDrag && dragBox && dragging) { (isZ ? selectColorsFromDragZ : selectColorsFromDrag)(dragBox, dragging); diff --git a/src/stores/color-store.ts b/src/stores/color-store.ts index 2447101f..f18bc11f 100644 --- a/src/stores/color-store.ts +++ b/src/stores/color-store.ts @@ -147,15 +147,30 @@ function createStore() { const { subscribe, set, update } = writable(storeData); let undoStack: StoreData[] = []; let redoStack: StoreData[] = []; + // special logic to enable not capturing too many steps via dragging + let pausePersistance = false; + let lastStore: StoreData = storeData; + const save = (store: StoreData) => + localStorage.setItem( + "color-pal", + JSON.stringify(convertStoreColorToHex(store)) + ); const persistUpdate = (updateFunc: (old: StoreData) => StoreData) => update((oldStore) => { + console.log( + "persist update", + pausePersistance, + undoStack.length, + redoStack.length + ); + if (pausePersistance) { + lastStore = oldStore; + return updateFunc(oldStore); + } undoStack.push(oldStore); redoStack = []; const newVal: StoreData = updateFunc(oldStore); - localStorage.setItem( - "color-pal", - JSON.stringify(convertStoreColorToHex(newVal)) - ); + save(newVal); return newVal; }); const palUp = (updateFunc: (old: Palette) => Palette) => @@ -170,20 +185,40 @@ function createStore() { const doSort = (comparator: (a: Color, b: Color) => number) => () => palUp((n) => ({ ...n, colors: n.colors.sort(comparator) })); + const saveUpdate = (updateFunc: (old: StoreData) => StoreData) => + update((oldStore) => { + const newVal = updateFunc(oldStore); + save(newVal); + return newVal; + }); + return { subscribe, undo: () => - simpleUpdate((currentVal) => { + saveUpdate((currentVal) => { if (undoStack.length === 0) return currentVal; redoStack.push(currentVal); return undoStack.pop()!; }), redo: () => - simpleUpdate((currentVal) => { + saveUpdate((currentVal) => { if (redoStack.length === 0) return currentVal; undoStack.push(currentVal); return redoStack.pop()!; }), + pausePersistance: () => + simpleUpdate((currentVal) => { + lastStore = currentVal; + undoStack.push(currentVal); + redoStack = []; + pausePersistance = true; + return currentVal; + }), + resumePersistance: () => { + pausePersistance = false; + persistUpdate(() => lastStore); + undoStack.pop(); + }, setPalettes: simpleSet("palettes"), setCurrentPal: simpleSet("currentPal"),