MDL-64331 modals: Be careful closing modals
authorDamyon Wiese <damyon@moodle.com>
Thu, 28 Mar 2019 01:14:10 +0000 (09:14 +0800)
committerDamyon Wiese <damyon@moodle.com>
Tue, 2 Apr 2019 02:02:26 +0000 (10:02 +0800)
Don't close a modal when the user clicks outside of it and the modal contains a form.

lib/amd/build/modal.min.js
lib/amd/src/modal.js
lib/tests/behat/action_modal.feature
lib/yui/build/moodle-core-notification-dialogue/moodle-core-notification-dialogue-debug.js
lib/yui/build/moodle-core-notification-dialogue/moodle-core-notification-dialogue-min.js
lib/yui/build/moodle-core-notification-dialogue/moodle-core-notification-dialogue.js
lib/yui/src/notification/js/dialogue.js

index 096b837..f3a2db2 100644 (file)
Binary files a/lib/amd/build/modal.min.js and b/lib/amd/build/modal.min.js differ
index 799b3dc..4e61d10 100644 (file)
@@ -35,6 +35,7 @@ define(['jquery', 'core/templates', 'core/notification', 'core/key_codes',
         FOOTER: '[data-region="footer"]',
         HIDE: '[data-action="hide"]',
         DIALOG: '[role=dialog]',
+        FORM: 'form',
         MENU_BAR: '[role=menubar]',
         HAS_Z_INDEX: '.moodle-has-zindex',
         CAN_RECEIVE_FOCUS: 'input:not([type="hidden"]), a[href], button, textarea, select, [tabindex]',
@@ -571,6 +572,18 @@ define(['jquery', 'core/templates', 'core/notification', 'core/key_codes',
         }.bind(this));
     };
 
+    /**
+     * Hide this modal if it does not contain a form.
+     *
+     * @method hideIfNotForm
+     */
+    Modal.prototype.hideIfNotForm = function() {
+        var formElement = this.modal.find(SELECTORS.FORM);
+        if (formElement.length == 0) {
+            this.hide();
+        }
+    };
+
     /**
      * Hide this modal.
      *
@@ -727,7 +740,7 @@ define(['jquery', 'core/templates', 'core/notification', 'core/key_codes',
                 // So, we check if we can still find the container element or not. If not, then the DOM tree is changed.
                 // It's best not to hide the modal in that case.
                 if ($(e.target).closest(SELECTORS.CONTAINER).length) {
-                    this.hide();
+                    this.hideIfNotForm();
                 }
             }
         }.bind(this));
index 81dc26c..837b7a6 100644 (file)
@@ -2,7 +2,7 @@
 Feature: Close modals by clicking outside them
   In order to easily close the currently open pop-up
   As a user
-  Clicking outside the modal should close it.
+  Clicking outside the modal should close it if it doesn't contain a form.
 
   @javascript
   Scenario: The popup closes when clicked on dead space - YUI
@@ -20,20 +20,19 @@ Feature: Close modals by clicking outside them
     And I click on "a new question" "link"
     # Cannot use the normal ‘I click on’ here, because the pop-up gets in the way.
     And I click on ".moodle-dialogue-lightbox" "css_element" skipping visibility check
-    Then I should not see "Choose a question type to add"
+    # The modal does not close because it contains a form.
+    Then I should see "Choose a question type to add"
 
   @javascript
   Scenario: The popup closes when clicked on dead space - Modal
-    Given the following "courses" exist:
-      | fullname | shortname |
-      | Course 1 | C1        |
-    And I log in as "admin"
-    And I am on "Course 1" course homepage
-    And I turn editing mode on
-    And I click on "Add topics" "link"
+    Given I log in as "admin"
+    And I am on site homepage
+    And I click on "Calendar" "link"
+    And I click on "New event" "button"
     When I click on "[data-region='modal-container']" "css_element"
-    Then ".modal-backdrop" "css_element" should not be visible
-    And ".modal-content" "css_element" should not be visible
+    # The modal does not close becaue it contains a form.
+    Then ".modal-backdrop" "css_element" should be visible
+    And ".modal-content" "css_element" should be visible
 
   @javascript
   Scenario: The popup help closes when clicked
index 7eec157..ca595e4 100644 (file)
Binary files a/lib/yui/build/moodle-core-notification-dialogue/moodle-core-notification-dialogue-debug.js and b/lib/yui/build/moodle-core-notification-dialogue/moodle-core-notification-dialogue-debug.js differ
index 4e29bd4..4f4a443 100644 (file)
Binary files a/lib/yui/build/moodle-core-notification-dialogue/moodle-core-notification-dialogue-min.js and b/lib/yui/build/moodle-core-notification-dialogue/moodle-core-notification-dialogue-min.js differ
index 1cefaaa..7766ab5 100644 (file)
Binary files a/lib/yui/build/moodle-core-notification-dialogue/moodle-core-notification-dialogue.js and b/lib/yui/build/moodle-core-notification-dialogue/moodle-core-notification-dialogue.js differ
index d7bcfd0..f8664c6 100644 (file)
@@ -15,7 +15,8 @@ var DIALOGUE_NAME = 'Moodle dialogue',
     MENUBAR_SELECTOR = '[role=menubar]',
     DOT = '.',
     HAS_ZINDEX = 'moodle-has-zindex',
-    CAN_RECEIVE_FOCUS_SELECTOR = 'input:not([type="hidden"]), a[href], button, textarea, select, [tabindex]';
+    CAN_RECEIVE_FOCUS_SELECTOR = 'input:not([type="hidden"]), a[href], button, textarea, select, [tabindex]',
+    FORM_SELECTOR = 'form';
 
 /**
  * A re-usable dialogue box with Moodle classes applied.
@@ -73,6 +74,20 @@ Y.extend(DIALOGUE, Y.Panel, {
      */
     _hiddenSiblings: null,
 
+    /**
+     * Hide the modal only if it doesn't contain a form.
+     *
+     * @method hideIfNotForm
+     */
+    hideIfNotForm: function() {
+        var bb = this.get('boundingBox'),
+            formElement = bb.one(FORM_SELECTOR);
+
+        if (formElement === null) {
+            this.hide();
+        }
+    },
+
     /**
      * Initialise the dialogue.
      *
@@ -132,7 +147,7 @@ Y.extend(DIALOGUE, Y.Panel, {
             var maskNode = this.get('maskNode');
             if (this._currentMaskNodeId !== maskNode.get('_yuid')) {
                 this._currentMaskNodeId = maskNode.get('_yuid');
-                maskNode.on('click', this.hide, this);
+                maskNode.on('click', this.hideIfNotForm, this);
             }
 
             if (bb.getStyle('position') !== 'fixed') {