FIX: Do not save bookmark if close (X) on modal is clicked (#9541)

FIX: Do not save bookmark if close (X) on modal is clicked (#9541)

  • After this change the bookmark will still be saved if clicking out of the modal or pressing escape
  • To achieve this I implemented an initiatedBy parameter for modal closing from d-modal. If clicking on the cross it is initiated by close, if clicking out of the modal it is by click out.
  • These options can then be compared in controllers consuming onClose

Co-authored-by: Joffrey JAFFEUX j.jaffeux@gmail.com

diff --git a/app/assets/javascripts/discourse/app/components/d-modal.js b/app/assets/javascripts/discourse/app/components/d-modal.js
index a237201..bc81971 100644
--- a/app/assets/javascripts/discourse/app/components/d-modal.js
+++ b/app/assets/javascripts/discourse/app/components/d-modal.js
@@ -76,10 +76,12 @@ export default Component.extend({
       $target.hasClass("modal-middle-container") ||
       $target.hasClass("modal-outer-container")
     ) {
-      // Delegate click to modal close if clicked outside.
-      // We do this because some CSS of ours seems to cover
-      // the backdrop and makes it unclickable.
-      $(".modal-header button.modal-close").click();
+      // Send modal close (which bubbles to ApplicationRoute) if clicked outside.
+      // We do this because some CSS of ours seems to cover the backdrop and makes
+      // it unclickable.
+      return (
+        this.attrs.closeModal && this.attrs.closeModal("initiatedByClickOut")
+      );
     }
   },
 
diff --git a/app/assets/javascripts/discourse/app/controllers/bookmark.js b/app/assets/javascripts/discourse/app/controllers/bookmark.js
index 604a6f2..30dc412 100644
--- a/app/assets/javascripts/discourse/app/controllers/bookmark.js
+++ b/app/assets/javascripts/discourse/app/controllers/bookmark.js
@@ -84,7 +84,11 @@ export default Controller.extend(ModalFunctionality, {
    * We always want to save the bookmark unless the user specifically
    * clicks the save or cancel button to mimic browser behaviour.
    */
-  onClose() {
+  onClose(opts = {}) {
+    if (opts.initiatedByCloseButton) {
+      this._closeWithoutSaving = true;
+    }
+
     this._unbindKeyboardShortcuts();
     this._restoreGlobalShortcuts();
     if (!this._closeWithoutSaving && !this._savingBookmarkManually) {
diff --git a/app/assets/javascripts/discourse/app/routes/application.js b/app/assets/javascripts/discourse/app/routes/application.js
index a16f97f..f164375 100644
--- a/app/assets/javascripts/discourse/app/routes/application.js
+++ b/app/assets/javascripts/discourse/app/routes/application.js
@@ -155,7 +155,7 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, {
     },
 
     // Close the current modal, and destroy its state.
-    closeModal() {
+    closeModal(initiatedBy) {
       const route = getOwner(this).lookup("route:application");
       let modalController = route.controllerFor("modal");
       const controllerName = modalController.get("name");
@@ -178,7 +178,10 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, {
           `controller:${controllerName}`
         );
         if (controller && controller.onClose) {
-          controller.onClose();
+          controller.onClose({
+            initiatedByCloseButton: initiatedBy === "initiatedByCloseButton",
+            initiatedByClickOut: initiatedBy === "initiatedByClickOut"
+          });
         }
         modalController.set("name", null);
       }
diff --git a/app/assets/javascripts/discourse/app/templates/components/d-modal.hbs b/app/assets/javascripts/discourse/app/templates/components/d-modal.hbs
index 1a81e46..e9e608d 100644
--- a/app/assets/javascripts/discourse/app/templates/components/d-modal.hbs
+++ b/app/assets/javascripts/discourse/app/templates/components/d-modal.hbs
@@ -3,7 +3,7 @@
     <div class="modal-inner-container">
       <div class="modal-header">
         {{#if dismissable}}
-          {{d-button icon="times" action=(route-action "closeModal") class="btn btn-flat modal-close close" title="modal.close"}}
+          {{d-button icon="times" action=(route-action "closeModal" "initiatedByCloseButton") class="btn-flat modal-close close" title="modal.close"}}
         {{/if}}
 
         {{#if panels}}

GitHub sha: 0d519c78

This commit appears in #9541 which was approved by eviltrout. It was merged by martin.