From 7ab7e6bb239eafde49d02f5779704ea6d3d958d4 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 23 Aug 2024 09:28:28 +1000 Subject: [PATCH] FEATURE: allow plugins to specify keyboard shortcuts for hidden toolbar items (#28456) Previous to this change there is no clean way to apply keyboard shortcuts to things such as "add poll" and other hidden options in the toolbar This allows shortcuts to be specified similar to how they are on the toolbar Co-authored-by: Joffrey JAFFEUX --- .../discourse/app/components/d-editor.js | 46 +++++++++---- .../discourse/app/lib/plugin-api.gjs | 4 +- .../discourse/app/services/composer.js | 9 ++- .../tests/acceptance/composer-test.js | 69 +++++++++++++++++-- .../tests/acceptance/table-builder-test.js | 5 +- .../dropdown-select-box-row.hbs | 4 +- .../components/toolbar-popup-menu-options.js | 31 ++++++++- .../toolbar-popup-menu-options.scss | 20 +++++- docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md | 4 ++ .../acceptance/discourse-presence-test.js | 3 +- spec/system/table_builder_spec.rb | 6 +- 11 files changed, 165 insertions(+), 36 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/d-editor.js b/app/assets/javascripts/discourse/app/components/d-editor.js index 89f9b8aa634..c1fc8d1f6f4 100644 --- a/app/assets/javascripts/discourse/app/components/d-editor.js +++ b/app/assets/javascripts/discourse/app/components/d-editor.js @@ -322,6 +322,19 @@ export default Component.extend(TextareaTextManipulation, { }); }); + if (this.popupMenuOptions && this.onPopupMenuAction) { + this.popupMenuOptions.forEach((popupButton) => { + if (popupButton.shortcut && popupButton.condition) { + const shortcut = + `${PLATFORM_KEY_MODIFIER}+${popupButton.shortcut}`.toLowerCase(); + this._itsatrap.bind(shortcut, () => { + this.onPopupMenuAction(popupButton, this.newToolbarEvent()); + return false; + }); + } + }); + } + this._itsatrap.bind("tab", () => this.indentSelection("right")); this._itsatrap.bind("shift+tab", () => this.indentSelection("left")); this._itsatrap.bind(`${PLATFORM_KEY_MODIFIER}+shift+.`, () => @@ -785,6 +798,23 @@ export default Component.extend(TextareaTextManipulation, { } }, + newToolbarEvent(trimLeading) { + const selected = this.getSelected(trimLeading); + return { + selected, + selectText: (from, length) => + this.selectText(from, length, { scroll: false }), + applySurround: (head, tail, exampleKey, opts) => + this.applySurround(selected, head, tail, exampleKey, opts), + applyList: (head, exampleKey, opts) => + this._applyList(selected, head, exampleKey, opts), + formatCode: (...args) => this.send("formatCode", args), + addText: (text) => this.addText(selected, text), + getText: () => this.value, + toggleDirection: () => this._toggleDirection(), + }; + }, + actions: { emoji() { if (this.disabled) { @@ -799,21 +829,7 @@ export default Component.extend(TextareaTextManipulation, { return; } - const selected = this.getSelected(button.trimLeading); - const toolbarEvent = { - selected, - selectText: (from, length) => - this.selectText(from, length, { scroll: false }), - applySurround: (head, tail, exampleKey, opts) => - this.applySurround(selected, head, tail, exampleKey, opts), - applyList: (head, exampleKey, opts) => - this._applyList(selected, head, exampleKey, opts), - formatCode: (...args) => this.send("formatCode", args), - addText: (text) => this.addText(selected, text), - getText: () => this.value, - toggleDirection: () => this._toggleDirection(), - }; - + const toolbarEvent = this.newToolbarEvent(button.trimLeading); if (button.sendAction) { return button.sendAction(toolbarEvent); } else { diff --git a/app/assets/javascripts/discourse/app/lib/plugin-api.gjs b/app/assets/javascripts/discourse/app/lib/plugin-api.gjs index bc5a4ff6647..d7728765963 100644 --- a/app/assets/javascripts/discourse/app/lib/plugin-api.gjs +++ b/app/assets/javascripts/discourse/app/lib/plugin-api.gjs @@ -3,7 +3,7 @@ // 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.37.0"; +export const PLUGIN_API_VERSION = "1.37.1"; import $ from "jquery"; import { h } from "virtual-dom"; @@ -960,6 +960,7 @@ class PluginApi { * @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 {string} opts.shortcut - The keyboard shortcut to apply, NOTE: this will unconditionally add CTRL/META key (eg: m means CTRL+m). * @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. * @@ -970,6 +971,7 @@ class PluginApi { * }, * icon: 'far-bold', * label: 'composer.bold_some_text', + * shortcut: 'm', * condition: (composer) => { * return composer.editingPost; * } diff --git a/app/assets/javascripts/discourse/app/services/composer.js b/app/assets/javascripts/discourse/app/services/composer.js index 22d23aa42a2..d602cfb8855 100644 --- a/app/assets/javascripts/discourse/app/services/composer.js +++ b/app/assets/javascripts/discourse/app/services/composer.js @@ -665,7 +665,7 @@ export default class ComposerService extends Service { } @action - onPopupMenuAction(menuItem) { + onPopupMenuAction(menuItem, toolbarEvent) { // menuItem is an object with keys name & action like so: { name: "toggle-invisible, action: "toggleInvisible" } // `action` value can either be a string (to lookup action by) or a function to call this.appEvents.trigger( @@ -673,7 +673,12 @@ export default class ComposerService extends Service { menuItem ); if (typeof menuItem.action === "function") { - return menuItem.action(this.toolbarEvent); + // note due to the way args are passed to actions we need + // to treate the explicity toolbarEvent as a fallback for no + // event + // Long term we want to avoid needing this awkwardness and pass + // the event explicitly + return menuItem.action(this.toolbarEvent || toolbarEvent); } else { return ( this.actions?.[menuItem.action]?.bind(this) || // Legacy-style contributions from themes/plugins diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-test.js index 3d967acdd5a..3a952d7378d 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/composer-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/composer-test.js @@ -10,8 +10,10 @@ import { } from "@ember/test-helpers"; import { test } from "qunit"; import sinon from "sinon"; +import { PLATFORM_KEY_MODIFIER } from "discourse/lib/keyboard-shortcuts"; import LinkLookup from "discourse/lib/link-lookup"; import { withPluginApi } from "discourse/lib/plugin-api"; +import { translateModKey } from "discourse/lib/utilities"; import Composer, { CREATE_TOPIC, NEW_TOPIC_KEY, @@ -618,7 +620,7 @@ acceptance("Composer", function (needs) { await click(".topic-post:nth-of-type(1) button.reply"); await menu.expand(); - await menu.selectRowByName(I18n.t("composer.toggle_whisper")); + await menu.selectRowByName("toggle-whisper"); assert.strictEqual( count(".composer-actions svg.d-icon-far-eye-slash"), @@ -627,7 +629,7 @@ acceptance("Composer", function (needs) { ); await menu.expand(); - await menu.selectRowByName(I18n.t("composer.toggle_whisper")); + await menu.selectRowByName("toggle-whisper"); assert.ok( !exists(".composer-actions svg.d-icon-far-eye-slash"), @@ -635,14 +637,14 @@ acceptance("Composer", function (needs) { ); await menu.expand(); - await menu.selectRowByName(I18n.t("composer.toggle_whisper")); + await menu.selectRowByName("toggle-whisper"); await click(".toggle-fullscreen"); await menu.expand(); assert.ok( - menu.rowByName(I18n.t("composer.toggle_whisper")).exists(), + menu.rowByName("toggle-whisper").exists(), "whisper toggling is still present when going fullscreen" ); }); @@ -732,7 +734,7 @@ acceptance("Composer", function (needs) { await selectKit(".toolbar-popup-menu-options").expand(); await selectKit(".toolbar-popup-menu-options").selectRowByName( - I18n.t("composer.toggle_whisper") + "toggle-whisper" ); assert.strictEqual( @@ -752,7 +754,7 @@ acceptance("Composer", function (needs) { await selectKit(".toolbar-popup-menu-options").expand(); await selectKit(".toolbar-popup-menu-options").selectRowByName( - I18n.t("composer.toggle_unlisted") + "toggle-invisible" ); assert.ok( @@ -1404,6 +1406,61 @@ acceptance("composer buttons API", function (needs) { allow_uncategorized_topics: true, }); + test("buttons can support a shortcut", async function (assert) { + withPluginApi("0", (api) => { + api.addComposerToolbarPopupMenuOption({ + action: (toolbarEvent) => { + toolbarEvent.applySurround("**", "**"); + }, + shortcut: "alt+b", + icon: "far-bold", + name: "bold", + title: "some_title", + label: "some_label", + + condition: () => { + return true; + }, + }); + }); + + await visit("/t/internationalization-localization/280"); + await click(".post-controls button.reply"); + await fillIn(".d-editor-input", "hello the world"); + + const editor = document.querySelector(".d-editor-input"); + editor.setSelectionRange(6, 9); // select the text input in the composer + + await triggerKeyEvent( + ".d-editor-input", + "keydown", + "B", + Object.assign({ altKey: true }, metaModifier) + ); + + assert.strictEqual(editor.value, "hello **the** world", "it adds the bold"); + + const dropdown = selectKit(".toolbar-popup-menu-options"); + await dropdown.expand(); + + const row = dropdown.rowByName("bold").el(); + assert + .dom(row) + .hasAttribute( + "title", + I18n.t("some_title") + + ` (${translateModKey(PLATFORM_KEY_MODIFIER + "+alt+b")})`, + "it shows the title with shortcut" + ); + assert + .dom(row) + .hasText( + I18n.t("some_label") + + ` ${translateModKey(PLATFORM_KEY_MODIFIER + "+alt+b")}`, + "it shows the label with shortcut" + ); + }); + test("buttons can be added conditionally", async function (assert) { withPluginApi("0", (api) => { api.addComposerToolbarPopupMenuOption({ diff --git a/app/assets/javascripts/discourse/tests/acceptance/table-builder-test.js b/app/assets/javascripts/discourse/tests/acceptance/table-builder-test.js index c509104c25c..2f06a6ff94e 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/table-builder-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/table-builder-test.js @@ -2,7 +2,6 @@ import { click, visit } from "@ember/test-helpers"; import { test } from "qunit"; import { acceptance, exists } from "discourse/tests/helpers/qunit-helpers"; import selectKit from "discourse/tests/helpers/select-kit-helper"; -import I18n from "discourse-i18n"; acceptance("Table Builder", function (needs) { needs.user(); @@ -14,7 +13,7 @@ acceptance("Table Builder", function (needs) { await selectKit(".toolbar-popup-menu-options").expand(); assert - .dom(`.select-kit-row[data-name='${I18n.t("composer.insert_table")}']`) + .dom(`.select-kit-row[data-name='toggle-spreadsheet']`) .exists("it shows the builder button"); }); @@ -27,7 +26,7 @@ acceptance("Table Builder", function (needs) { await selectKit(".toolbar-popup-menu-options").expand(); assert - .dom(`.select-kit-row[data-name='${I18n.t("composer.insert_table")}']`) + .dom(`.select-kit-row[data-name='toggle-spreadsheet']`) .exists("it shows the builder button"); }); }); diff --git a/app/assets/javascripts/select-kit/addon/components/dropdown-select-box/dropdown-select-box-row.hbs b/app/assets/javascripts/select-kit/addon/components/dropdown-select-box/dropdown-select-box-row.hbs index 3e06f37917f..2b0c587650a 100644 --- a/app/assets/javascripts/select-kit/addon/components/dropdown-select-box/dropdown-select-box-row.hbs +++ b/app/assets/javascripts/select-kit/addon/components/dropdown-select-box/dropdown-select-box-row.hbs @@ -9,5 +9,7 @@
{{html-safe this.label}} - {{html-safe this.description}} + {{#if this.description}} + {{html-safe this.description}} + {{/if}}
\ No newline at end of file diff --git a/app/assets/javascripts/select-kit/addon/components/toolbar-popup-menu-options.js b/app/assets/javascripts/select-kit/addon/components/toolbar-popup-menu-options.js index e6859a9118e..bd6a262eadd 100644 --- a/app/assets/javascripts/select-kit/addon/components/toolbar-popup-menu-options.js +++ b/app/assets/javascripts/select-kit/addon/components/toolbar-popup-menu-options.js @@ -1,3 +1,5 @@ +import { PLATFORM_KEY_MODIFIER } from "discourse/lib/keyboard-shortcuts"; +import { translateModKey } from "discourse/lib/utilities"; import I18n from "discourse-i18n"; import DropdownSelectBoxComponent from "select-kit/components/dropdown-select-box"; @@ -17,9 +19,36 @@ export default DropdownSelectBoxComponent.extend({ return contents .map((content) => { if (content.condition) { + let label; + if (content.label) { + label = I18n.t(content.label); + if (content.shortcut) { + label += ` ${translateModKey( + PLATFORM_KEY_MODIFIER + )}+${translateModKey(content.shortcut)}`; + } + } + + let title; + if (content.title) { + title = I18n.t(content.title); + if (content.shortcut) { + title += ` (${translateModKey( + PLATFORM_KEY_MODIFIER + )}+${translateModKey(content.shortcut)})`; + } + } + + let name = content.name; + if (!name && content.label) { + name = I18n.t(content.label); + } + return { icon: content.icon, - name: I18n.t(content.label), + label, + title, + name, id: { name: content.name, action: content.action }, }; } diff --git a/app/assets/stylesheets/common/select-kit/toolbar-popup-menu-options.scss b/app/assets/stylesheets/common/select-kit/toolbar-popup-menu-options.scss index 21fd35e5997..db9bd6baf54 100644 --- a/app/assets/stylesheets/common/select-kit/toolbar-popup-menu-options.scss +++ b/app/assets/stylesheets/common/select-kit/toolbar-popup-menu-options.scss @@ -7,18 +7,34 @@ .select-kit-body { box-shadow: none; - width: 230px; border-radius: 0; } .select-kit-row { - padding: 0.75em 0.5em; + padding: 0.65em 0.5em; border-bottom: 1px solid rgba(var(--primary-low-rgb), 0.5); + display: flex; + align-items: center; + + // popup-menu doesnt use description text atm + // it's just easier to align the icon with text then + .icons { + align-self: center; + } + + .shortcut { + margin-left: 1em; + } &:last-child { border: none; } + .texts .name { + display: flex; + align-items: center; + } + .texts .name, .icons .d-icon { font-size: var(--font-0); diff --git a/docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md b/docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md index c32c02b73d8..08d1b943cc2 100644 --- a/docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md +++ b/docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md @@ -7,6 +7,10 @@ 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.37.1] - 2024-08-21 + +- Added support for `shortcut` in `addComposerToolbarPopupMenuOption` which allows to add a keyboard shortcut to the popup menu option. + ## [1.37.0] - 2024-08-19 - Added `addAboutPageActivity` which allows plugins/TCs to register a custom site activity item in the new /about page. Requires the server-side `register_stat` plugin API. diff --git a/plugins/discourse-presence/test/javascripts/acceptance/discourse-presence-test.js b/plugins/discourse-presence/test/javascripts/acceptance/discourse-presence-test.js index a21f6ca1312..972e184213f 100644 --- a/plugins/discourse-presence/test/javascripts/acceptance/discourse-presence-test.js +++ b/plugins/discourse-presence/test/javascripts/acceptance/discourse-presence-test.js @@ -13,7 +13,6 @@ import { query, } from "discourse/tests/helpers/qunit-helpers"; import selectKit from "discourse/tests/helpers/select-kit-helper"; -import I18n from "discourse-i18n"; acceptance("Discourse Presence Plugin", function (needs) { needs.user({ whisperer: true }); @@ -83,7 +82,7 @@ acceptance("Discourse Presence Plugin", function (needs) { const menu = selectKit(".toolbar-popup-menu-options"); await menu.expand(); - await menu.selectRowByName(I18n.t("composer.toggle_whisper")); + await menu.selectRowByName("toggle-whisper"); assert.strictEqual( count(".composer-actions svg.d-icon-far-eye-slash"), diff --git a/spec/system/table_builder_spec.rb b/spec/system/table_builder_spec.rb index 2b8eed0a83c..ef5ecb57559 100644 --- a/spec/system/table_builder_spec.rb +++ b/spec/system/table_builder_spec.rb @@ -33,7 +33,7 @@ describe "Table Builder", type: :system do visit("/latest") page.find("#create-topic").click page.find(".toolbar-popup-menu-options").click - page.find(".select-kit-row[data-name='Insert Table']").click + page.find(".select-kit-row[data-name='toggle-spreadsheet']").click insert_table_modal.type_in_cell(0, 0, "Item 1") insert_table_modal.type_in_cell(0, 1, "Item 2") insert_table_modal.type_in_cell(0, 2, "Item 3") @@ -59,7 +59,7 @@ describe "Table Builder", type: :system do visit("/latest") page.find("#create-topic").click page.find(".toolbar-popup-menu-options").click - page.find(".select-kit-row[data-name='Insert Table']").click + page.find(".select-kit-row[data-name='toggle-spreadsheet']").click insert_table_modal.cancel expect(page).to have_no_css(".insert-table-modal") end @@ -68,7 +68,7 @@ describe "Table Builder", type: :system do visit("/latest") page.find("#create-topic").click page.find(".toolbar-popup-menu-options").click - page.find(".select-kit-row[data-name='Insert Table']").click + page.find(".select-kit-row[data-name='toggle-spreadsheet']").click insert_table_modal.type_in_cell(0, 0, "Item 1") insert_table_modal.cancel expect(page).to have_css(".dialog-container .dialog-content")