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.
This commit is contained in:
Alan Guo Xiang Tan 2023-10-06 07:43:40 +08:00 committed by GitHub
parent a27823fd3c
commit 913fd3a7b3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 185 additions and 172 deletions

View File

@ -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,
};

View File

@ -0,0 +1,9 @@
export const customPopupMenuOptions = [];
export function clearPopupMenuOptions() {
customPopupMenuOptions.length = 0;
}
export function addPopupMenuOption(option) {
customPopupMenuOptions.push(option);
}

View File

@ -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);
}
/**

View File

@ -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

View File

@ -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() {

View File

@ -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

View File

@ -6,26 +6,17 @@ 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",
});
}
@ -33,6 +24,6 @@ export default {
name: "apply-details",
initialize() {
withPluginApi("0.8.7", initializeDetails);
withPluginApi("1.14.0", initializeDetails);
},
};

View File

@ -1,13 +1,11 @@
import { acceptance, query } from "discourse/tests/helpers/qunit-helpers";
import I18n from "I18n";
import { clearPopupMenuOptionsCallback } from "discourse/controllers/composer";
import selectKit from "discourse/tests/helpers/select-kit-helper";
import { test } from "qunit";
import { click, fillIn, visit } from "@ember/test-helpers";
acceptance("Details Button", function (needs) {
needs.user();
needs.hooks.beforeEach(() => clearPopupMenuOptionsCallback());
test("details button", async function (assert) {
const popupMenu = selectKit(".toolbar-popup-menu-options");
@ -19,7 +17,7 @@ acceptance("Details Button", function (needs) {
await categoryChooser.selectRowByValue(2);
await popupMenu.expand();
await popupMenu.selectRowByValue("insertDetails");
await popupMenu.selectRowByName(I18n.t("details.title"));
assert.strictEqual(
query(".d-editor-input").value,
@ -36,7 +34,7 @@ acceptance("Details Button", function (needs) {
textarea.selectionEnd = textarea.value.length;
await popupMenu.expand();
await popupMenu.selectRowByValue("insertDetails");
await popupMenu.selectRowByName(I18n.t("details.title"));
assert.strictEqual(
query(".d-editor-input").value,
@ -63,7 +61,7 @@ acceptance("Details Button", function (needs) {
textarea.selectionEnd = 28;
await popupMenu.expand();
await popupMenu.selectRowByValue("insertDetails");
await popupMenu.selectRowByName(I18n.t("details.title"));
assert.strictEqual(
query(".d-editor-input").value,
@ -90,7 +88,7 @@ acceptance("Details Button", function (needs) {
textarea.selectionEnd = 29;
await popupMenu.expand();
await popupMenu.selectRowByValue("insertDetails");
await popupMenu.selectRowByName(I18n.t("details.title"));
assert.strictEqual(
query(".d-editor-input").value,
@ -128,7 +126,7 @@ acceptance("Details Button", function (needs) {
textarea.selectionEnd = textarea.value.length;
await popupMenu.expand();
await popupMenu.selectRowByValue("insertDetails");
await popupMenu.selectRowByName(I18n.t("details.title"));
assert.strictEqual(
query(".d-editor-input").value,

View File

@ -1,44 +1,28 @@
import discourseComputed from "discourse-common/utils/decorators";
import { withPluginApi } from "discourse/lib/plugin-api";
import PollUiBuilder from "../components/modal/poll-ui-builder";
import { getOwner } from "@ember/application";
function initializePollUIBuilder(api) {
api.modifyClass("controller:composer", {
pluginId: "discourse-poll-ui-builder",
@discourseComputed(
"siteSettings.poll_enabled",
"siteSettings.poll_minimum_trust_level_to_create",
"model.topic.pm_with_non_human_user"
)
canBuildPoll(pollEnabled, minimumTrustLevel, pmWithNonHumanUser) {
api.addComposerToolbarPopupMenuOption({
action: (toolbarEvent) => {
api.container.lookup("service:modal").show(PollUiBuilder, {
model: { toolbarEvent },
});
},
icon: "chart-bar",
label: "poll.ui_builder.title",
condition: (composer) => {
const siteSettings = api.container.lookup("service:site-settings");
const currentUser = api.getCurrentUser();
return (
pollEnabled &&
(pmWithNonHumanUser ||
(this.currentUser &&
(this.currentUser.staff ||
this.currentUser.trust_level >= minimumTrustLevel)))
siteSettings.poll_enabled &&
(composer.model.topic?.pm_with_non_human_user ||
(currentUser &&
(currentUser.staff ||
currentUser.trust_level >=
siteSettings.poll_minimum_trust_level_to_create)))
);
},
actions: {
showPollBuilder() {
getOwner(this)
.lookup("service:modal")
.show(PollUiBuilder, {
model: { toolbarEvent: this.toolbarEvent },
});
},
},
});
api.addToolbarPopupMenuOptionsCallback(() => {
return {
action: "showPollBuilder",
icon: "chart-bar",
label: "poll.ui_builder.title",
condition: "canBuildPoll",
};
});
}
@ -46,6 +30,6 @@ export default {
name: "add-poll-ui-builder",
initialize() {
withPluginApi("0.8.7", initializePollUIBuilder);
withPluginApi("1.14.0", initializePollUIBuilder);
},
};

View File

@ -4,7 +4,6 @@ import {
exists,
query,
} from "discourse/tests/helpers/qunit-helpers";
import { clearPopupMenuOptionsCallback } from "discourse/controllers/composer";
import { test } from "qunit";
import { click, visit } from "@ember/test-helpers";
@ -14,7 +13,7 @@ acceptance("Poll breakdown", function (needs) {
poll_enabled: true,
poll_groupable_user_fields: "something",
});
needs.hooks.beforeEach(() => clearPopupMenuOptionsCallback());
needs.pretender((server, helper) => {
server.get("/polls/grouped_poll_results.json", () =>
helper.response({

View File

@ -3,7 +3,6 @@ import {
exists,
updateCurrentUser,
} from "discourse/tests/helpers/qunit-helpers";
import { clearPopupMenuOptionsCallback } from "discourse/controllers/composer";
import { displayPollBuilderButton } from "discourse/plugins/poll/helpers/display-poll-builder-button";
import { test } from "qunit";
@ -13,7 +12,6 @@ acceptance("Poll Builder - polls are disabled", function (needs) {
poll_enabled: false,
poll_minimum_trust_level_to_create: 2,
});
needs.hooks.beforeEach(() => clearPopupMenuOptionsCallback());
test("regular user - sufficient trust level", async function (assert) {
updateCurrentUser({ moderator: false, admin: false, trust_level: 3 });

View File

@ -1,10 +1,10 @@
import I18n from "I18n";
import {
acceptance,
exists,
updateCurrentUser,
} from "discourse/tests/helpers/qunit-helpers";
import { click } from "@ember/test-helpers";
import { clearPopupMenuOptionsCallback } from "discourse/controllers/composer";
import { displayPollBuilderButton } from "discourse/plugins/poll/helpers/display-poll-builder-button";
import { test } from "qunit";
@ -14,29 +14,34 @@ acceptance("Poll Builder - polls are enabled", function (needs) {
poll_enabled: true,
poll_minimum_trust_level_to_create: 1,
});
needs.hooks.beforeEach(() => clearPopupMenuOptionsCallback());
test("regular user - sufficient trust level", async function (assert) {
updateCurrentUser({ moderator: false, admin: false, trust_level: 1 });
await displayPollBuilderButton();
assert.ok(
exists(".select-kit-row[data-value='showPollBuilder']"),
"it shows the builder button"
);
const pollBuilderButtonSelector = `.select-kit-row[data-name='${I18n.t(
"poll.ui_builder.title"
)}']`;
assert.dom(pollBuilderButtonSelector).exists("it shows the builder button");
await click(pollBuilderButtonSelector);
await click(".select-kit-row[data-value='showPollBuilder']");
assert.true(
exists(".poll-type-value-regular.active"),
"regular type is active"
);
await click(".poll-type-value-multiple");
assert.true(
exists(".poll-type-value-multiple.active"),
"multiple type is active"
);
await click(".poll-type-value-regular");
assert.true(
exists(".poll-type-value-regular.active"),
"regular type is active"
@ -59,9 +64,8 @@ acceptance("Poll Builder - polls are enabled", function (needs) {
await displayPollBuilderButton();
assert.ok(
exists(".select-kit-row[data-value='showPollBuilder']"),
"it shows the builder button"
);
assert
.dom(`.select-kit-row[data-name='${I18n.t("poll.ui_builder.title")}']`)
.exists("it shows the builder button");
});
});

View File

@ -1,14 +1,10 @@
import { acceptance, exists } from "discourse/tests/helpers/qunit-helpers";
import { clearPopupMenuOptionsCallback } from "discourse/controllers/composer";
import { test } from "qunit";
import { click, visit } from "@ember/test-helpers";
acceptance("Poll in a post reply history", function (needs) {
needs.user();
needs.settings({ poll_enabled: true });
needs.hooks.beforeEach(() => {
clearPopupMenuOptionsCallback();
});
needs.pretender((server, helper) => {
server.get("/t/topic_with_poll_in_post_reply_history.json", () => {

View File

@ -1,14 +1,10 @@
import { acceptance, count } from "discourse/tests/helpers/qunit-helpers";
import { clearPopupMenuOptionsCallback } from "discourse/controllers/composer";
import { test } from "qunit";
import { click, visit } from "@ember/test-helpers";
acceptance("Poll quote", function (needs) {
needs.user();
needs.settings({ poll_enabled: true });
needs.hooks.beforeEach(() => {
clearPopupMenuOptionsCallback();
});
needs.pretender((server, helper) => {
server.get("/posts/by_number/130/1", () => {

View File

@ -5,15 +5,11 @@ import {
publishToMessageBus,
} from "discourse/tests/helpers/qunit-helpers";
import { test } from "qunit";
import { clearPopupMenuOptionsCallback } from "discourse/controllers/composer";
import { click, visit } from "@ember/test-helpers";
acceptance("Poll results", function (needs) {
needs.user();
needs.settings({ poll_enabled: true });
needs.hooks.beforeEach(() => {
clearPopupMenuOptionsCallback();
});
needs.pretender((server, helper) => {
server.get("/posts/by_number/134/1", () => {
@ -659,9 +655,6 @@ acceptance("Poll results", function (needs) {
acceptance("Poll results - no voters", function (needs) {
needs.user();
needs.settings({ poll_enabled: true });
needs.hooks.beforeEach(() => {
clearPopupMenuOptionsCallback();
});
needs.pretender((server, helper) => {
server.get("/posts/by_number/134/1", () => {

View File

@ -1,14 +1,11 @@
import { acceptance, queryAll } from "discourse/tests/helpers/qunit-helpers";
import { clearPopupMenuOptionsCallback } from "discourse/controllers/composer";
import { test } from "qunit";
import { click, visit } from "@ember/test-helpers";
acceptance("Rendering polls with bar charts - desktop", function (needs) {
needs.user();
needs.settings({ poll_enabled: true });
needs.hooks.beforeEach(() => {
clearPopupMenuOptionsCallback();
});
needs.pretender((server, helper) => {
server.get("/polls/voters.json", (request) => {
let body = {};

View File

@ -1,5 +1,4 @@
import { acceptance, queryAll } from "discourse/tests/helpers/qunit-helpers";
import { clearPopupMenuOptionsCallback } from "discourse/controllers/composer";
import { test } from "qunit";
import { click, visit } from "@ember/test-helpers";
@ -19,9 +18,6 @@ acceptance("Rendering polls with bar charts - mobile", function (needs) {
});
});
});
needs.hooks.beforeEach(() => {
clearPopupMenuOptionsCallback();
});
test("Public number poll", async function (assert) {
await visit("/t/-/13");