DropdownInput preview support and ColorButton history improvements (#1598)

* DropdownInput support preview

* fix typo and rm logs

* Add previewable flag

* fix cr typos

* Improve color button history

* rename

* update dropdown preview behaviour

* Color picker preset color

* Another way to handle blend mode preview

* Apply suggestions from code review

* Use on_commit instead of on_update for some dropdowns

* Debugging progress

* add debug

* active not equal to highlight in some cases

* rm logs

---------

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

editor/src/messages/dialog/export_dialog/export_dialog_message_handler.rs

@@ -103,7 +103,7 @@ impl LayoutHolder for ExportDialogMessageHandler {
 			.map(|(val, name, disabled)| {
 				MenuListEntry::new(format!("{val:?}"))
 					.label(name)
-					.on_update(move |_| ExportDialogMessage::ExportBounds(val).into())
+					.on_commit(move |_| ExportDialogMessage::ExportBounds(val).into())
 					.disabled(disabled)
 			})
 			.collect()];

editor/src/messages/layout/layout_message_handler.rs

@@ -121,11 +121,11 @@ impl LayoutMessageHandler {
 			Widget::DropdownInput(dropdown_input) => {
 				let callback_message = match action {
 					WidgetValueAction::Commit => {
-						let update_value = value.as_u64().expect("DropdownInput commit was not of type: u64");
+						let update_value = value.as_u64().expect(&format!("DropdownInput commit was not of type `u64`, found {value:?}"));
 						(dropdown_input.entries.iter().flatten().nth(update_value as usize).unwrap().on_commit.callback)(&())
 					}
 					WidgetValueAction::Update => {
-						let update_value = value.as_u64().expect("DropdownInput update was not of type: u64");
+						let update_value = value.as_u64().expect(&format!("DropdownInput update was not of type `u64`, found {value:?}"));
 						dropdown_input.selected_index = Some(update_value as u32);
 						(dropdown_input.entries.iter().flatten().nth(update_value as usize).unwrap().on_update.callback)(&())
 					}
@@ -174,32 +174,26 @@ impl LayoutMessageHandler {
 
 				responses.add(callback_message);
 			}
-			Widget::NumberInput(number_input) => {
+			Widget::NumberInput(number_input) => match action {
-				match action {
+				WidgetValueAction::Commit => {
-					WidgetValueAction::Commit => {
+					let callback_message = (number_input.on_commit.callback)(&());
-						let callback_message = (number_input.on_commit.callback)(&());
+					responses.add(callback_message);
+				}
+				WidgetValueAction::Update => match value {
+					Value::Number(num) => {
+						let update_value = num.as_f64().unwrap();
+						number_input.value = Some(update_value);
+						let callback_message = (number_input.on_update.callback)(number_input);
 						responses.add(callback_message);
 					}
-					WidgetValueAction::Update => {
+					Value::String(str) => match str.as_str() {
-						match value {
+						"Increment" => responses.add((number_input.increment_callback_increase.callback)(number_input)),
-							Value::Number(num) => {
+						"Decrement" => responses.add((number_input.increment_callback_decrease.callback)(number_input)),
-								let update_value = num.as_f64().unwrap();
+						_ => panic!("Invalid string found when updating `NumberInput`"),
-								number_input.value = Some(update_value);
+					},
-								let callback_message = (number_input.on_update.callback)(number_input);
+					_ => {}
-								responses.add(callback_message);
+				},
-							}
+			},
-							Value::String(str) => match str.as_str() {
-								"Increment" => responses.add((number_input.increment_callback_increase.callback)(number_input)),
-								"Decrement" => responses.add((number_input.increment_callback_decrease.callback)(number_input)),
-								_ => {
-									panic!("Invalid string found when updating `NumberInput`")
-								}
-							},
-							_ => {} // If it's some other type we could just ignore it and leave the value as is
-						}
-					}
-				}
-			}
 			Widget::ParameterExposeButton(parameter_expose_button) => {
 				let callback_message = match action {
 					WidgetValueAction::Commit => (parameter_expose_button.on_commit.callback)(&()),

editor/src/messages/portfolio/document/document_message_handler.rs

@@ -690,7 +690,6 @@ impl MessageHandler<DocumentMessage, DocumentMessageData<'_>> for DocumentMessag
 				}
 			}
 			DocumentMessage::SetBlendModeForSelectedLayers { blend_mode } => {
-				self.backup(responses);
 				for layer in self.selected_nodes.selected_layers_except_artboards(self.metadata()) {
 					responses.add(GraphOperationMessage::BlendModeSet { layer, blend_mode });
 				}
@@ -1183,15 +1182,15 @@ impl DocumentMessageHandler {
 						MenuListEntry::new(format!("{:?}", DocumentMode::SelectMode))
 							.label(DocumentMode::SelectMode.to_string())
 							.icon(DocumentMode::SelectMode.icon_name())
-							.on_update(|_| DialogMessage::RequestComingSoonDialog { issue: Some(330) }.into()),
+							.on_commit(|_| DialogMessage::RequestComingSoonDialog { issue: Some(330) }.into()),
 						MenuListEntry::new(format!("{:?}", DocumentMode::GuideMode))
 							.label(DocumentMode::GuideMode.to_string())
 							.icon(DocumentMode::GuideMode.icon_name())
-							.on_update(|_| DialogMessage::RequestComingSoonDialog { issue: Some(331) }.into()),
+							.on_commit(|_| DialogMessage::RequestComingSoonDialog { issue: Some(331) }.into()),
 					]])
 					.selected_index(Some(self.document_mode as u32))
-					.draw_icon( true)
+					.draw_icon(true)
-					.interactive( false) // TODO: set to true when dialogs are not spawned
+					.interactive(false) // TODO: set to true when dialogs are not spawned
 					.widget_holder(),
 				Separator::new(SeparatorType::Section).widget_holder(),
 			],
@@ -1533,6 +1532,7 @@ impl DocumentMessageHandler {
 						MenuListEntry::new(format!("{blend_mode:?}"))
 							.label(blend_mode.to_string())
 							.on_update(move |_| DocumentMessage::SetBlendModeForSelectedLayers { blend_mode }.into())
+							.on_commit(|_| DocumentMessage::StartTransaction.into())
 					})
 					.collect()
 			})

