changeset 7727:e482cf3b4593

RT-38358: [Dialogs] Need to specify how Dialog.close() is handled
author jgiles
date Wed, 20 Aug 2014 12:55:42 +1200
parents d8500a1bace1
children eb55468a0a26
files modules/controls/src/main/java/javafx/scene/control/Dialog.java modules/controls/src/main/java/javafx/scene/control/FXDialog.java modules/controls/src/main/java/javafx/scene/control/HeavyweightDialog.java
diffstat 3 files changed, 80 insertions(+), 81 deletions(-) [+]
line wrap: on
line diff
--- a/modules/controls/src/main/java/javafx/scene/control/Dialog.java	Tue Aug 19 16:50:11 2014 -0700
+++ b/modules/controls/src/main/java/javafx/scene/control/Dialog.java	Wed Aug 20 12:55:42 2014 +1200
@@ -217,11 +217,6 @@
     private final FXDialog dialog;
 
     private boolean isClosing;
-    
-    // used to indicate upwards from FXDialog implementations into Dialog whether
-    // the dialog was closed normally (i.e. via buttons) or abnormally (i.e. via
-    // the top-right cancel button, alt-f4, etc).
-    boolean closeWasNormal;
 
     
     
@@ -309,41 +304,50 @@
         if (isClosing) return;
         isClosing = true;
 
-        // This code is called just before close, and ONLY in cases where the 
+        final R result = getResult();
+
+        // if the result is null and we do not have permission to close the
+        // dialog, then we cancel the close request before any events are
+        // even fired
+        if (result == null && ! dialog.requestPermissionToClose(this)) {
+            isClosing = false;
+            return;
+        }
+
+        // This code is called just before close, and ONLY in cases where the
         // dialog was closed abnormally (as represented by closeWasNormal).
-        // In these cases, and where the dialog had a cancel button, we call 
+        // In these cases, and where the dialog had a cancel button, we call
         // into the result converter to see what to do. This is used primarily
         // to handle the requirement that the X button has the same result as
         // clicking the cancel button.
