FEATURE: prevent accidental canceling when drafting penalties (#9105)

Pop up a confirmation box when there is input. This prevents accidental closing
of the dialog boxes due to clicking outside.

This adds a development hook on modals in the form of a `beforeClose`
function. Modal windows can abort the close if the funtion returns false.

Additionally fixing a few issues with loop and state on the modal popups:

Escape key with bootbox is keyup.
Updating modal to close on keyup as well so escape key is working.
Fixes an issue where pressing esc will loop immediately back to the modal by:
keydown -> bootbox -> keyup -> acts as "cancel", restores modal

Needs a next call to reopenModal otherwise, keyup is handled again by the modal.
Fixes an issue where pressing esc will loop immediately back to the confirm:
esc keyup will be handled and bubble immediately back to the modal.

Additionally, only handle key events when the #discourse-modal is visible.
This resolves issues where escape or enter events were being handled by
a hidden modal window.
This commit is contained in:
Jeff Wong 2020-03-05 13:55:35 -10:00 committed by GitHub
parent 54f67661ac
commit 243284f998
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 77 additions and 15 deletions

View File

@ -1,6 +1,7 @@
import ModalFunctionality from "discourse/mixins/modal-functionality"; import ModalFunctionality from "discourse/mixins/modal-functionality";
import { popupAjaxError } from "discourse/lib/ajax-error"; import { popupAjaxError } from "discourse/lib/ajax-error";
import Mixin from "@ember/object/mixin"; import Mixin from "@ember/object/mixin";
import { next } from "@ember/runloop";
import { Promise } from "rsvp"; import { Promise } from "rsvp";
export default Mixin.create(ModalFunctionality, { export default Mixin.create(ModalFunctionality, {
@ -11,6 +12,7 @@ export default Mixin.create(ModalFunctionality, {
user: null, user: null,
postId: null, postId: null,
successCallback: null, successCallback: null,
confirmClose: false,
resetModal() { resetModal() {
this.setProperties({ this.setProperties({
@ -21,10 +23,30 @@ export default Mixin.create(ModalFunctionality, {
postEdit: null, postEdit: null,
postAction: "delete", postAction: "delete",
before: null, before: null,
successCallback: null successCallback: null,
confirmClose: false
}); });
}, },
beforeClose() {
// prompt a confirmation if we have unsaved content
if (
(this.reason && this.reason.length > 1) ||
(this.message && this.message.length > 1 && !this.confirmClose)
) {
this.send("hideModal");
bootbox.confirm(I18n.t("admin.user.confirm_cancel_penalty"), result => {
if (result) {
this.set("confirmClose", true);
this.send("closeModal");
} else {
next(() => this.send("reopenModal"));
}
});
return false;
}
},
penalize(cb) { penalize(cb) {
let before = this.before; let before = this.before;
let promise = before ? before() : Promise.resolve(); let promise = before ? before() : Promise.resolve();

View File

@ -31,13 +31,16 @@ export default Component.extend({
@on("didInsertElement") @on("didInsertElement")
setUp() { setUp() {
$("html").on("keydown.discourse-modal", e => { $("html").on("keyup.discourse-modal", e => {
if (e.which === 27 && this.dismissable) { //only respond to events when the modal is visible
next(() => $(".modal-header button.modal-close").click()); if ($("#discourse-modal:visible").length > 0) {
} if (e.which === 27 && this.dismissable) {
next(() => $(".modal-header button.modal-close").click());
}
if (e.which === 13 && this.triggerClickOnEnter(e)) { if (e.which === 13 && this.triggerClickOnEnter(e)) {
next(() => $(".modal-footer .btn-primary").click()); next(() => $(".modal-footer .btn-primary").click());
}
} }
}); });
@ -46,7 +49,7 @@ export default Component.extend({
@on("willDestroyElement") @on("willDestroyElement")
cleanUp() { cleanUp() {
$("html").off("keydown.discourse-modal"); $("html").off("keyup.discourse-modal");
this.appEvents.off("modal:body-shown", this, "_modalBodyShown"); this.appEvents.off("modal:body-shown", this, "_modalBodyShown");
}, },

View File

@ -157,12 +157,23 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, {
// Close the current modal, and destroy its state. // Close the current modal, and destroy its state.
closeModal() { closeModal() {
this.render("hide-modal", { into: "modal", outlet: "modalBody" });
const route = getOwner(this).lookup("route:application"); const route = getOwner(this).lookup("route:application");
let modalController = route.controllerFor("modal"); let modalController = route.controllerFor("modal");
const controllerName = modalController.get("name"); const controllerName = modalController.get("name");
if (controllerName) {
const controller = getOwner(this).lookup(
`controller:${controllerName}`
);
if (controller && controller.beforeClose) {
if (false === controller.beforeClose()) {
return;
}
}
}
this.render("hide-modal", { into: "modal", outlet: "modalBody" });
if (controllerName) { if (controllerName) {
const controller = getOwner(this).lookup( const controller = getOwner(this).lookup(
`controller:${controllerName}` `controller:${controllerName}`

View File

@ -4291,6 +4291,7 @@ en:
threshold_reached: "Received too many bounces from that email." threshold_reached: "Received too many bounces from that email."
trust_level_change_failed: "There was a problem changing the user's trust level." trust_level_change_failed: "There was a problem changing the user's trust level."
suspend_modal_title: "Suspend User" suspend_modal_title: "Suspend User"
confirm_cancel_penalty: "Are you sure you want to discard the penalty?"
trust_level_2_users: "Trust Level 2 Users" trust_level_2_users: "Trust Level 2 Users"
trust_level_3_requirements: "Trust Level 3 Requirements" trust_level_3_requirements: "Trust Level 3 Requirements"
trust_level_locked_tip: "trust level is locked, system will not promote or demote user" trust_level_locked_tip: "trust level is locked, system will not promote or demote user"

View File

@ -34,6 +34,31 @@ QUnit.test("suspend a user - cancel", async assert => {
assert.equal(find(".suspend-user-modal:visible").length, 0); assert.equal(find(".suspend-user-modal:visible").length, 0);
}); });
QUnit.test("suspend a user - cancel with input", async assert => {
await visit("/admin/users/1234/regular");
await click(".suspend-user");
assert.equal(find(".suspend-user-modal:visible").length, 1);
await fillIn(".suspend-reason", "for breaking the rules");
await fillIn(".suspend-message", "this is an email reason why");
await click(".d-modal-cancel");
assert.equal(find(".bootbox.modal:visible").length, 1);
await click(".modal-footer .btn-default");
assert.equal(find(".suspend-user-modal:visible").length, 1);
assert.equal(
find(".suspend-message")[0].value,
"this is an email reason why"
);
await click(".d-modal-cancel");
await click(".modal-footer .btn-primary");
assert.equal(find(".suspend-user-modal:visible").length, 0);
});
QUnit.test("suspend, then unsuspend a user", async assert => { QUnit.test("suspend, then unsuspend a user", async assert => {
const suspendUntilCombobox = selectKit(".suspend-until .combobox"); const suspendUntilCombobox = selectKit(".suspend-until .combobox");

View File

@ -28,7 +28,7 @@ QUnit.test("modal", async function(assert) {
await click(".login-button"); await click(".login-button");
assert.ok(find(".d-modal:visible").length === 1, "modal should reappear"); assert.ok(find(".d-modal:visible").length === 1, "modal should reappear");
await keyEvent("#main-outlet", "keydown", 27); await keyEvent("#main-outlet", "keyup", 27);
assert.ok( assert.ok(
find(".d-modal:visible").length === 0, find(".d-modal:visible").length === 0,
"ESC should close the modal" "ESC should close the modal"
@ -47,7 +47,7 @@ QUnit.test("modal", async function(assert) {
find(".d-modal:visible").length === 1, find(".d-modal:visible").length === 1,
"modal should not disappear when you click outside" "modal should not disappear when you click outside"
); );
await keyEvent("#main-outlet", "keydown", 27); await keyEvent("#main-outlet", "keyup", 27);
assert.ok( assert.ok(
find(".d-modal:visible").length === 1, find(".d-modal:visible").length === 1,
"ESC should not close the modal" "ESC should not close the modal"
@ -61,7 +61,7 @@ QUnit.test("modal-keyboard-events", async function(assert) {
await click(".toggle-admin-menu"); await click(".toggle-admin-menu");
await click(".topic-admin-status-update button"); await click(".topic-admin-status-update button");
await keyEvent(".d-modal", "keydown", 13); await keyEvent(".d-modal", "keyup", 13);
assert.ok( assert.ok(
find("#modal-alert:visible").length === 1, find("#modal-alert:visible").length === 1,
@ -72,7 +72,7 @@ QUnit.test("modal-keyboard-events", async function(assert) {
"hitting Enter does not dismiss modal due to alert error" "hitting Enter does not dismiss modal due to alert error"
); );
await keyEvent("#main-outlet", "keydown", 27); await keyEvent("#main-outlet", "keyup", 27);
assert.ok( assert.ok(
find(".d-modal:visible").length === 0, find(".d-modal:visible").length === 0,
"ESC should close the modal" "ESC should close the modal"
@ -82,7 +82,7 @@ QUnit.test("modal-keyboard-events", async function(assert) {
await click(".d-editor-button-bar .btn.link"); await click(".d-editor-button-bar .btn.link");
await keyEvent(".d-modal", "keydown", 13); await keyEvent(".d-modal", "keyup", 13);
assert.ok( assert.ok(
find(".d-modal:visible").length === 0, find(".d-modal:visible").length === 0,
"modal should disappear on hitting Enter" "modal should disappear on hitting Enter"