From e676e1a780b746a8329d849cd31ec4fbd7fecd2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20L=C3=B6ffler?= Date: Mon, 7 Nov 2022 22:19:45 +0100 Subject: [PATCH] Modified error handling so that WelcomePage is no longer closed after confirming the error dialog. --- .../scenebuilder/app/SceneBuilderApp.java | 124 ++++++++++++------ .../WelcomeDialogWindowController.java | 57 ++++---- .../app/i18n/SceneBuilderApp.properties | 1 + 3 files changed, 112 insertions(+), 70 deletions(-) diff --git a/app/src/main/java/com/oracle/javafx/scenebuilder/app/SceneBuilderApp.java b/app/src/main/java/com/oracle/javafx/scenebuilder/app/SceneBuilderApp.java index 25439a934..c0e6337a0 100644 --- a/app/src/main/java/com/oracle/javafx/scenebuilder/app/SceneBuilderApp.java +++ b/app/src/main/java/com/oracle/javafx/scenebuilder/app/SceneBuilderApp.java @@ -499,6 +499,10 @@ private void createEmptyDocumentWindow() { @Override public void handleOpenFilesAction(List files) { + handleOpenFilesAction(files, null); + } + + public void handleOpenFilesAction(List files, Runnable onSuccess) { assert files != null; assert files.isEmpty() == false; @@ -511,21 +515,76 @@ public void handleOpenFilesAction(List files) { // Fix for #45 if (userLibrary.isFirstExplorationCompleted()) { - performOpenFiles(fileObjs); + var openResult = performOpenFiles(fileObjs); + handleFileOpenResult(openResult, onSuccess); } else { // open files only after the first exploration has finished userLibrary.firstExplorationCompletedProperty().addListener(new InvalidationListener() { @Override public void invalidated(Observable observable) { if (userLibrary.isFirstExplorationCompleted()) { - performOpenFiles(fileObjs); + var openResult = performOpenFiles(fileObjs); userLibrary.firstExplorationCompletedProperty().removeListener(this); + handleFileOpenResult(openResult, onSuccess); } } }); } } + /** + * The onSuccess Runnable is only executed if the {@link FileOpenResult} has no + * errors at all. + * + * @param result {@link FileOpenResult} holding the list of files to be + * opened and a map of files with exceptions in case of an + * error. + * @param onSuccess A runnable holding the action to be performed on success. + */ + private void handleFileOpenResult(FileOpenResult result, Runnable onSuccess) { + if (result.hasErrors()) { + showFileOpenErrors(result); + } else { + if (onSuccess != null) { + onSuccess.run(); + } + } + } + + /** + * In case of errors (>= 1 exception) a message dialog is shown to the user. + * There are versions for exactly 1 error and for more than 1 errors. + * + * @param openResult {@link FileOpenResult} holding a map of files with + * exceptions. + */ + private void showFileOpenErrors(FileOpenResult openResult) { + if (openResult.errors.size() == 1) { + final File fxmlFile = openResult.errors().keySet().iterator().next(); + final Exception x = openResult.errors().get(fxmlFile); + final ErrorDialog errorDialog = new ErrorDialog(null); + errorDialog.setMessage(I18N.getString("alert.open.failure1.message", displayName(fxmlFile.getPath()))); + errorDialog.setDetails(I18N.getString("alert.open.failure1.details")); + errorDialog.setDebugInfoWithThrowable(x); + errorDialog.setTitle(I18N.getString("alert.open.failure.title")); + errorDialog.showAndWait(); + } else if (openResult.errors.size() > 1){ + final ErrorDialog errorDialog = new ErrorDialog(null); + if (openResult.errors().size() == openResult.filesToOpen.size()) { + // Open operation has failed for all the files + errorDialog.setMessage(I18N.getString("alert.open.failureN.message")); + errorDialog.setDetails(I18N.getString("alert.open.failureN.details")); + } else { + // Open operation has failed for some files + errorDialog.setMessage(I18N.getString("alert.open.failureMofN.message", + openResult.errors().size(), openResult.filesToOpen.size())); + errorDialog.setDetails(I18N.getString("alert.open.failureMofN.details")); + } + errorDialog.setTitle(I18N.getString("alert.open.failure.title")); + errorDialog.showAndWait(); + } + } + @Override public void handleMessageBoxFailure(Exception x) { final ErrorDialog errorDialog = new ErrorDialog(null); @@ -657,11 +716,12 @@ public DocumentWindowController getFrontDocumentWindow() { return null; } - private void performOpenFiles(List fxmlFiles) { + private FileOpenResult performOpenFiles(List fxmlFiles) { assert fxmlFiles != null; assert fxmlFiles.isEmpty() == false; final Map exceptions = new HashMap<>(); + final List openedFiles = new ArrayList<>(); for (File fxmlFile : fxmlFiles) { try { final DocumentWindowController dwc @@ -674,47 +734,20 @@ private void performOpenFiles(List fxmlFiles) { var hostWindow = findFirstUnusedDocumentWindowController().orElse(makeNewWindow()); hostWindow.loadFromFile(fxmlFile); hostWindow.openWindow(); + openedFiles.add(fxmlFile); } } catch (Exception xx) { exceptions.put(fxmlFile, xx); } } - - switch (exceptions.size()) { - case 0: { // Good - // Update recent items with opened files - final PreferencesController pc = PreferencesController.getSingleton(); - pc.getRecordGlobal().addRecentItems(fxmlFiles); - break; - } - case 1: { - final File fxmlFile = exceptions.keySet().iterator().next(); - final Exception x = exceptions.get(fxmlFile); - final ErrorDialog errorDialog = new ErrorDialog(null); - errorDialog.setMessage(I18N.getString("alert.open.failure1.message", displayName(fxmlFile.getPath()))); - errorDialog.setDetails(I18N.getString("alert.open.failure1.details")); - errorDialog.setDebugInfoWithThrowable(x); - errorDialog.setTitle(I18N.getString("alert.title.open")); - errorDialog.showAndWait(); - break; - } - default: { - final ErrorDialog errorDialog = new ErrorDialog(null); - if (exceptions.size() == fxmlFiles.size()) { - // Open operation has failed for all the files - errorDialog.setMessage(I18N.getString("alert.open.failureN.message")); - errorDialog.setDetails(I18N.getString("alert.open.failureN.details")); - } else { - // Open operation has failed for some files - errorDialog.setMessage(I18N.getString("alert.open.failureMofN.message", - exceptions.size(), fxmlFiles.size())); - errorDialog.setDetails(I18N.getString("alert.open.failureMofN.details")); - } - errorDialog.setTitle(I18N.getString("alert.title.open")); - errorDialog.showAndWait(); - break; - } + + // Update recent items with opened files + if (!openedFiles.isEmpty()) { + final PreferencesController pc = PreferencesController.getSingleton(); + pc.getRecordGlobal().addRecentItems(openedFiles); } + + return new FileOpenResult(fxmlFiles, exceptions); } private void performExit() { @@ -1083,4 +1116,19 @@ public static void applyToAllDocumentWindows(Consumer consumer.accept(dwc); } } + + /** + * Describes the result of FXML file loading process. As in general SceneBuilder + * can open multiple files at the same time. Hence this record can hold of the + * files supposed to be opened. In case of errors, the exceptions are stored in + * a map per file. + */ + record FileOpenResult(List filesToOpen, Map errors) { + public boolean isSuccess() { + return errors.isEmpty(); + } + public boolean hasErrors() { + return !errors.isEmpty(); + } + } } diff --git a/app/src/main/java/com/oracle/javafx/scenebuilder/app/welcomedialog/WelcomeDialogWindowController.java b/app/src/main/java/com/oracle/javafx/scenebuilder/app/welcomedialog/WelcomeDialogWindowController.java index 64028b831..5ed1924c7 100644 --- a/app/src/main/java/com/oracle/javafx/scenebuilder/app/welcomedialog/WelcomeDialogWindowController.java +++ b/app/src/main/java/com/oracle/javafx/scenebuilder/app/welcomedialog/WelcomeDialogWindowController.java @@ -256,35 +256,31 @@ boolean filePathExists(String filePath) { * Builder. */ private void handleOpen(List filePaths) { - Consumer> missingFilesHandler = missingFiles->{ - if (!missingFiles.isEmpty()) { - var questionDialog = questionMissingFilesCleanup(getStage(), missingFiles); - var x = getInstance().getStage().getX(); - var y = getInstance().getStage().getY(); - var width = getInstance().getStage().getWidth(); - var height = getInstance().getStage().getHeight(); - questionDialog.getStage().setX(x+width/3); - questionDialog.getStage().setY(y+height/3); - if (questionDialog.showAndWait() == AlertDialog.ButtonID.OK) { - removeMissingFilesFromPrefs(missingFiles); - loadAndPopulateRecentItemsInBackground(); - } - } - }; + handleOpen(filePaths, + this::askUserToRemoveMissingRecentFiles, + this::attemptOpenExistingFiles); + } - Consumer> existingFilesHandler = paths->{ - if (sceneBuilderApp.startupTasksFinishedBinding().get()) { - sceneBuilderApp.handleOpenFilesAction(paths); - getStage().hide(); - } else { - showMasker(() -> { - sceneBuilderApp.handleOpenFilesAction(paths); - getStage().hide(); - }); + private void askUserToRemoveMissingRecentFiles(List missingFiles) { + if (!missingFiles.isEmpty()) { + var questionDialog = questionMissingFilesCleanup(getStage(), missingFiles); + if (questionDialog.showAndWait() == AlertDialog.ButtonID.OK) { + removeMissingFilesFromPrefs(missingFiles); + loadAndPopulateRecentItemsInBackground(); } - }; + } + } + + private void attemptOpenExistingFiles(List paths) { + if (sceneBuilderApp.startupTasksFinishedBinding().get()) { + openFilesAndHideStage(paths); + } else { + showMasker(() -> openFilesAndHideStage(paths)); + } + } - handleOpen(filePaths, missingFilesHandler, existingFilesHandler); + private void openFilesAndHideStage(List files) { + sceneBuilderApp.handleOpenFilesAction(files,()->getStage().hide()); } /** @@ -301,21 +297,20 @@ void handleOpen(List filePaths, if (filePaths.isEmpty()) { return; } - + var candidates = filePaths.stream() .collect(Collectors.groupingBy(this::filePathExists)); - + List missingFiles = candidates.getOrDefault(Boolean.FALSE, new ArrayList<>()); missingFilesHandler.accept(missingFiles); List paths = candidates.getOrDefault(Boolean.TRUE, new ArrayList<>()) .stream() .toList(); - + if (paths.isEmpty()) { return; } - fileLoader.accept(paths); } @@ -335,8 +330,6 @@ private void showMasker(Runnable onEndAction) { if (isFinished) { Platform.runLater(() -> { onEndAction.run(); - - // restore state in case welcome dialog is opened again contentPane.setDisable(false); masker.setVisible(false); }); diff --git a/app/src/main/resources/com/oracle/javafx/scenebuilder/app/i18n/SceneBuilderApp.properties b/app/src/main/resources/com/oracle/javafx/scenebuilder/app/i18n/SceneBuilderApp.properties index 31dddd7d3..48fc94337 100644 --- a/app/src/main/resources/com/oracle/javafx/scenebuilder/app/i18n/SceneBuilderApp.properties +++ b/app/src/main/resources/com/oracle/javafx/scenebuilder/app/i18n/SceneBuilderApp.properties @@ -396,6 +396,7 @@ alert.title.copy = Copy alert.title.start = Startup alert.title.messagebox = External Open +alert.open.failure.title = File Open Error alert.open.failure1.message = Could not open ''{0}'' alert.open.failure1.details = Open operation has failed. Make sure that the chosen file is a valid FXML document. alert.open.failureN.message = Could not open the specified files