editor/src/messages/portfolio/document/node_graph/node_properties.rs

@@ -373,7 +373,7 @@ fn number_widget(document_node: &DocumentNode, node_id: NodeId, index: usize, na
 	widgets
 }
 
-//TODO Generalize this instead of using a separate function per dropdown menu enum
+// TODO: Generalize this instead of using a separate function per dropdown menu enum
 fn color_channel(document_node: &DocumentNode, node_id: NodeId, index: usize, name: &str, blank_assist: bool) -> LayoutGroup {
 	let mut widgets = start_widgets(document_node, node_id, index, name, FrontendGraphDataType::General, blank_assist);
 	if let &NodeInput::Value {
@@ -428,7 +428,7 @@ fn rgba_channel(document_node: &DocumentNode, node_id: NodeId, index: usize, nam
 	LayoutGroup::Row { widgets }.with_tooltip("Color Channel")
 }
 
-// TODO Generalize this instead of using a separate function per dropdown menu enum
+// TODO: Generalize this instead of using a separate function per dropdown menu enum
 fn noise_type(document_node: &DocumentNode, node_id: NodeId, index: usize, name: &str, blank_assist: bool) -> LayoutGroup {
 	let mut widgets = start_widgets(document_node, node_id, index, name, FrontendGraphDataType::General, blank_assist);
 	if let &NodeInput::Value {
@@ -454,7 +454,7 @@ fn noise_type(document_node: &DocumentNode, node_id: NodeId, index: usize, name:
 	LayoutGroup::Row { widgets }.with_tooltip("Style of noise pattern")
 }
 
-// TODO Generalize this instead of using a separate function per dropdown menu enum
+// TODO: Generalize this instead of using a separate function per dropdown menu enum
 fn fractal_type(document_node: &DocumentNode, node_id: NodeId, index: usize, name: &str, blank_assist: bool, disabled: bool) -> LayoutGroup {
 	let mut widgets = start_widgets(document_node, node_id, index, name, FrontendGraphDataType::General, blank_assist);
 	if let &NodeInput::Value {
@@ -480,7 +480,7 @@ fn fractal_type(document_node: &DocumentNode, node_id: NodeId, index: usize, nam
 	LayoutGroup::Row { widgets }.with_tooltip("Style of layered levels of the noise pattern")
 }
 
-// TODO Generalize this instead of using a separate function per dropdown menu enum
+// TODO: Generalize this instead of using a separate function per dropdown menu enum
 fn cellular_distance_function(document_node: &DocumentNode, node_id: NodeId, index: usize, name: &str, blank_assist: bool, disabled: bool) -> LayoutGroup {
 	let mut widgets = start_widgets(document_node, node_id, index, name, FrontendGraphDataType::General, blank_assist);
 	if let &NodeInput::Value {
@@ -509,7 +509,7 @@ fn cellular_distance_function(document_node: &DocumentNode, node_id: NodeId, ind
 	LayoutGroup::Row { widgets }.with_tooltip("Distance function used by the cellular noise")
 }
 
-// TODO Generalize this instead of using a separate function per dropdown menu enum
+// TODO: Generalize this instead of using a separate function per dropdown menu enum
 fn cellular_return_type(document_node: &DocumentNode, node_id: NodeId, index: usize, name: &str, blank_assist: bool, disabled: bool) -> LayoutGroup {
 	let mut widgets = start_widgets(document_node, node_id, index, name, FrontendGraphDataType::General, blank_assist);
 	if let &NodeInput::Value {
@@ -535,7 +535,7 @@ fn cellular_return_type(document_node: &DocumentNode, node_id: NodeId, index: us
 	LayoutGroup::Row { widgets }.with_tooltip("Return type of the cellular noise")
 }
 
-// TODO Generalize this instead of using a separate function per dropdown menu enum
+// TODO: Generalize this instead of using a separate function per dropdown menu enum
 fn domain_warp_type(document_node: &DocumentNode, node_id: NodeId, index: usize, name: &str, blank_assist: bool, disabled: bool) -> LayoutGroup {
 	let mut widgets = start_widgets(document_node, node_id, index, name, FrontendGraphDataType::General, blank_assist);
 	if let &NodeInput::Value {

editor/src/messages/tool/tool_messages/brush_tool.rs

@@ -165,7 +165,7 @@ impl LayoutHolder for BrushTool {
 					.map(|blend_mode| {
 						MenuListEntry::new(format!("{blend_mode:?}"))
 							.label(blend_mode.to_string())
-							.on_update(|_| BrushToolMessage::UpdateOptions(BrushToolMessageOptionsUpdate::BlendMode(*blend_mode)).into())
+							.on_commit(|_| BrushToolMessage::UpdateOptions(BrushToolMessageOptionsUpdate::BlendMode(*blend_mode)).into())
 					})
 					.collect()
 			})

editor/src/messages/tool/tool_messages/select_tool.rs

@@ -98,7 +98,7 @@ impl SelectTool {
 			.map(|mode| {
 				MenuListEntry::new(format!("{mode:?}"))
 					.label(mode.to_string())
-					.on_update(move |_| SelectToolMessage::SelectOptions(SelectOptionsUpdate::NestedSelectionBehavior(*mode)).into())
+					.on_commit(move |_| SelectToolMessage::SelectOptions(SelectOptionsUpdate::NestedSelectionBehavior(*mode)).into())
 			})
 			.collect();
 

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

@@ -31,7 +31,7 @@
 
 	const editor = getContext<Editor>("editor");
 
-	const dispatch = createEventDispatcher<{ color: Color; start: undefined }>();
+	const dispatch = createEventDispatcher<{ color: Color; startHistoryTransaction: undefined }>();
 
 	export let color: Color;
 	export let allowNone = false;
@@ -150,7 +150,7 @@
 		document.addEventListener("pointermove", onPointerMove);
 		document.addEventListener("pointerup", onPointerUp);
 
-		dispatch("start");
+		dispatch("startHistoryTransaction");
 	}
 
 	function removeEvents() {
@@ -219,6 +219,7 @@
 	}
 
 	function setColorPreset(preset: PresetColors) {
+		dispatch("startHistoryTransaction");
 		if (preset === "none") {
 			setNewHSVA(0, 0, 0, 1, true);
 			setColor(new Color("none"));
@@ -259,6 +260,7 @@
 		try {
 			// eslint-disable-next-line @typescript-eslint/no-explicit-any
 			const result = await new (window as any).EyeDropper().open();
+			dispatch("startHistoryTransaction");
 			setColorCode(result.sRGBHex);
 		} catch {
 			// Do nothing
@@ -314,7 +316,10 @@
 				<LayoutRow>
 					<TextInput
 						value={newColor.toHexOptionalAlpha() || "-"}
-						on:commitText={({ detail }) => setColorCode(detail)}
+						on:commitText={({ detail }) => {
+							dispatch("startHistoryTransaction");
+							setColorCode(detail);
+						}}
 						centered={true}
 						tooltip={"Color code in hexadecimal format. 6 digits if opaque, 8 with alpha.\nAccepts input of CSS color values including named colors."}
 						bind:this={hexCodeInputWidget}
@@ -335,6 +340,9 @@
 								strength = detail;
 								setColorRGB(channel, detail);
 							}}
+							on:startHistoryTransaction={() => {
+								dispatch("startHistoryTransaction");
+							}}
 							min={0}
 							max={255}
 							minWidth={56}
@@ -359,6 +367,9 @@
 								strength = detail;
 								setColorHSV(channel, detail);
 							}}
+							on:startHistoryTransaction={() => {
+								dispatch("startHistoryTransaction");
+							}}
 							min={0}
 							max={channel === "h" ? 360 : 100}
 							unit={channel === "h" ? "°" : "%"}
@@ -379,6 +390,9 @@
 					if (detail !== undefined) alpha = detail / 100;
 					setColorAlphaPercent(detail);
 				}}
+				on:startHistoryTransaction={() => {
+					dispatch("startHistoryTransaction");
+				}}
 				min={0}
 				max={100}
 				rangeMin={0}

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

@@ -19,7 +19,7 @@
 	let scroller: LayoutCol | undefined;
 	let searchTextInput: TextInput | undefined;
 
-	const dispatch = createEventDispatcher<{ open: boolean; activeEntry: MenuListEntry; naturalWidth: number }>();
+	const dispatch = createEventDispatcher<{ open: boolean; activeEntry: MenuListEntry; hoverInEntry: MenuListEntry; hoverOutEntry: undefined; naturalWidth: number }>();
 
 	export let entries: MenuListEntry[][];
 	export let activeEntry: MenuListEntry | undefined = undefined;
@@ -164,7 +164,10 @@
 	}
 
 	function onEntryPointerEnter(menuListEntry: MenuListEntry) {
-		if (!menuListEntry.children?.length) return;
+		if (!menuListEntry.children?.length) {
+			dispatch("hoverInEntry", menuListEntry);
+			return;
+		}
 
 		let childReference = getChildReference(menuListEntry);
 		if (childReference) {
@@ -174,7 +177,10 @@
 	}
 
 	function onEntryPointerLeave(menuListEntry: MenuListEntry) {
-		if (!menuListEntry.children?.length) return;
+		if (!menuListEntry.children?.length) {
+			dispatch("hoverOutEntry");
+			return;
+		}
 
 		let childReference = getChildReference(menuListEntry);
 		if (childReference) {
@@ -346,9 +352,9 @@
 		highlighted = newHighlight;
 
 		// Interactive menus should keep the active entry the same as the highlighted one
-		if (interactive && newHighlight?.value !== activeEntry?.value && newHighlight) {
+		// if (interactive && newHighlight?.value !== activeEntry?.value && newHighlight) {
-			dispatch("activeEntry", newHighlight);
+		// 	dispatch("activeEntry", newHighlight);
-		}
+		// }
 
 		// Scroll into view
 		let container = scroller?.div?.();

frontend/src/components/widgets/WidgetSpan.svelte

@@ -96,7 +96,16 @@
 		{/if}
 		{@const dropdownInput = narrowWidgetProps(component.props, "DropdownInput")}
 		{#if dropdownInput}
-			<DropdownInput {...exclude(dropdownInput)} on:selectedIndex={({ detail }) => widgetValueCommitAndUpdate(index, detail)} />
+			<DropdownInput
+				{...exclude(dropdownInput)}
+				on:hoverInEntry={({ detail }) => {
+					return widgetValueUpdate(index, detail);
+				}}
+				on:hoverOutEntry={({ detail }) => {
+					return widgetValueUpdate(index, detail);
+				}}
+				on:selectedIndex={({ detail }) => widgetValueCommitAndUpdate(index, detail)}
+			/>
 		{/if}
 		{@const fontInput = narrowWidgetProps(component.props, "FontInput")}
 		{#if fontInput}

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

@@ -10,7 +10,7 @@
 
 	const DASH_ENTRY = { value: "", label: "-" };
 
-	const dispatch = createEventDispatcher<{ selectedIndex: number }>();
+	const dispatch = createEventDispatcher<{ selectedIndex: number; hoverInEntry: number; hoverOutEntry: number }>();
 
 	let menuList: MenuList | undefined;
 	let self: LayoutRow | undefined;
@@ -24,11 +24,17 @@
 
 	let activeEntry = makeActiveEntry();
 	let activeEntrySkipWatcher = false;
+	let initialSelectedIndex: number | undefined = undefined;
 	let open = false;
 	let minWidth = 0;
 
 	$: watchSelectedIndex(selectedIndex);
 	$: watchActiveEntry(activeEntry);
+	$: watchOpen(open);
+
+	function watchOpen(open: boolean) {
+		initialSelectedIndex = open ? selectedIndex : undefined;
+	}
 
 	// Called only when `selectedIndex` is changed from outside this component
 	function watchSelectedIndex(_?: number) {
@@ -41,10 +47,20 @@
 		if (activeEntrySkipWatcher) {
 			activeEntrySkipWatcher = false;
 		} else if (activeEntry !== DASH_ENTRY) {
+			// We need to set to the initial value first to track a right history step, as if we hover in initial selection.
+			dispatch("hoverInEntry", initialSelectedIndex);
 			dispatch("selectedIndex", entries.flat().indexOf(activeEntry));
 		}
 	}
 
+	function dispatchHoverInEntry(hoveredEntry: MenuListEntry) {
+		dispatch("hoverInEntry", entries.flat().indexOf(hoveredEntry));
+	}
+
+	function dispatchHoverOutEntry() {
+		dispatch("hoverOutEntry", initialSelectedIndex);
+	}
+
 	function makeActiveEntry(): MenuListEntry {
 		const allEntries = entries.flat();
 
@@ -81,6 +97,8 @@
 		on:naturalWidth={({ detail }) => (minWidth = detail)}
 		{activeEntry}
 		on:activeEntry={({ detail }) => (activeEntry = detail)}
+		on:hoverInEntry={({ detail }) => dispatchHoverInEntry(detail)}
+		on:hoverOutEntry={() => dispatchHoverOutEntry()}
 		{open}
 		on:open={({ detail }) => (open = detail)}
 		{entries}

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

@@ -200,6 +200,11 @@
 
 		let newValue = evaluateMathExpression(textWithLeadingZeroes);
 		if (newValue !== undefined && isNaN(newValue)) newValue = undefined; // Rejects `sqrt(-1)`
+
+		if (newValue !== undefined) {
+			const oldValue = value !== undefined && isInteger ? Math.round(value) : value;
+			if (newValue !== oldValue) dispatch("startHistoryTransaction");
+		}
 		updateValue(newValue);
 
 		editing = false;