Fix hover transfer bugs occurring with the color picker popover (#4146)

* Fix hover transfer bugs occurring with the color picker popover

* Code review fix

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

@@ -4,7 +4,7 @@
 	import LayoutCol from "/src/components/layout/LayoutCol.svelte";
 	import LayoutRow from "/src/components/layout/LayoutRow.svelte";
 	import WidgetLayout from "/src/components/widgets/WidgetLayout.svelte";
-	import type { ColorPickerStore } from "/src/stores/color-picker";
+	import type { ColorPickerCallbacks, ColorPickerStore } from "/src/stores/color-picker";
 	import type { EditorWrapper, FillChoice, MenuDirection } from "/wrapper/pkg/graphite_wasm_wrapper";
 
 	const dispatch = createEventDispatcher<{ colorOrGradient: FillChoice; startHistoryTransaction: undefined; commitHistoryTransaction: undefined }>();
@@ -27,15 +27,18 @@
 	// Open/close lifecycle: when `open` flips, register/clear the global callbacks (so events route to *this* instance)
 	// and tell the Rust handler to (re)initialize its state from the current `colorOrGradient`.
 	let lastOpen = false;
+	// Identity used by `clearCallbacks` to skip stale clears when another picker has already taken over the store's callbacks
+	let installedCallbacks: ColorPickerCallbacks | undefined;
 	$: handleOpenChange(open);
 
 	function handleOpenChange(isOpen: boolean) {
 		if (isOpen && !lastOpen) {
-			colorPickerStore.setCallbacks({
+			installedCallbacks = {
 				onColorChanged: (value) => dispatch("colorOrGradient", value),
 				onStartTransaction: () => dispatch("startHistoryTransaction"),
 				onCommitTransaction: () => dispatch("commitHistoryTransaction"),
-			});
+			};
+			colorPickerStore.setCallbacks(installedCallbacks);
 			editor.openColorPicker(colorOrGradient, allowNone, disabled);
 			// Auto-select the hex color code text input. Deferred so the layout has time to render after the picker opens.
 			setTimeout(() => {
@@ -43,7 +46,8 @@
 				if (hexInput instanceof HTMLInputElement) hexInput.select();
 			}, 0);
 		} else if (!isOpen && lastOpen) {
-			colorPickerStore.clearCallbacks();
+			if (installedCallbacks) colorPickerStore.clearCallbacks(installedCallbacks);
+			installedCallbacks = undefined;
 			editor.closeColorPicker();
 		}
 		lastOpen = isOpen;
@@ -55,7 +59,8 @@
 
 	onDestroy(() => {
 		if (!lastOpen) return;
-		colorPickerStore.clearCallbacks();
+		if (installedCallbacks) colorPickerStore.clearCallbacks(installedCallbacks);
+		installedCallbacks = undefined;
 		editor.closeColorPicker();
 	});
 </script>

frontend/src/components/layout/FloatingMenu.svelte

@@ -336,7 +336,7 @@
 
 		// HOVER TRANSFER
 		// Transfer from this open floating menu to a sibling floating menu if the pointer hovers to a valid neighboring floating menu spawner
-		hoverTransfer(self, ownSpawner, targetSpawner);
+		if (strayCloses) hoverTransfer(self, ownSpawner, targetSpawner);
 
 		// POINTER STRAY
 		// Close the floating menu if the pointer has strayed far enough from its bounds (and it's not hovering over its own spawner)

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

@@ -40,10 +40,16 @@
 		// The `unFocus()` call in `onTextChangeCanceled()` causes itself to be run again, so this if statement skips a second run
 		if (!editing) return;
 
+		// Capture before `onTextChangeCanceled` blurs the input
+		const currentValue = self?.getValue();
+
 		onTextChangeCanceled();
 
-		// TODO: Find a less hacky way to do this
-		if (self) dispatch("commitText", self.getValue());
+		// Only commit on a real edit, so a blur fired when the focused input is removed from the DOM (e.g., from a picker closing
+		// during hover transfer) doesn't round-trip the original value back to the backend and overwrite concurrent state.
+		if (self && currentValue !== undefined && currentValue !== value) {
+			dispatch("commitText", currentValue);
+		}
 
 		// Required if value is not changed by the parent component upon update:value event
 		self?.setInputElementValue(self.getValue());

frontend/src/stores/color-picker.ts

@@ -35,7 +35,8 @@ const { subscribe, update } = store;
 export type ColorPickerStore = {
 	subscribe: typeof subscribe;
 	setCallbacks: (callbacks: ColorPickerCallbacks) => void;
-	clearCallbacks: () => void;
+	// Identity-checked so a hover-transfer race where another picker's `setCallbacks` already replaced these doesn't clobber the new picker's callbacks
+	clearCallbacks: (expected: ColorPickerCallbacks) => void;
 	setDragging: (dragging: boolean) => void;
 };
 
@@ -88,10 +89,12 @@ export function createColorPickerStore(subscriptions: SubscriptionsRouter): Colo
 				return state;
 			});
 		},
-		clearCallbacks: () => {
+		clearCallbacks: (expected: ColorPickerCallbacks) => {
 			update((state) => {
-				state.callbacks = {};
 				state.isDragging = false;
+				if (state.callbacks === expected) {
+					state.callbacks = {};
+				}
 				return state;
 			});
 		},