-        if (! closeWasNormal) {
-            R result = getResult();
-            if (result == null) {
-                ButtonType cancelButton = null;
-                for (ButtonType button : getDialogPane().getButtonTypes()) {
-                    if (button.getButtonData() == ButtonData.CANCEL_CLOSE) {
-                        cancelButton = button;
-                        break;
-                    }
+        // TODO we used to discern between normal and abnormal closures of the
+        // stage, but this was not resilient enough. For now, we no longer check
+        // in the code below whether the close was normal or not, but depending
+        // on how things work out, this may need to change
+        if (result == null) {
+            ButtonType cancelButton = null;
+            for (ButtonType button : getDialogPane().getButtonTypes()) {
+                if (button.getButtonData() == ButtonData.CANCEL_CLOSE) {
+                    cancelButton = button;
+                    break;
                 }
+            }
 
-                impl_setResultAndClose(cancelButton, false);
-            }
-            
-            closeWasNormal = true;
+            impl_setResultAndClose(cancelButton, false);
         }
-        
+
         // start normal closing process
         Event.fireEvent(this, new DialogEvent(this, DialogEvent.DIALOG_HIDING));
-        
+
         DialogEvent closeRequestEvent = new DialogEvent(this, DialogEvent.DIALOG_CLOSE_REQUEST);
         Event.fireEvent(this, closeRequestEvent);
         if (closeRequestEvent.isConsumed()) {
             isClosing = false;
             return;
         }
-        
-        dialog.close(true);
-        
+
+        dialog.close();
+
         Event.fireEvent(this, new DialogEvent(this, DialogEvent.DIALOG_HIDDEN));
 
         isClosing = false;
--- a/modules/controls/src/main/java/javafx/scene/control/FXDialog.java	Tue Aug 19 16:50:11 2014 -0700
+++ b/modules/controls/src/main/java/javafx/scene/control/FXDialog.java	Wed Aug 20 12:55:42 2014 +1200
@@ -25,6 +25,7 @@
 package javafx.scene.control;
 
 import java.net.URL;
+import java.util.List;
 
 import javafx.beans.property.BooleanProperty;
 import javafx.beans.property.ReadOnlyBooleanProperty;
@@ -72,7 +73,36 @@
      * 
      **************************************************************************/
 
+    public boolean requestPermissionToClose(final Dialog<?> dialog) {
+        // We only allow the dialog to be closed abnormally (i.e. via the X button)
+        // when there is a cancel button in the dialog, or when there is only
+        // one button in the dialog. In all other cases, we disable the ability
+        // (as best we can) to close a dialog abnormally.
+        boolean denyClose = true;
 
+        // if we are here, the close was abnormal, so we must call close to
+        // clean up, if we don't consume the event to cancel closing...
+        DialogPane dialogPane = dialog.getDialogPane();
+        if (dialogPane != null) {
+
+            List<ButtonType> buttons = dialogPane.getButtonTypes();
+            if (buttons.size() == 1) {
+                denyClose = false;
+            } else {
+                // look for cancel button type
+                for (ButtonType button : buttons) {
+                    if (button == null) continue;
+                    ButtonBar.ButtonData type = button.getButtonData();
+                    if (type == ButtonBar.ButtonData.CANCEL_CLOSE) {
+                        denyClose = false;
+                        break;
+                    }
+                }
+            }
+        }
+
+        return !denyClose;
+    }
 
 
     /***************************************************************************
@@ -85,8 +115,11 @@
     
     public abstract void showAndWait();
 
-    public abstract void close(boolean closeWasNormal);
-    
+    // This should only be called from Dialog - it should never be called by
+    // subclasses of FXDialog. Implementations should never call up to
+    // Dialog.close().
+    public abstract void close();
+
     public abstract void initOwner(Window owner);
 
     public abstract Window getOwner();
--- a/modules/controls/src/main/java/javafx/scene/control/HeavyweightDialog.java	Tue Aug 19 16:50:11 2014 -0700
+++ b/modules/controls/src/main/java/javafx/scene/control/HeavyweightDialog.java	Wed Aug 20 12:55:42 2014 +1200
@@ -24,18 +24,15 @@
  */
 package javafx.scene.control;
 
-import java.util.List;
-
 import javafx.beans.property.BooleanProperty;
 import javafx.beans.property.ReadOnlyBooleanProperty;
 import javafx.beans.property.ReadOnlyDoubleProperty;
 import javafx.beans.property.StringProperty;
-import javafx.collections.ObservableList;
 import javafx.scene.Node;
 import javafx.scene.Scene;
-import javafx.scene.control.ButtonBar.ButtonData;
+import javafx.scene.input.KeyCode;
+import javafx.scene.input.KeyEvent;
 import javafx.scene.layout.StackPane;
-import javafx.scene.paint.Color;
 import javafx.stage.Modality;
 import javafx.stage.Screen;
 import javafx.stage.Stage;
@@ -122,50 +119,27 @@
         stage.setResizable(false);
         
         stage.setOnCloseRequest(windowEvent -> {
-            // We only allow the dialog to be closed abnormally (i.e. via the X button)
-            // when there is a cancel button in the dialog, or when there is only
-            // one button in the dialog. In all other cases, we disable the ability
-            // (as best we can) to close a dialog abnormally.
-            boolean denyClose = true;
+            if (requestPermissionToClose(dialog)) {
+                dialog.close();
+            } else {
+                // if we are here, we consume the event to prevent closing the dialog
+                windowEvent.consume();
+            }
+        });
 
-            // if the close was normal, we don't need to call close ourselves,
-            // so just return
-            if (dialog.closeWasNormal) return;
-
-            // if we are here, the close was abnormal, so we must call close to
-            // clean up, if we don't consume the event to cancel closing...
-            DialogPane dialogPane = dialog.getDialogPane();
-            if (dialogPane != null) {
-
-                List<ButtonType> buttons = dialogPane.getButtonTypes();
-                if (buttons.size() == 1) {
-                    denyClose = false;
-                } else {
-                    // look for cancel button type
-                    for (ButtonType button : buttons) {
-                        if (button == null) continue;
-                        ButtonData type = button.getButtonData();
-                        if (type == ButtonData.CANCEL_CLOSE) {
-                            denyClose = false;
-                            break;
-                        }
-                    }
+        stage.addEventHandler(KeyEvent.KEY_RELEASED, keyEvent -> {
+            if (keyEvent.getCode() == KeyCode.ESCAPE) {
+                if (requestPermissionToClose(dialog)) {
+                    dialog.close();
+                    keyEvent.consume();
                 }
             }
-            
-            // if we are here, we consume the event to prevent closing the dialog
-            if (denyClose) {
-                windowEvent.consume();
-            } else {
-                close(false);
-            }
         });
 
         sceneRoot = new StackPane();
         sceneRoot.getStyleClass().setAll("dialog");
         
         scene = new Scene(sceneRoot);
-//        scene.setFill(Color.TRANSPARENT);
         stage.setScene(scene);
     }
 
@@ -222,24 +196,12 @@
         stage.showAndWait();
     }
 
-    boolean isClosing = false;
-    @Override public void close(boolean closeWasNormal) {
-        if (isClosing) return;
-        
-        isClosing = true;
-        dialog.closeWasNormal = closeWasNormal;
-        
-        // enabling this code has the effect of firing the Dialog.onHiding and onHidden events twice
-        if (dialog.isShowing()) {
-            dialog.close();
-        }
-        
+    @Override public void close() {
         if (stage.isShowing()) {
             stage.hide();
         }
-        isClosing = false;
     }
-    
+
     @Override public ReadOnlyBooleanProperty showingProperty() {
         return stage.showingProperty();
     }