Split widget callbacks into update and commit so only the latter adds a history state (#1584)

* feat: split commit and update layout

* feat: add on_commit callback

* Code review

* fix: refactor

---------

Co-authored-by: Keavon Chambers <keavon@keavon.com>

frontend/src/components/floating-menus/ColorPicker.svelte

@@ -31,7 +31,7 @@
 
 	const editor = getContext<Editor>("editor");
 
-	const dispatch = createEventDispatcher<{ color: Color }>();
+	const dispatch = createEventDispatcher<{ color: Color; start: undefined }>();
 
 	export let color: Color;
 	export let allowNone = false;
@@ -149,6 +149,8 @@
 	function addEvents() {
 		document.addEventListener("pointermove", onPointerMove);
 		document.addEventListener("pointerup", onPointerUp);
+
+		dispatch("start");
 	}
 
 	function removeEvents() {

frontend/src/components/widgets/WidgetSpan.svelte

@@ -57,8 +57,16 @@
 		return widgets;
 	}
 
-	function updateLayout(index: number, value: unknown) {
-		editor.instance.updateLayout(layoutTarget, widgets[index].widgetId, value);
+	function widgetValueCommit(index: number, value: unknown) {
+		editor.instance.widgetValueCommit(layoutTarget, widgets[index].widgetId, value);
+	}
+
+	function widgetValueUpdate(index: number, value: unknown) {
+		editor.instance.widgetValueUpdate(layoutTarget, widgets[index].widgetId, value);
+	}
+
+	function widgetValueCommitAndUpdate(index: number, value: unknown) {
+		editor.instance.widgetValueCommitAndUpdate(layoutTarget, widgets[index].widgetId, value);
 	}
 
 	// TODO: This seems to work, but verify the correctness and terseness of this, it's adapted from https://stackoverflow.com/a/67434028/775283
@@ -76,31 +84,31 @@
 	{#each widgets as component, index}
 		{@const checkboxInput = narrowWidgetProps(component.props, "CheckboxInput")}
 		{#if checkboxInput}
-			<CheckboxInput {...exclude(checkboxInput)} on:checked={({ detail }) => updateLayout(index, detail)} />
+			<CheckboxInput {...exclude(checkboxInput)} on:checked={({ detail }) => widgetValueCommitAndUpdate(index, detail)} />
 		{/if}
 		{@const colorInput = narrowWidgetProps(component.props, "ColorButton")}
 		{#if colorInput}
-			<ColorButton {...exclude(colorInput)} on:value={({ detail }) => updateLayout(index, detail)} />
+			<ColorButton {...exclude(colorInput)} on:value={({ detail }) => widgetValueUpdate(index, detail)} on:startHistoryTransaction={() => widgetValueCommit(index, colorInput.value)} />
 		{/if}
 		{@const curvesInput = narrowWidgetProps(component.props, "CurveInput")}
 		{#if curvesInput}
-			<CurveInput {...exclude(curvesInput)} on:value={({ detail }) => debouncer((value) => updateLayout(index, value), { debounceTime: 120 }).debounceUpdateValue(detail)} />
+			<CurveInput {...exclude(curvesInput)} on:value={({ detail }) => debouncer((value) => widgetValueCommitAndUpdate(index, value), { debounceTime: 120 }).debounceUpdateValue(detail)} />
 		{/if}
 		{@const dropdownInput = narrowWidgetProps(component.props, "DropdownInput")}
 		{#if dropdownInput}
-			<DropdownInput {...exclude(dropdownInput)} on:selectedIndex={({ detail }) => updateLayout(index, detail)} />
+			<DropdownInput {...exclude(dropdownInput)} on:selectedIndex={({ detail }) => widgetValueCommitAndUpdate(index, detail)} />
 		{/if}
 		{@const fontInput = narrowWidgetProps(component.props, "FontInput")}
 		{#if fontInput}
-			<FontInput {...exclude(fontInput)} on:changeFont={({ detail }) => updateLayout(index, detail)} />
+			<FontInput {...exclude(fontInput)} on:changeFont={({ detail }) => widgetValueCommitAndUpdate(index, detail)} />
 		{/if}
 		{@const parameterExposeButton = narrowWidgetProps(component.props, "ParameterExposeButton")}
 		{#if parameterExposeButton}
-			<ParameterExposeButton {...exclude(parameterExposeButton)} action={() => updateLayout(index, undefined)} />
+			<ParameterExposeButton {...exclude(parameterExposeButton)} action={() => widgetValueCommitAndUpdate(index, undefined)} />
 		{/if}
 		{@const iconButton = narrowWidgetProps(component.props, "IconButton")}
 		{#if iconButton}
-			<IconButton {...exclude(iconButton)} action={() => updateLayout(index, undefined)} />
+			<IconButton {...exclude(iconButton)} action={() => widgetValueCommitAndUpdate(index, undefined)} />
 		{/if}
 		{@const iconLabel = narrowWidgetProps(component.props, "IconLabel")}
 		{#if iconLabel}
@@ -114,14 +122,15 @@
 		{#if numberInput}
 			<NumberInput
 				{...exclude(numberInput)}
-				on:value={({ detail }) => debouncer((value) => updateLayout(index, value)).debounceUpdateValue(detail)}
-				incrementCallbackIncrease={() => updateLayout(index, "Increment")}
-				incrementCallbackDecrease={() => updateLayout(index, "Decrement")}
+				on:value={({ detail }) => debouncer((value) => widgetValueUpdate(index, value)).debounceUpdateValue(detail)}
+				on:startHistoryTransaction={() => widgetValueCommit(index, numberInput.value)}
+				incrementCallbackIncrease={() => widgetValueCommitAndUpdate(index, "Increment")}
+				incrementCallbackDecrease={() => widgetValueCommitAndUpdate(index, "Decrement")}
 			/>
 		{/if}
 		{@const pivotInput = narrowWidgetProps(component.props, "PivotInput")}
 		{#if pivotInput}
-			<PivotInput {...exclude(pivotInput)} on:position={({ detail }) => updateLayout(index, detail)} />
+			<PivotInput {...exclude(pivotInput)} on:position={({ detail }) => widgetValueCommitAndUpdate(index, detail)} />
 		{/if}
 		{@const popoverButton = narrowWidgetProps(component.props, "PopoverButton")}
 		{#if popoverButton}
@@ -136,7 +145,7 @@
 		{/if}
 		{@const radioInput = narrowWidgetProps(component.props, "RadioInput")}
 		{#if radioInput}
-			<RadioInput {...exclude(radioInput)} on:selectedIndex={({ detail }) => updateLayout(index, detail)} />
+			<RadioInput {...exclude(radioInput)} on:selectedIndex={({ detail }) => widgetValueCommitAndUpdate(index, detail)} />
 		{/if}
 		{@const separator = narrowWidgetProps(component.props, "Separator")}
 		{#if separator}
@@ -148,19 +157,19 @@
 		{/if}
 		{@const textAreaInput = narrowWidgetProps(component.props, "TextAreaInput")}
 		{#if textAreaInput}
-			<TextAreaInput {...exclude(textAreaInput)} on:commitText={({ detail }) => updateLayout(index, detail)} />
+			<TextAreaInput {...exclude(textAreaInput)} on:commitText={({ detail }) => widgetValueCommitAndUpdate(index, detail)} />
 		{/if}
 		{@const textButton = narrowWidgetProps(component.props, "TextButton")}
 		{#if textButton}
-			<TextButton {...exclude(textButton)} action={() => updateLayout(index, undefined)} />
+			<TextButton {...exclude(textButton)} action={() => widgetValueCommitAndUpdate(index, undefined)} />
 		{/if}
 		{@const breadcrumbTrailButtons = narrowWidgetProps(component.props, "BreadcrumbTrailButtons")}
 		{#if breadcrumbTrailButtons}
-			<BreadcrumbTrailButtons {...exclude(breadcrumbTrailButtons)} action={(index) => updateLayout(index, index)} />
+			<BreadcrumbTrailButtons {...exclude(breadcrumbTrailButtons)} action={(index) => widgetValueCommitAndUpdate(index, index)} />
 		{/if}
 		{@const textInput = narrowWidgetProps(component.props, "TextInput")}
 		{#if textInput}
-			<TextInput {...exclude(textInput)} on:commitText={({ detail }) => updateLayout(index, detail)} />
+			<TextInput {...exclude(textInput)} on:commitText={({ detail }) => widgetValueCommitAndUpdate(index, detail)} />
 		{/if}
 		{@const textLabel = narrowWidgetProps(component.props, "TextLabel")}
 		{#if textLabel}

frontend/src/components/widgets/buttons/ColorButton.svelte

@@ -7,7 +7,7 @@
 	import LayoutCol from "@graphite/components/layout/LayoutCol.svelte";
 	import TextLabel from "@graphite/components/widgets/labels/TextLabel.svelte";
 
-	const dispatch = createEventDispatcher<{ value: Color }>();
+	const dispatch = createEventDispatcher<{ value: Color; startHistoryTransaction: undefined }>();
 
 	let open = false;
 
@@ -31,6 +31,11 @@
 			value = detail;
 			dispatch("value", detail);
 		}}
+		on:startHistoryTransaction={() => {
+			// This event is sent to the backend so it knows to start a transaction for the history system. See discussion for some explanation:
+			// <https://github.com/GraphiteEditor/Graphite/pull/1584#discussion_r1477592483>
+			dispatch("startHistoryTransaction");
+		}}
 		{allowNone}
 	/>
 </LayoutCol>

frontend/src/components/widgets/inputs/NumberInput.svelte

@@ -11,7 +11,7 @@
 	const BUTTON_LEFT = 0;
 	const BUTTON_RIGHT = 2;
 
-	const dispatch = createEventDispatcher<{ value: number | undefined }>();
+	const dispatch = createEventDispatcher<{ value: number | undefined; startHistoryTransaction: undefined }>();
 
 	// Label
 	export let label: string | undefined = undefined;
@@ -293,6 +293,9 @@
 		initialValueBeforeDragging = value;
 		cumulativeDragDelta = 0;
 
+		// Tell the backend that we are beginning a transaction for the history system
+		startDragging();
+
 		// We ignore the first event invocation's `e.movementX` value because it's unreliable.
 		// In both Chrome and Firefox (tested on Windows 10), the first `e.movementX` value is occasionally a very large number
 		// (around positive 1000, even if movement was in the negative direction). This seems to happen more often if the movement is rapid.
@@ -430,6 +433,9 @@
 			// We're dragging now, so that's the new state.
 			rangeSliderClickDragState = "Dragging";
 
+			// Tell the backend that we are beginning a transaction for the history system
+			startDragging();
+
 			// We want to begin watching for an abort while dragging the slider.
 			addEventListener("pointermove", sliderAbortFromDragging);
 			addEventListener("keydown", sliderAbortFromDragging);
@@ -474,6 +480,12 @@
 		removeEventListener("keydown", sliderAbortFromDragging);
 	}
 
+	function startDragging() {
+		// This event is sent to the backend so it knows to start a transaction for the history system. See discussion for some explanation:
+		// <https://github.com/GraphiteEditor/Graphite/pull/1584#discussion_r1477592483>
+		dispatch("startHistoryTransaction");
+	}
+
 	// We want to let the user abort while dragging the slider by right clicking or pressing Escape.
 	// This function also helps recover and clean up if the window loses focus while dragging the slider.
 	// Since we reuse the function for both the "pointermove" and "keydown" events, it is split into parts that only run for a `PointerEvent` or `KeyboardEvent`.

frontend/src/components/window/title-bar/TitleBar.svelte

@@ -54,7 +54,7 @@
 				...entry,
 
 				// Shared names with fields that need to be converted from the type used in `MenuBarEntry` to that of `MenuListEntry`
-				action: () => editor.instance.updateLayout(updateMenuBarLayout.layoutTarget, entry.action.widgetId, undefined),
+				action: () => editor.instance.widgetValueCommitAndUpdate(updateMenuBarLayout.layoutTarget, entry.action.widgetId, undefined),
 				children: entry.children ? entry.children.map((entries) => entries.map((entry) => menuBarEntryToMenuListEntry(entry))) : undefined,
 
 				// New fields in `MenuListEntry`