Skip to content

Commit

Permalink
MDL-64331 modals: Be careful closing modals
Browse files Browse the repository at this point in the history
Don't close a modal when the user clicks outside of it and the modal contains a form.
  • Loading branch information
Damyon Wiese committed Apr 2, 2019
1 parent 0920f35 commit cdc7390
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 21 deletions.
2 changes: 1 addition & 1 deletion lib/amd/build/modal.min.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 14 additions & 1 deletion lib/amd/src/modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]',
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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));
Expand Down
21 changes: 10 additions & 11 deletions lib/tests/behat/action_modal.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,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.
Expand Down Expand Up @@ -103,6 +104,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.
*
Expand Down Expand Up @@ -162,7 +177,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') {
Expand Down
Loading

0 comments on commit cdc7390

Please sign in to comment.