Improve UX of importing vs. opening files (#3661)

* wip

* fix drag and drop

* fix

* fix tests

* fix tests

* fix warning

* Partial code review

* add dialog

* fix web

* fix web

* push back release candidate expiry

* Code review

* Reduce code duplication for pasting files in frontend

---------

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

editor/src/dispatcher.rs

@@ -571,10 +571,9 @@ mod test {
 				"Demo artwork '{document_name}' has more than 1 line (remember to open and re-save it in Graphite)",
 			);
 
-			let responses = editor.editor.handle_message(PortfolioMessage::OpenDocumentFile {
-				document_name: Some(document_name.to_string()),
-				document_path: None,
-				document_serialized_content,
+			let responses = editor.editor.handle_message(PortfolioMessage::OpenFile {
+				path: file_name.into(),
+				content: document_serialized_content.bytes().collect(),
 			});
 
 			// Check if the graph renders

editor/src/messages/frontend/frontend_message.rs

@@ -107,7 +107,6 @@ pub enum FrontendMessage {
 		font: Font,
 		url: String,
 	},
-	TriggerImport,
 	TriggerPersistenceRemoveDocument {
 		#[serde(rename = "documentId")]
 		document_id: DocumentId,
@@ -122,7 +121,8 @@ pub enum FrontendMessage {
 	TriggerLoadRestAutoSaveDocuments,
 	TriggerOpenLaunchDocuments,
 	TriggerLoadPreferences,
-	TriggerOpenDocument,
+	TriggerOpen,
+	TriggerImport,
 	TriggerSavePreferences {
 		preferences: PreferencesMessageHandler,
 	},

editor/src/messages/input_mapper/input_mappings.rs

@@ -442,7 +442,7 @@ pub fn input_mappings(zoom_with_scroll: bool) -> Mapping {
 		entry!(KeyDown(Tab); modifiers=[Control, Shift], action_dispatch=PortfolioMessage::PrevDocument),
 		entry!(KeyDown(KeyW); modifiers=[Accel], action_dispatch=PortfolioMessage::CloseActiveDocumentWithConfirmation),
 		entry!(KeyDown(KeyW); modifiers=[Accel, Alt], action_dispatch=PortfolioMessage::CloseAllDocumentsWithConfirmation),
-		entry!(KeyDown(KeyO); modifiers=[Accel], action_dispatch=PortfolioMessage::OpenDocument),
+		entry!(KeyDown(KeyO); modifiers=[Accel], action_dispatch=PortfolioMessage::Open),
 		entry!(KeyDown(KeyI); modifiers=[Accel], action_dispatch=PortfolioMessage::Import),
 		entry!(KeyDown(KeyX); modifiers=[Accel], action_dispatch=PortfolioMessage::Cut { clipboard: Clipboard::Device }),
 		entry!(KeyDown(KeyC); modifiers=[Accel], action_dispatch=PortfolioMessage::Copy { clipboard: Clipboard::Device }),

editor/src/messages/menu_bar/menu_bar_message_handler.rs

@@ -120,8 +120,8 @@ impl LayoutHolder for MenuBarMessageHandler {
 						MenuListEntry::new("Open…")
 							.label("Open…")
 							.icon("Folder")
-							.tooltip_shortcut(action_shortcut!(PortfolioMessageDiscriminant::OpenDocument))
-							.on_commit(|_| PortfolioMessage::OpenDocument.into()),
+							.tooltip_shortcut(action_shortcut!(PortfolioMessageDiscriminant::Open))
+							.on_commit(|_| PortfolioMessage::Open.into()),
 						MenuListEntry::new("Open Demo Artwork…")
 							.label("Open Demo Artwork…")
 							.icon("Image")
@@ -161,7 +161,8 @@ impl LayoutHolder for MenuBarMessageHandler {
 							.label("Import…")
 							.icon("FileImport")
 							.tooltip_shortcut(action_shortcut!(PortfolioMessageDiscriminant::Import))
-							.on_commit(|_| PortfolioMessage::Import.into()),
+							.on_commit(|_| PortfolioMessage::Import.into())
+							.disabled(no_active_document),
 						MenuListEntry::new("Export…")
 							.label("Export…")
 							.icon("FileExport")

editor/src/messages/portfolio/portfolio_message.rs

@@ -58,7 +58,6 @@ pub enum PortfolioMessage {
 		font_style: String,
 		data: Vec<u8>,
 	},
-	Import,
 	LoadDocumentResources {
 		document_id: DocumentId,
 	},
@@ -66,7 +65,16 @@ pub enum PortfolioMessage {
 		name: String,
 	},
 	NextDocument,
-	OpenDocument,
+	Open,
+	Import,
+	OpenFile {
+		path: PathBuf,
+		content: Vec<u8>,
+	},
+	ImportFile {
+		path: PathBuf,
+		content: Vec<u8>,
+	},
 	OpenDocumentFile {
 		document_name: Option<String>,
 		document_path: Option<PathBuf>,
@@ -82,11 +90,13 @@ pub enum PortfolioMessage {
 		to_front: bool,
 		select_after_open: bool,
 	},
-	ToggleResetNodesToDefinitionsOnOpen,
-	PasteIntoFolder {
-		clipboard: Clipboard,
-		parent: LayerNodeIdentifier,
-		insert_index: usize,
+	OpenImage {
+		name: Option<String>,
+		image: Image<Color>,
+	},
+	OpenSvg {
+		name: Option<String>,
+		svg: String,
 	},
 	PasteSerializedData {
 		data: String,
@@ -94,9 +104,6 @@ pub enum PortfolioMessage {
 	PasteSerializedVector {
 		data: String,
 	},
-	CenterPastedLayers {
-		layers: Vec<LayerNodeIdentifier>,
-	},
 	PasteImage {
 		name: Option<String>,
 		image: Image<Color>,
@@ -109,6 +116,15 @@ pub enum PortfolioMessage {
 		mouse: Option<(f64, f64)>,
 		parent_and_insert_index: Option<(LayerNodeIdentifier, usize)>,
 	},
+	// TODO: Unused except by tests, remove?
+	PasteIntoFolder {
+		clipboard: Clipboard,
+		parent: LayerNodeIdentifier,
+		insert_index: usize,
+	},
+	CenterPastedLayers {
+		layers: Vec<LayerNodeIdentifier>,
+	},
 	PrevDocument,
 	RequestWelcomeScreenButtonsLayout,
 	RequestStatusBarInfoLayout,
@@ -132,6 +148,7 @@ pub enum PortfolioMessage {
 		document_id: DocumentId,
 		ignore_hash: bool,
 	},
+	ToggleResetNodesToDefinitionsOnOpen,
 	ToggleDataPanelOpen,
 	TogglePropertiesPanelOpen,
 	ToggleLayersPanelOpen,

editor/src/messages/portfolio/portfolio_message_handler.rs

@@ -17,6 +17,7 @@ use crate::messages::portfolio::document::utility_types::clipboards::{Clipboard,
 use crate::messages::portfolio::document::utility_types::network_interface::OutputConnector;
 use crate::messages::portfolio::document::utility_types::nodes::SelectedNodes;
 use crate::messages::portfolio::document_migration::*;
+use crate::messages::portfolio::utility_types::FileContent;
 use crate::messages::preferences::SelectionMode;
 use crate::messages::prelude::*;
 use crate::messages::tool::common_functionality::graph_modification_utils;
@@ -27,11 +28,13 @@ use derivative::*;
 use glam::{DAffine2, DVec2};
 use graph_craft::document::NodeId;
 use graphene_std::Color;
+use graphene_std::raster_types::Image;
 use graphene_std::renderer::Quad;
 use graphene_std::subpath::BezierHandles;
 use graphene_std::text::Font;
 use graphene_std::vector::misc::HandleId;
 use graphene_std::vector::{PointId, SegmentId, Vector, VectorModificationType};
+use std::path::PathBuf;
 use std::vec;
 
 #[derive(ExtractField)]
@@ -426,10 +429,6 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageContext<'_>> for Portfolio
 				}
 			}
 			PortfolioMessage::EditorPreferences => self.executor.update_editor_preferences(preferences.editor_preferences()),
-			PortfolioMessage::Import => {
-				// This portfolio message wraps the frontend message so it can be listed as an action, which isn't possible for frontend messages
-				responses.add(FrontendMessage::TriggerImport);
-			}
 			PortfolioMessage::LoadDocumentResources { document_id } => {
 				let catalog = &self.persistent_data.font_catalog;
 
@@ -465,9 +464,75 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageContext<'_>> for Portfolio
 					responses.add(PortfolioMessage::SelectDocument { document_id: next_id });
 				}
 			}
-			PortfolioMessage::OpenDocument => {
+			PortfolioMessage::Open => {
 				// This portfolio message wraps the frontend message so it can be listed as an action, which isn't possible for frontend messages
-				responses.add(FrontendMessage::TriggerOpenDocument);
+				responses.add(FrontendMessage::TriggerOpen);
+			}
+			PortfolioMessage::Import => {
+				// This portfolio message wraps the frontend message so it can be listed as an action, which isn't possible for frontend messages
+				responses.add(FrontendMessage::TriggerImport);
+			}
+			PortfolioMessage::OpenFile { path, content } => {
+				let name = path.file_stem().map(|n| n.to_string_lossy().to_string());
+				match Self::read_file(&path, content) {
+					FileContent::Document(content) => {
+						responses.add(PortfolioMessage::OpenDocumentFile {
+							document_name: name,
+							document_path: Some(path),
+							document_serialized_content: content,
+						});
+					}
+					FileContent::Svg(svg) => {
+						responses.add(PortfolioMessage::OpenSvg { name, svg });
+					}
+					FileContent::Image(image) => {
+						responses.add(PortfolioMessage::OpenImage { name, image });
+					}
+					FileContent::Unsupported => {
+						// TODO: Show a more thoughtfully designed error message to the user
+						responses.add(DialogMessage::DisplayDialogError {
+							title: "Unsupported format".into(),
+							description: "This file cannot be opened because it is not a supported image file type.".into(),
+						})
+					}
+				}
+			}
+			PortfolioMessage::ImportFile { path, content } => {
+				let name = path.file_stem().map(|n| n.to_string_lossy().to_string());
+				match Self::read_file(&path, content) {
+					FileContent::Document(content) => {
+						// TODO: Consider importing a document as a node into the current document
+						// For now treat importing a document as opening it
+						responses.add(PortfolioMessage::OpenDocumentFile {
+							document_name: name,
+							document_path: Some(path),
+							document_serialized_content: content,
+						});
+					}
+					FileContent::Svg(svg) => {
+						responses.add(PortfolioMessage::PasteSvg {
+							name,
+							svg,
+							mouse: None,
+							parent_and_insert_index: None,
+						});
+					}
+					FileContent::Image(image) => {
+						responses.add(PortfolioMessage::PasteImage {
+							name,
+							image,
+							mouse: None,
+							parent_and_insert_index: None,
+						});
+					}
+					FileContent::Unsupported => {
+						// TODO: Show a more thoughtfully designed error message to the user
+						responses.add(DialogMessage::DisplayDialogError {
+							title: "Unsupported format".into(),
+							description: "This file cannot be imported because it is not a supported image file type.".into(),
+						})
+					}
+				}
 			}
 			PortfolioMessage::OpenDocumentFile {
 				document_name,
@@ -596,6 +661,47 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageContext<'_>> for Portfolio
 					responses.add(PortfolioMessage::SelectDocument { document_id });
 				}
 			}
+			PortfolioMessage::OpenImage { name, image } => {
+				responses.add(PortfolioMessage::NewDocumentWithName {
+					name: name.clone().unwrap_or(DEFAULT_DOCUMENT_NAME.into()),
+				});
+
+				responses.add(DocumentMessage::PasteImage {
+					name,
+					image,
+					mouse: None,
+					parent_and_insert_index: None,
+				});
+
+				// Wait for the document to be rendered so the click targets can be calculated in order to determine the artboard size that will encompass the pasted image
+				responses.add(DeferMessage::AfterGraphRun {
+					messages: vec![DocumentMessage::WrapContentInArtboard { place_artboard_at_origin: true }.into()],
+				});
+				responses.add(DeferMessage::AfterNavigationReady {
+					messages: vec![DocumentMessage::ZoomCanvasToFitAll.into()],
+				});
+			}
+			PortfolioMessage::OpenSvg { name, svg } => {
+				responses.add(PortfolioMessage::NewDocumentWithName {
+					name: name.clone().unwrap_or(DEFAULT_DOCUMENT_NAME.into()),
+				});
+
+				responses.add(DocumentMessage::PasteSvg {
+					name,
+					svg,
+					mouse: None,
+					parent_and_insert_index: None,
+				});
+
+				// Wait for the document to be rendered so the click targets can be calculated in order to determine the artboard size that will encompass the pasted SVG
+				responses.add(DeferMessage::AfterGraphRun {
+					messages: vec![DocumentMessage::WrapContentInArtboard { place_artboard_at_origin: true }.into()],
+				});
+				responses.add(DeferMessage::AfterNavigationReady {
+					messages: vec![DocumentMessage::ZoomCanvasToFitAll.into()],
+				});
+			}
+			// TODO: Unused except by tests, remove?
 			PortfolioMessage::PasteIntoFolder { clipboard, parent, insert_index } => {
 				let mut all_new_ids = Vec::new();
 				let paste = |entry: &CopyBufferEntry, responses: &mut VecDeque<_>, all_new_ids: &mut Vec<NodeId>| {
@@ -856,28 +962,14 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageContext<'_>> for Portfolio
 				mouse,
 				parent_and_insert_index,
 			} => {
-				let create_document = self.documents.is_empty();
-
-				if create_document {
-					responses.add(PortfolioMessage::NewDocumentWithName {
-						name: name.clone().unwrap_or(DEFAULT_DOCUMENT_NAME.into()),
-					});
-				}
-
-				responses.add(DocumentMessage::PasteImage {
-					name,
-					image,
-					mouse,
-					parent_and_insert_index,
-				});
-
-				if create_document {
-					// Wait for the document to be rendered so the click targets can be calculated in order to determine the artboard size that will encompass the pasted image
-					responses.add(DeferMessage::AfterGraphRun {
-						messages: vec![DocumentMessage::WrapContentInArtboard { place_artboard_at_origin: true }.into()],
-					});
-					responses.add(DeferMessage::AfterNavigationReady {
-						messages: vec![DocumentMessage::ZoomCanvasToFitAll.into()],
+				if self.documents.is_empty() {
+					responses.add(PortfolioMessage::OpenImage { name, image });
+				} else {
+					responses.add(DocumentMessage::PasteImage {
+						name,
+						image,
+						mouse,
+						parent_and_insert_index,
 					});
 				}
 			}
@@ -887,29 +979,14 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageContext<'_>> for Portfolio
 				mouse,
 				parent_and_insert_index,
 			} => {
-				let create_document = self.documents.is_empty();
-
-				if create_document {
-					responses.add(PortfolioMessage::NewDocumentWithName {
-						name: name.clone().unwrap_or(DEFAULT_DOCUMENT_NAME.into()),
-					});
-				}
-
-				responses.add(DocumentMessage::PasteSvg {
-					name,
-					svg,
-					mouse,
-					parent_and_insert_index,
-				});
-
-				if create_document {
-					// Wait for the document to be rendered so the click targets can be calculated in order to determine the artboard size that will encompass the pasted image
-					responses.add(DeferMessage::AfterGraphRun {
-						messages: vec![DocumentMessage::WrapContentInArtboard { place_artboard_at_origin: true }.into()],
-					});
-
-					responses.add(DeferMessage::AfterNavigationReady {
-						messages: vec![DocumentMessage::ZoomCanvasToFitAll.into()],
+				if self.documents.is_empty() {
+					responses.add(PortfolioMessage::OpenSvg { name, svg });
+				} else {
+					responses.add(DocumentMessage::PasteSvg {
+						name,
+						svg,
+						mouse,
+						parent_and_insert_index,
 					});
 				}
 			}
@@ -940,9 +1017,9 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageContext<'_>> for Portfolio
 							TextButton::new("Open Document")
 								.icon(Some("Folder".into()))
 								.flush(true)
-								.on_commit(|_| PortfolioMessage::OpenDocument.into())
+								.on_commit(|_| PortfolioMessage::Open.into())
 								.widget_instance(),
-							ShortcutLabel::new(action_shortcut!(PortfolioMessageDiscriminant::OpenDocument)).widget_instance(),
+							ShortcutLabel::new(action_shortcut!(PortfolioMessageDiscriminant::Open)).widget_instance(),
 						],
 						vec![
 							TextButton::new("Open Demo Artwork")
@@ -1202,21 +1279,22 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageContext<'_>> for Portfolio
 
 	fn actions(&self) -> ActionList {
 		let mut common = actions!(PortfolioMessageDiscriminant;
-			CloseActiveDocumentWithConfirmation,
-			CloseAllDocuments,
-			CloseAllDocumentsWithConfirmation,
-			Import,
-			NextDocument,
-			OpenDocument,
-			PasteIntoFolder,
-			PrevDocument,
-			ToggleRulers,
+			Open,
 			ToggleDataPanelOpen,
 		);
 
 		// Extend with actions that require an active document
 		if let Some(document) = self.active_document() {
 			common.extend(document.actions());
+			common.extend(actions!(PortfolioMessageDiscriminant;
+				CloseActiveDocumentWithConfirmation,
+				CloseAllDocuments,
+				CloseAllDocumentsWithConfirmation,
+				ToggleRulers,
+				NextDocument,
+				PrevDocument,
+				Import,
+			));
 
 			// Extend with actions that must have a selected layer
 			if document.network_interface.selected_nodes().selected_layers(document.metadata()).next().is_some() {
@@ -1281,6 +1359,32 @@ impl PortfolioMessageHandler {
 		}
 	}
 
+	fn read_file(path: &PathBuf, content: Vec<u8>) -> FileContent {
+		let extension = path.extension().and_then(|ext| ext.to_str()).unwrap_or_default().to_lowercase();
+		match extension.as_str() {
+			FILE_EXTENSION => match String::from_utf8(content) {
+				Ok(content) => FileContent::Document(content),
+				Err(_) => FileContent::Unsupported,
+			},
+			"svg" => match String::from_utf8(content) {
+				Ok(content) => FileContent::Svg(content),
+				Err(_) => FileContent::Unsupported,
+			},
+			_ => {
+				let format = image::guess_format(&content).unwrap_or_else(|_| image::ImageFormat::from_path(path).unwrap_or(image::ImageFormat::Png));
+				match image::load_from_memory_with_format(&content, format) {
+					Ok(image) => {
+						// TODO: Handle Image formats with more than 8 bits per channel
+						let image_data = image.to_rgba8();
+						let image = Image::<Color>::from_image_data(image_data.as_raw(), image.width(), image.height());
+						FileContent::Image(image)
+					}
+					Err(_) => FileContent::Unsupported,
+				}
+			}
+		}
+	}
+
 	fn load_document(&mut self, mut new_document: DocumentMessageHandler, document_id: DocumentId, layers_panel_open: bool, responses: &mut VecDeque<Message>, to_front: bool) {
 		if to_front {
 			self.document_ids.push_front(document_id);

editor/src/messages/portfolio/utility_types.rs

@@ -1,3 +1,5 @@
+use graphene_std::Color;
+use graphene_std::raster::Image;
 use graphene_std::text::{Font, FontCache};
 
 #[derive(Debug, Default)]
@@ -104,3 +106,14 @@ impl From<String> for PanelType {
 		}
 	}
 }
+
+pub enum FileContent {
+	/// A Graphite document.
+	Document(String),
+	/// A bitmap image.
+	Image(Image<Color>),
+	/// An SVG file string.
+	Svg(String),
+	/// Any other unsupported/unrecognized file type.
+	Unsupported,
+}