From aa12e9853e3a047a50365c4be7ed41e01f91cc10 Mon Sep 17 00:00:00 2001 From: "Francois (fge)" Date: Mon, 9 Aug 2021 09:19:48 +0000 Subject: [PATCH] [IMP] web: improved api of commands generated by a provider --- .../static/src/core/debug/debug_providers.js | 20 ++-- .../src/webclient/commands/command_palette.js | 49 ++++++---- .../webclient/commands/command_palette.xml | 16 +--- .../src/webclient/commands/command_service.js | 10 +- .../webclient/commands/default_providers.js | 33 ++++--- .../commands/command_service_tests.js | 95 +++++++++++++++++-- 6 files changed, 160 insertions(+), 63 deletions(-) diff --git a/addons/web/static/src/core/debug/debug_providers.js b/addons/web/static/src/core/debug/debug_providers.js index 24e839fbf50..2bcba19e131 100644 --- a/addons/web/static/src/core/debug/debug_providers.js +++ b/addons/web/static/src/core/debug/debug_providers.js @@ -13,18 +13,16 @@ commandProviderRegistry.add("debug", { if (env.services.user.isAdmin) { if (env.debug) { result.push({ - name: "Deactivate debug mode", - category: "debug", - action: () => { + action() { const route = env.services.router.current; route.search.debug = ""; browser.location.href = browser.location.origin + routeToUrl(route); }, + category: "debug", + name: "Deactivate debug mode", }); result.push({ - name: "Debug menu", - category: "debug", - action: () => { + action() { return { placeHolder: "Choose a debug action...", provide: async (env) => { @@ -33,20 +31,22 @@ commandProviderRegistry.add("debug", { return items .filter((item) => item.type === "item") .map((item) => ({ - name: item.description, action: item.callback, + name: item.description, })); }, }; }, + category: "debug", + name: "Debug menu", }); } else { result.push({ - name: "Activate debug mode", - category: "debug", - action: () => { + action() { browser.location.search = "?debug=assets"; }, + category: "debug", + name: "Activate debug mode", }); } } diff --git a/addons/web/static/src/webclient/commands/command_palette.js b/addons/web/static/src/webclient/commands/command_palette.js index 906395c775e..c58a16588e9 100644 --- a/addons/web/static/src/webclient/commands/command_palette.js +++ b/addons/web/static/src/webclient/commands/command_palette.js @@ -18,7 +18,20 @@ const commandCategoryRegistry = registry.category("command_categories"); const commandProviderRegistry = registry.category("command_provider"); /** - * @typedef {import("./command_service").Command} Command + * @typedef {{ + * Component?: Component, + * action: ()=>(void | NextProvider), + * category?: string, + * name: string, + * props?: Object, + * }} CommandItem + */ + +/** + * @typedef {{ + * placeHolder?: string, + * provide: ()=>CommandItem[], + * }} NextProvider */ /** @@ -38,7 +51,7 @@ function commandsWithinCategory(categoryName) { } export class DefaultCommandItem extends Component {} -DefaultCommandItem.template = "web.defaultCommandItem"; +DefaultCommandItem.template = "web.DefaultCommandItem"; export class CommandPalette extends Component { setup() { @@ -57,7 +70,7 @@ export class CommandPalette extends Component { useHotkey("ArrowDown", () => this.selectCommandAndScrollTo("NEXT"), { allowRepeat: true }); /** - * @type {{commands: Command[], selectedCommand: Command, placeHolder: String}} + * @type {{commands: CommandItem[], selectedCommand: CommandItem, placeHolder: String}} */ this.state = useState({ commands: [], @@ -174,20 +187,24 @@ export class CommandPalette extends Component { this.executeSelectedCommand(); } - async executeSelectedCommand() { + async executeCommand(command) { + const result = await command.action(); + if (result) { + const { placeHolder, provide } = result; + this.providerStack.push({ __default__: [{ provide }] }); + this.setCommands("__default__", { + activeElement: this.activeElement, + }); + this.state.placeHolder = placeHolder || DEFAULT_PLACEHOLDER; + this.searchBar.el.value = ""; + } else { + this.props.closeMe(); + } + } + + executeSelectedCommand() { if (this.state.selectedCommand) { - const result = await this.state.selectedCommand.action(); - if (result) { - const { placeHolder, provide } = result; - this.providerStack.push({ __default__: [{ provide }] }); - this.setCommands("__default__", { - activeElement: this.activeElement, - }); - this.state.placeHolder = placeHolder || DEFAULT_PLACEHOLDER; - this.searchBar.el.value = ""; - } else { - this.props.closeMe(); - } + this.executeCommand(this.state.selectedCommand); } } diff --git a/addons/web/static/src/webclient/commands/command_palette.xml b/addons/web/static/src/webclient/commands/command_palette.xml index 6a0a0218530..d7b6c6c4f75 100644 --- a/addons/web/static/src/webclient/commands/command_palette.xml +++ b/addons/web/static/src/webclient/commands/command_palette.xml @@ -19,7 +19,7 @@ - + @@ -29,30 +29,22 @@ -
- - - - - - -
- +
- +
- + + diff --git a/addons/web/static/src/webclient/commands/command_service.js b/addons/web/static/src/webclient/commands/command_service.js index c15ceab6522..28e602b0da2 100644 --- a/addons/web/static/src/webclient/commands/command_service.js +++ b/addons/web/static/src/webclient/commands/command_service.js @@ -3,10 +3,14 @@ import { registry } from "@web/core/registry"; import { CommandPaletteDialog } from "./command_palette_dialog"; +/** + * @typedef {import("./command_palette").NextProvider} NextProvider + */ + /** * @typedef {{ * name: string, - * action: ()=>void, + * action: ()=>(void | NextProvider), * category?: string, * hotkey?: string, * hotkeyOptions?: any, @@ -114,6 +118,10 @@ export const commandService = { unregisterCommand(token); }; }, + /** + * @param {HTMLElement} activeElement + * @returns {Command[]} + */ getCommands(activeElement) { return [...registeredCommands.values()].filter( (command) => command.activeElement === activeElement diff --git a/addons/web/static/src/webclient/commands/default_providers.js b/addons/web/static/src/webclient/commands/default_providers.js index ab2b942aa8e..615992da6eb 100644 --- a/addons/web/static/src/webclient/commands/default_providers.js +++ b/addons/web/static/src/webclient/commands/default_providers.js @@ -9,14 +9,11 @@ import { DefaultCommandItem } from "./command_palette"; const { Component } = owl; -class HotkeyCommandItem extends Component { +export class HotkeyCommandItem extends Component { setup() { - if (this.props.hotkey) { - useHotkey(this.props.hotkey, () => { - this.props.action(); - this.trigger("close"); - }); - } + useHotkey(this.props.hotkey, () => { + this.trigger("execute-command"); + }); } getKeysToPress(command) { @@ -30,15 +27,21 @@ class HotkeyCommandItem extends Component { return result.map((key) => key.toUpperCase()); } } -HotkeyCommandItem.template = "web.hotkeyCommandItem"; +HotkeyCommandItem.template = "web.HotkeyCommandItem"; const commandProviderRegistry = registry.category("command_provider"); commandProviderRegistry.add("command", { provide: (env, options = {}) => { const commands = env.services.command.getCommands(options.activeElement); return commands.map((command) => ({ - ...command, Component: command.hotkey ? HotkeyCommandItem : DefaultCommandItem, + action: command.action, + category: command.category, + name: command.name, + props: { + hotkey: command.hotkey, + hotkeyOptions: command.hotkeyOptions, + }, })); }, }); @@ -63,19 +66,19 @@ commandProviderRegistry.add("data-hotkeys", { `${el.innerText.slice(0, 50)}${el.innerText.length > 50 ? "..." : ""}`) || "no description provided"; commands.push({ - name: capitalize(description.trim().toLowerCase()), - hotkey: `${overlayModifier}+${el.dataset.hotkey}`, + Component: HotkeyCommandItem, action: () => { // AAB: not sure it is enough, we might need to trigger all events that occur when you actually click el.focus(); el.click(); }, category, + name: capitalize(description.trim().toLowerCase()), + props: { + hotkey: `${overlayModifier}+${el.dataset.hotkey}`, + }, }); } - return commands.map((command) => ({ - ...command, - Component: HotkeyCommandItem, - })); + return commands; }, }); diff --git a/addons/web/static/tests/webclient/commands/command_service_tests.js b/addons/web/static/tests/webclient/commands/command_service_tests.js index 34ec8b9483d..d2cc07474d8 100644 --- a/addons/web/static/tests/webclient/commands/command_service_tests.js +++ b/addons/web/static/tests/webclient/commands/command_service_tests.js @@ -2,14 +2,14 @@ import { browser } from "@web/core/browser/browser"; import { dialogService } from "@web/core/dialog/dialog_service"; +import { hotkeyService } from "@web/core/hotkeys/hotkey_service"; import { registry } from "@web/core/registry"; import { uiService, useActiveElement } from "@web/core/ui/ui_service"; import { useCommand } from "@web/webclient/commands/command_hook"; import { commandService } from "@web/webclient/commands/command_service"; -import { hotkeyService } from "@web/core/hotkeys/hotkey_service"; -import { makeTestEnv } from "../../helpers/mock_env"; +import { HotkeyCommandItem } from "@web/webclient/commands/default_providers"; +import { clearRegistryWithCleanup, makeTestEnv } from "../../helpers/mock_env"; import { click, getFixture, nextTick, patchWithCleanup, triggerHotkey } from "../../helpers/utils"; -import { clearRegistryWithCleanup } from "../../helpers/mock_env"; const { Component, mount, tags } = owl; const { xml } = tags; @@ -358,8 +358,8 @@ QUnit.test("defined multiple providers with the same nameSpace", async (assert) nameSpace: "__default__", provide: () => defaultNames.map((name) => ({ - name, action, + name, })), }); @@ -367,8 +367,8 @@ QUnit.test("defined multiple providers with the same nameSpace", async (assert) commandProviderRegistry.add("other", { provide: () => otherNames.map((name) => ({ - name, action, + name, })), }); @@ -408,8 +408,8 @@ QUnit.test("can switch between command providers", async (assert) => { commandProviderRegistry.add("default", { provide: () => defaultNames.map((name) => ({ - name, action, + name, })), }); @@ -418,8 +418,8 @@ QUnit.test("can switch between command providers", async (assert) => { nameSpace: "@", provide: () => otherNames.map((name) => ({ - name, action, + name, })), }); @@ -483,16 +483,16 @@ QUnit.test("multi level commands", async (assert) => { placeHolder: "Who is the next King ?", provide: () => otherNames.map((name) => ({ - name, action: () => {}, + name, })), }); commandProviderRegistry.add("default", { provide: () => defaultNames.map((name) => ({ - name, action, + name, })), }); @@ -536,6 +536,83 @@ QUnit.test("multi level commands", async (assert) => { ); }); +QUnit.test("multi level commands with hotkey", async (assert) => { + assert.expect(5); + + patchWithCleanup(browser, { + clearTimeout: () => {}, + setTimeout: (later, wait) => { + later(); + }, + }); + + clearRegistryWithCleanup(commandProviderRegistry); + + const otherNames = ["Cersei", "Lannister"]; + const action = () => ({ + placeHolder: "Who is the next King ?", + provide: () => + otherNames.map((name) => ({ + action: () => {}, + name, + })), + }); + + const hotkey = "a"; + const name = "John"; + commandProviderRegistry.add("default", { + provide: () => [ + { + Component: HotkeyCommandItem, + action, + name, + props: { + hotkey, + }, + }, + ], + }); + + testComponent = await mount(TestComponent, { env, target }); + + // Open palette + triggerHotkey("control+k"); + await nextTick(); + + assert.deepEqual( + target.querySelector(".o_command_palette_search input").value, + "", + "search input is empty" + ); + + assert.deepEqual( + target.querySelector(".o_command_palette_search input").placeholder, + "Search for an action...", + "the default placeholder is correctly displayed" + ); + + assert.deepEqual( + [...target.querySelectorAll(".o_command")].map((el) => el.textContent.toLowerCase()), + [(name + hotkey).toLowerCase()], + "all default commands are present" + ); + + triggerHotkey("a"); + await nextTick(); + + assert.deepEqual( + target.querySelector(".o_command_palette_search input").placeholder, + "Who is the next King ?", + "the new placeholder is correctly displayed" + ); + + assert.deepEqual( + [...target.querySelectorAll(".o_command")].map((el) => el.textContent), + otherNames, + "all default commands are present" + ); +}); + QUnit.test("navigate in the command palette with the arrows", async (assert) => { assert.expect(6);