From c9c55d86a446df4fc11641f0bb7cae1346a9410a Mon Sep 17 00:00:00 2001 From: Karol Kosek Date: Mon, 14 Feb 2022 13:03:42 +0100 Subject: [PATCH] Userland: Ask first for unsaved changes after clicking an "Open" action Previously there was some inconsistency between the apps when clicking the "Open" action while the file wasn't saved. Some programs (Font Editor) immediately asked you if you wanted to save the modified file, while others (Text Editor, Hex Editor and Playground) would show the save dialog only *after* you selected a file. I think it's better to ask a user right away if they want to save file, because a dialog after selecting a file should be generally related to that selected file, like an error opening a file, an import window etc. --- Userland/Applications/HexEditor/HexEditorWidget.cpp | 6 +++--- Userland/Applications/TextEditor/MainWidget.cpp | 8 ++++---- Userland/DevTools/Playground/main.cpp | 9 ++++----- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/Userland/Applications/HexEditor/HexEditorWidget.cpp b/Userland/Applications/HexEditor/HexEditorWidget.cpp index 378eb65dd4..5b09e55d10 100644 --- a/Userland/Applications/HexEditor/HexEditorWidget.cpp +++ b/Userland/Applications/HexEditor/HexEditorWidget.cpp @@ -85,11 +85,11 @@ HexEditorWidget::HexEditorWidget() }); m_open_action = GUI::CommonActions::make_open_action([this](auto&) { - auto response = FileSystemAccessClient::Client::the().try_open_file(window(), {}, Core::StandardPaths::home_directory(), Core::OpenMode::ReadWrite); - if (response.is_error()) + if (!request_close()) return; - if (!request_close()) + auto response = FileSystemAccessClient::Client::the().try_open_file(window(), {}, Core::StandardPaths::home_directory(), Core::OpenMode::ReadWrite); + if (response.is_error()) return; open_file(response.value()); diff --git a/Userland/Applications/TextEditor/MainWidget.cpp b/Userland/Applications/TextEditor/MainWidget.cpp index 85716b6692..f7dd185dd1 100644 --- a/Userland/Applications/TextEditor/MainWidget.cpp +++ b/Userland/Applications/TextEditor/MainWidget.cpp @@ -267,10 +267,6 @@ MainWidget::MainWidget() }); m_open_action = GUI::CommonActions::make_open_action([this](auto&) { - auto response = FileSystemAccessClient::Client::the().try_open_file(window()); - if (response.is_error()) - return; - if (editor().document().is_modified()) { auto save_document_first_result = GUI::MessageBox::ask_about_unsaved_changes(window(), m_path, editor().document().undo_stack().last_unmodified_timestamp()); if (save_document_first_result == GUI::Dialog::ExecResult::ExecYes) @@ -279,6 +275,10 @@ MainWidget::MainWidget() return; } + auto response = FileSystemAccessClient::Client::the().try_open_file(window()); + if (response.is_error()) + return; + read_file(*response.value()); }); diff --git a/Userland/DevTools/Playground/main.cpp b/Userland/DevTools/Playground/main.cpp index 6946bb41c7..d0cfe3e5ea 100644 --- a/Userland/DevTools/Playground/main.cpp +++ b/Userland/DevTools/Playground/main.cpp @@ -175,11 +175,6 @@ ErrorOr serenity_main(Main::Arguments arguments) }); TRY(file_menu->try_add_action(GUI::CommonActions::make_open_action([&](auto&) { - Optional open_path = GUI::FilePicker::get_open_filepath(window); - - if (!open_path.has_value()) - return; - if (window->is_modified()) { auto result = GUI::MessageBox::ask_about_unsaved_changes(window, file_path, editor->document().undo_stack().last_unmodified_timestamp()); if (result == GUI::MessageBox::ExecYes) @@ -188,6 +183,10 @@ ErrorOr serenity_main(Main::Arguments arguments) return; } + Optional open_path = GUI::FilePicker::get_open_filepath(window); + if (!open_path.has_value()) + return; + auto file = Core::File::construct(open_path.value()); if (!file->open(Core::OpenMode::ReadOnly) && file->error() != ENOENT) { GUI::MessageBox::show(window, String::formatted("Opening \"{}\" failed: {}", open_path.value(), strerror(errno)), "Error", GUI::MessageBox::Type::Error);