DEV: Improve addToolbarPopupMenuOptionsCallback plugin api (#23769)

Why this change? Previously just using the `addToolbarPopupMenuOptionsCallback` plugin API itself was insufficient because it required the return object to include an `action` key which only accepted a name of the action function as a string. This was highly problematic because the action function had to be defined on the `composer` service which means using the `modifyClass` API to add the action function. This made the API awkward to use leading to poor developer experiencec. What does this change do? This commit introduces a couple of improvemnts to the API. 1. First the API has been renamed to `addComposerToolbarPopupMenuOption` because the API no longer accepts a callback function which was quite redundant. Instead, it now accepts an Object. The `addToolbarPopupMenuOptionsCallback` API function is deprecated and will be dropped in Discourse 3.3. Note that passing the API a function is still supported but will be dropped when the `addToolbarPopupMenuOptionsCallback` is removed. 2. The `action` key in the Object passed to the function can now be a function and is passed the `toolbarEvent` object when called. 3. The `condition` on key in the Object passed to the function can now be a function and is passed the `composer` service when called.
discourse · Oct 5, 2023 · 913fd3a · 913fd3a
1 parent a27823f
commit 913fd3a
Show file tree

Hide file tree

Showing 17 changed files with 185 additions and 172 deletions.
diff --git a/app/assets/javascripts/discourse/app/controllers/composer.js b/app/assets/javascripts/discourse/app/controllers/composer.js
@@ -1,19 +1,38 @@
 import Composer, {
   addComposerSaveErrorCallback,
-  addPopupMenuOptionsCallback,
   clearComposerSaveErrorCallback,
-  clearPopupMenuOptionsCallback,
   toggleCheckDraftPopup,
 } from "discourse/services/composer";
 
+import {
+  addPopupMenuOption,
+  clearPopupMenuOptions,
+} from "discourse/lib/composer/custom-popup-menu-options";
+
+import deprecated from "discourse-common/lib/deprecated";
+
 // TODO add deprecation
 
 export default Composer;
 
+function clearPopupMenuOptionsCallback() {
+  deprecated(
+    "`clearPopupMenuOptionsCallback` is deprecated without replacement as the cleanup is handled automatically.",
+    {
+      id: "discourse.composer-controller.clear-popup-menu-options-callback",
+      since: "3.2",
+      dropFrom: "3.3",
+    }
+  );
+
+  clearPopupMenuOptions();
+}
+
 export {
   addComposerSaveErrorCallback,
-  addPopupMenuOptionsCallback,
+  addPopupMenuOption,
   clearComposerSaveErrorCallback,
+  clearPopupMenuOptions,
   clearPopupMenuOptionsCallback,
   toggleCheckDraftPopup,
 };
diff --git a/app/assets/javascripts/discourse/app/lib/composer/custom-popup-menu-options.js b/app/assets/javascripts/discourse/app/lib/composer/custom-popup-menu-options.js
@@ -0,0 +1,9 @@
+export const customPopupMenuOptions = [];
+
+export function clearPopupMenuOptions() {
+  customPopupMenuOptions.length = 0;
+}
+
+export function addPopupMenuOption(option) {
+  customPopupMenuOptions.push(option);
+}
diff --git a/app/assets/javascripts/discourse/app/lib/plugin-api.js b/app/assets/javascripts/discourse/app/lib/plugin-api.js
@@ -58,10 +58,8 @@ import {
   addPluginReviewableParam,
   registerReviewableActionModal,
 } from "discourse/components/reviewable-item";
-import {
-  addComposerSaveErrorCallback,
-  addPopupMenuOptionsCallback,
-} from "discourse/services/composer";
+import { addComposerSaveErrorCallback } from "discourse/services/composer";
+import { addPopupMenuOption } from "discourse/lib/composer/custom-popup-menu-options";
 import { addPostClassesCallback } from "discourse/widgets/post";
 import {
   addGroupPostSmallActionCode,
@@ -137,7 +135,7 @@ import { isTesting } from "discourse-common/config/environment";
 // docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md whenever you change the version
 // using the format described at https://keepachangelog.com/en/1.0.0/.
 
-export const PLUGIN_API_VERSION = "1.13.0";
+export const PLUGIN_API_VERSION = "1.14.0";
 
 // This helper prevents us from applying the same `modifyClass` over and over in test mode.
 function canModify(klass, type, resolverName, changes) {
@@ -724,23 +722,50 @@ class PluginApi {
   }
 
   /**
-   * Add a new button in the options popup menu.
+   * Add a new button in the composer's toolbar options popup menu.
    *
-   * Example:
+   * @callback action
+   * @param {Object} toolbarEvent - A toolbar event object.
+   * @param {function} toolbarEvent.applySurround - Surrounds the selected text with the given text.
+   * @param {function} toolbarEvent.addText - Append the given text to the selected text in the composer.
    *
-   * ```
-   * api.addToolbarPopupMenuOptionsCallback(() => {
-   *  return {
-   *    action: 'toggleWhisper',
-   *    icon: 'far-eye-slash',
-   *    label: 'composer.toggle_whisper',
-   *    condition: "canWhisper"
-   *  };
+   * @callback condition
+   * @param {Object} composer - The composer service object.
+   * @returns {boolean} - Whether the button should be displayed.
+   *
+   * @param {Object} opts - An Object.
+   * @param {string} opts.icon - The name of the FontAwesome icon to display for the button.
+   * @param {string} opts.label - The I18n translation key for the button's label.
+   * @param {action} opts.action - The action to perform when the button is clicked.
+   * @param {condition} opts.condition - A condition that must be met for the button to be displayed.
+   *
+   * @example
+   * api.addComposerToolbarPopupMenuOption({
+   *   action: (toolbarEvent) => {
+   *     toolbarEvent.applySurround("**", "**");
+   *   },
+   *   icon: 'far-bold',
+   *   label: 'composer.bold_some_text',
+   *   condition: (composer) => {
+   *     return composer.editingPost;
+   *   }
    * });
-   * ```
    **/
-  addToolbarPopupMenuOptionsCallback(callback) {
-    addPopupMenuOptionsCallback(callback);
+  addComposerToolbarPopupMenuOption(opts) {
+    addPopupMenuOption(opts);
+  }
+
+  addToolbarPopupMenuOptionsCallback(opts) {
+    deprecated(
+      "`addToolbarPopupMenuOptionsCallback` has been renamed to `addToolbarPopupMenuOption`",
+      {
+        id: "discourse.add-toolbar-popup-menu-options-callback",
+        since: "3.3",
+        dropFrom: "3.4",
+      }
+    );
+
+    this.addComposerToolbarPopupMenuOption(opts);
   }
 
   /**

diff --git a/app/assets/javascripts/discourse/app/services/composer.js b/app/assets/javascripts/discourse/app/services/composer.js
@@ -40,6 +40,7 @@ import prepareFormTemplateData from "discourse/lib/form-template-validation";
 import DiscardDraftModal from "discourse/components/modal/discard-draft";
 import PostEnqueuedModal from "discourse/components/modal/post-enqueued";
 import { disableImplicitInjections } from "discourse/lib/implicit-injections";
+import { customPopupMenuOptions } from "discourse/lib/composer/custom-popup-menu-options";
 
 async function loadDraft(store, opts = {}) {
   let { draft, draftKey, draftSequence } = opts;
@@ -75,7 +76,6 @@ async function loadDraft(store, opts = {}) {
   return composer;
 }
 
-const _popupMenuOptionsCallbacks = [];
 const _composerSaveErrorCallbacks = [];
 
 let _checkDraftPopup = !isTesting();
@@ -84,14 +84,6 @@ export function toggleCheckDraftPopup(enabled) {
   _checkDraftPopup = enabled;
 }
 
-export function clearPopupMenuOptionsCallback() {
-  _popupMenuOptionsCallbacks.length = 0;
-}
-
-export function addPopupMenuOptionsCallback(callback) {
-  _popupMenuOptionsCallbacks.push(callback);
-}
-
 export function clearComposerSaveErrorCallback() {
   _composerSaveErrorCallbacks.length = 0;
 }
@@ -342,16 +334,25 @@ export default class ComposerService extends Service {
     return whisperer && modelAction === Composer.REPLY;
   }
 
-  _setupPopupMenuOption(callback) {
-    let option = callback(this);
+  _setupPopupMenuOption(option) {
+    // Backwards compatibility support for when we used to accept a function.
+    // This can be dropped when `addToolbarPopupMenuOptionsCallback` is removed from `plugin-api.js`.
+    if (typeof option === "function") {
+      option = option(this);
+    }
+
     if (typeof option === "undefined") {
       return null;
     }
 
-    if (typeof option.condition === "undefined") {
+    const conditionType = typeof option.condition;
+
+    if (conditionType === "undefined") {
       option.condition = true;
-    } else if (typeof option.condition === "boolean") {
+    } else if (conditionType === "boolean") {
       // uses existing value
+    } else if (conditionType === "function") {
+      option.condition = option.condition(this);
     } else {
       option.condition = this.get(option.condition);
     }
@@ -370,62 +371,52 @@ export default class ComposerService extends Service {
       const options = [];
 
       options.push(
-        this._setupPopupMenuOption(() => {
-          return {
-            action: "toggleInvisible",
-            icon: "far-eye-slash",
-            label: "composer.toggle_unlisted",
-            condition: "canUnlistTopic",
-          };
+        this._setupPopupMenuOption({
+          action: "toggleInvisible",
+          icon: "far-eye-slash",
+          label: "composer.toggle_unlisted",
+          condition: "canUnlistTopic",
         })
       );
 
       if (this.capabilities.touch) {
         options.push(
-          this._setupPopupMenuOption(() => {
-            return {
-              action: "applyFormatCode",
-              icon: "code",
-              label: "composer.code_title",
-            };
+          this._setupPopupMenuOption({
+            action: "applyFormatCode",
+            icon: "code",
+            label: "composer.code_title",
           })
         );
 
         options.push(
-          this._setupPopupMenuOption(() => {
-            return {
-              action: "applyUnorderedList",
-              icon: "list-ul",
-              label: "composer.ulist_title",
-            };
+          this._setupPopupMenuOption({
+            action: "applyUnorderedList",
+            icon: "list-ul",
+            label: "composer.ulist_title",
           })
         );
 
         options.push(
-          this._setupPopupMenuOption(() => {
-            return {
-              action: "applyOrderedList",
-              icon: "list-ol",
-              label: "composer.olist_title",
-            };
+          this._setupPopupMenuOption({
+            action: "applyOrderedList",
+            icon: "list-ol",
+            label: "composer.olist_title",
           })
         );
       }
 
       options.push(
-        this._setupPopupMenuOption(() => {
-          return {
-            action: "toggleWhisper",
-            icon: "far-eye-slash",
-            label: "composer.toggle_whisper",
-            condition: "showWhisperToggle",
-          };
+        this._setupPopupMenuOption({
+          action: "toggleWhisper",
+          icon: "far-eye-slash",
+          label: "composer.toggle_whisper",
+          condition: "showWhisperToggle",
         })
       );
 
       return options.concat(
-        _popupMenuOptionsCallbacks
-          .map((callback) => this._setupPopupMenuOption(callback))
+        customPopupMenuOptions
+          .map((option) => this._setupPopupMenuOption(option))
           .filter((o) => o)
       );
     }
@@ -600,10 +591,14 @@ export default class ComposerService extends Service {
 
   @action
   onPopupMenuAction(menuAction) {
-    return (
-      this.actions?.[menuAction]?.bind(this) || // Legacy-style contributions from themes/plugins
-      this[menuAction]
-    )();
+    if (typeof menuAction === "function") {
+      return menuAction(this.toolbarEvent);
+    } else {
+      return (
+        this.actions?.[menuAction]?.bind(this) || // Legacy-style contributions from themes/plugins
+        this[menuAction]
+      )();
+    }
   }
 
   @action

diff --git a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js
@@ -93,6 +93,7 @@ import { resetModelTransformers } from "discourse/lib/model-transformers";
 import { cleanupTemporaryModuleRegistrations } from "./temporary-module-helper";
 import { clearBulkButtons } from "discourse/components/modal/topic-bulk-actions";
 import { resetBeforeAuthCompleteCallbacks } from "discourse/instance-initializers/auth-complete";
+import { clearPopupMenuOptions } from "discourse/lib/composer/custom-popup-menu-options";
 
 export function currentUser() {
   return User.create(sessionFixtures["/session/current.json"].current_user);
@@ -231,6 +232,7 @@ export function testCleanup(container, app) {
   cleanupCssGeneratorTags();
   clearBulkButtons();
   resetBeforeAuthCompleteCallbacks();
+  clearPopupMenuOptions();
 }
 
 function cleanupCssGeneratorTags() {

diff --git a/docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md b/docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md
@@ -7,6 +7,17 @@ in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [1.14.0] - 2023-10-06
+
+### Added
+
+- Added `addComposerToolbarPopupMenuOption` as a replacement for `addToolbarPopupMenuOptionsCallback` with new changes
+  introduced to the method's signature.
+
+### Changed
+
+- Deprecate `addToolbarPopupMenuOptionsCallback` in favor of `addComposerToolbarPopupMenuOption`.
+
 ## [1.13.0] - 2023-10-05
 
 ### Added

diff --git a/plugins/discourse-details/assets/javascripts/initializers/apply-details.js b/plugins/discourse-details/assets/javascripts/initializers/apply-details.js
@@ -6,33 +6,24 @@ function initializeDetails(api) {
     id: "discourse-details",
   });
 
-  api.addToolbarPopupMenuOptionsCallback(() => {
-    return {
-      action: "insertDetails",
-      icon: "caret-right",
-      label: "details.title",
-    };
-  });
-
-  api.modifyClass("controller:composer", {
-    pluginId: "discourse-details",
-    actions: {
-      insertDetails() {
-        this.toolbarEvent.applySurround(
-          "\n" + `[details="${I18n.t("composer.details_title")}"]` + "\n",
-          "\n[/details]\n",
-          "details_text",
-          { multiline: false }
-        );
-      },
+  api.addComposerToolbarPopupMenuOption({
+    action: function (toolbarEvent) {
+      toolbarEvent.applySurround(
+        "\n" + `[details="${I18n.t("composer.details_title")}"]` + "\n",
+        "\n[/details]\n",
+        "details_text",
+        { multiline: false }
+      );
     },
+    icon: "caret-right",
+    label: "details.title",
   });
 }
 
 export default {
   name: "apply-details",
 
   initialize() {
-    withPluginApi("0.8.7", initializeDetails);
+    withPluginApi("1.14.0", initializeDetails);
   },
 };