From 62ebc97bd887a6358e02cd131590083fdd80d12f Mon Sep 17 00:00:00 2001 From: FrancoisGe Date: Wed, 16 Feb 2022 09:14:19 +0000 Subject: [PATCH] [IMP] web: Command Palette Config Debounce MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit is intended to make command palette faster. To improve this, we decided to make the debounce delay configurable by namespace. We also decided to limit the number of Command displays to 100. closes odoo/odoo#84227 Signed-off-by: Géry Debongnie --- addons/mail/static/src/js/m2x_avatar_user.js | 16 ++- .../src/webclient/commands/mail_providers.js | 2 + .../src/core/commands/command_palette.js | 104 ++++++++++------- .../src/core/commands/command_service.js | 44 ++++--- .../web/static/src/core/debug/debug_menu.js | 13 ++- .../src/webclient/menus/menu_providers.js | 2 +- .../core/commands/command_palette_tests.js | 107 ++++++++++++++---- 7 files changed, 202 insertions(+), 86 deletions(-) diff --git a/addons/mail/static/src/js/m2x_avatar_user.js b/addons/mail/static/src/js/m2x_avatar_user.js index ffa2f7f8751..b76f8806ec9 100644 --- a/addons/mail/static/src/js/m2x_avatar_user.js +++ b/addons/mail/static/src/js/m2x_avatar_user.js @@ -97,10 +97,14 @@ export const Many2OneAvatarUser = Many2OneAvatar.extend(M2XAvatarMixin, { }, async action() { return env.services.command.openPalette({ - emptyMessageByNamespace: { "default": env._t("No users found") }, + configByNamespace: { + default: { + emptyMessage: env._t("No users found"), + }, + }, placeholder: env._t("Select a user..."), providers: [{ provide }], - }) + }); }, }); core.bus.trigger("set_legacy_command", "web.Many2OneAvatar.assignTo", getCommandDefinition, self.el); @@ -274,10 +278,14 @@ export const Many2ManyAvatarUser = FieldMany2ManyTagsAvatar.extend(M2MAvatarMixi }, action() { return env.services.command.openPalette({ - emptyMessageByNamespace: { "default": env._t("No users found") }, + configByNamespace: { + default: { + emptyMessage: env._t("No users found"), + }, + }, placeholder: env._t("Select a user..."), providers: [{ provide }], - }) + }); }, }) core.bus.trigger("set_legacy_command", "web.FieldMany2ManyTagsAvatar.assignTo", getCommandDefinition) diff --git a/addons/mail/static/src/webclient/commands/mail_providers.js b/addons/mail/static/src/webclient/commands/mail_providers.js index bc5e95e4829..da3a67b9b79 100644 --- a/addons/mail/static/src/webclient/commands/mail_providers.js +++ b/addons/mail/static/src/webclient/commands/mail_providers.js @@ -15,10 +15,12 @@ DialogCommand.template = xml` const commandSetupRegistry = registry.category("command_setup"); commandSetupRegistry.add("@", { + debounceDelay: 200, emptyMessage: _lt("No user found"), name: _lt("users"), }); commandSetupRegistry.add("#", { + debounceDelay: 200, emptyMessage: _lt("No channel found"), name: _lt("channels"), }); diff --git a/addons/web/static/src/core/commands/command_palette.js b/addons/web/static/src/core/commands/command_palette.js index e5c36154144..db51d63224b 100644 --- a/addons/web/static/src/core/commands/command_palette.js +++ b/addons/web/static/src/core/commands/command_palette.js @@ -35,8 +35,15 @@ const FUZZY_NAMESPACES = ["default"]; /** * @typedef {{ - * categoriesByNamespace?: {[namespace]: string[]}; - * emptyMessageByNamespace?: {[namespace]: string}; + * categories: string[]; + * debounceDelay: number; + * emptyMessage: string; + * }} NamespaceConfig + */ + +/** + * @typedef {{ + * configByNamespace?: {[namespace]: NamespaceConfig}; * FooterComponent?: Component; * placeholder?: string; * providers: Provider[]; @@ -74,12 +81,7 @@ export class CommandPalette extends Component { this.keepLast = new KeepLast(); this.DefaultCommandItem = DefaultCommandItem; this.activeElement = useService("ui").activeElement; - const applyDebounceSearchValue = debounce.apply(this, [this.applySearchValue, 200]); - this.applyDebounceSearchValue = (...args) => { - this.searchValuePromise = applyDebounceSearchValue.apply(this, args).catch(() => { - this.searchValuePromise = null; - }); - }; + this.defaultDebounceSearch = debounce.apply(this, [this.search, 0]); useAutofocus(); useHotkey("Enter", () => this.executeSelectedCommand()); @@ -94,9 +96,7 @@ export class CommandPalette extends Component { * searchValue: string, * selectedCommand: CommandItem }} */ - this.state = useState({ - commands: [], - }); + this.state = useState({}); this.listboxRef = useRef("listbox"); @@ -124,25 +124,22 @@ export class CommandPalette extends Component { * @param {CommandPaletteConfig} config */ async setCommandPaletteConfig(config) { - const result = { default: [] }; - for (const provider of config.providers) { - const namespace = provider.namespace || "default"; - if (namespace in result) { - result[namespace].push(provider); - } else { - result[namespace] = [provider]; - } - } - this.categoriesByNamespace = config.categoriesByNamespace; - this.emptyMessageByNamespace = config.emptyMessageByNamespace || {}; - this.providersByNamespace = result; - + this.configByNamespace = config.configByNamespace || {}; + this.debounceSearchByNamespace = {}; this.state.FooterComponent = config.FooterComponent; - this.state.placeholder = config.placeholder || DEFAULT_PLACEHOLDER.toString(); - const searchValue = config.searchValue || ""; - await this.applySearchValue(searchValue); + this.providersByNamespace = { default: [] }; + for (const provider of config.providers) { + const namespace = provider.namespace || "default"; + if (namespace in this.providersByNamespace) { + this.providersByNamespace[namespace].push(provider); + } else { + this.providersByNamespace[namespace] = [provider]; + } + } + this.namespaces = Object.keys(this.providersByNamespace); + await this.search(config.searchValue || ""); } /** @@ -159,13 +156,14 @@ export class CommandPalette extends Component { return result; }); let commands = (await this.keepLast.add(Promise.all(proms))).flat(); + const namespaceConfig = this.configByNamespace[namespace] || {}; if (options.searchValue && FUZZY_NAMESPACES.includes(namespace)) { commands = fuzzyLookup(options.searchValue, commands, (c) => c.name); } else { // we have to sort the commands by category to avoid navigation issues with the arrows - if (this.categoriesByNamespace && this.categoriesByNamespace[namespace]) { + if (namespaceConfig.categories) { let commandsSorted = []; - this.categoryKeys = this.categoriesByNamespace[namespace]; + this.categoryKeys = namespaceConfig.categories; if (!this.categoryKeys.includes("default")) { this.categoryKeys.push("default"); } @@ -179,7 +177,7 @@ export class CommandPalette extends Component { } this.state.commands = markRaw( - commands.map((command) => ({ + commands.slice(0, 100).map((command) => ({ ...command, keyId: this.keyId++, splitName: splitCommandName(command.name, options.searchValue), @@ -187,8 +185,9 @@ export class CommandPalette extends Component { ); this.selectCommand(this.state.commands.length ? 0 : -1); this.mouseSelectionActive = false; - this.state.emptyMessage = - this.emptyMessageByNamespace[namespace] || DEFAULT_EMPTY_MESSAGE.toString(); + this.state.emptyMessage = ( + namespaceConfig.emptyMessage || DEFAULT_EMPTY_MESSAGE + ).toString(); this.clearSearchValue = options.searchValue; } @@ -255,21 +254,46 @@ export class CommandPalette extends Component { } } - async applySearchValue(searchValue) { - this.state.searchValue = searchValue; - let namespace = "default"; - if (searchValue.length && searchValue[0] in this.providersByNamespace) { - namespace = searchValue[0]; - searchValue = searchValue.slice(1); - } + async search(value) { + this.state.searchValue = value; + const { namespace, searchValue } = this.processSearchValue(value); await this.setCommands(namespace, { searchValue, activeElement: this.activeElement, }); } + debounceSearch(value) { + const { namespace } = this.processSearchValue(value); + if (!this.debounceSearchByNamespace[namespace]) { + const namespaceConfig = this.configByNamespace[namespace] || {}; + if (namespaceConfig.debounceDelay) { + this.debounceSearchByNamespace[namespace] = debounce.apply(this, [ + this.search, + namespaceConfig.debounceDelay, + ]); + } else { + this.debounceSearchByNamespace[namespace] = this.defaultDebounceSearch; + } + } + const debounceSearch = + this.debounceSearchByNamespace[namespace] || this.defaultDebounceSearch; + this.searchValuePromise = debounceSearch.apply(this, [value]).catch(() => { + this.searchValuePromise = null; + }); + } + onSearchInput(ev) { - this.applyDebounceSearchValue(ev.target.value); + this.debounceSearch(ev.target.value); + } + + processSearchValue(searchValue) { + let namespace = "default"; + if (searchValue.length && this.namespaces.includes(searchValue[0])) { + namespace = searchValue[0]; + searchValue = searchValue.slice(1); + } + return { namespace, searchValue }; } } CommandPalette.template = "web.CommandPalette"; diff --git a/addons/web/static/src/core/commands/command_service.js b/addons/web/static/src/core/commands/command_service.js index f3068714936..dbac7239ab9 100644 --- a/addons/web/static/src/core/commands/command_service.js +++ b/addons/web/static/src/core/commands/command_service.js @@ -72,26 +72,40 @@ export const commandService = { * @returns the actual command palette config if the command palette is already open */ function openMainPalette(config = {}) { - const categoriesByNamespace = {}; - commandCategoryRegistry.getEntries().forEach(([category, el]) => { - const namespace = el.namespace ? el.namespace : "default"; - if (namespace in categoriesByNamespace) { - categoriesByNamespace[namespace].push(category); - } else { - categoriesByNamespace[namespace] = [category]; + const configByNamespace = {}; + for (const provider of commandProviderRegistry.getAll()) { + const namespace = provider.namespace || "default"; + if (!configByNamespace[namespace]) { + configByNamespace[namespace] = { + categories: [], + }; } - }); + } - const emptyMessageByNamespace = {}; - commandSetupRegistry.getEntries().forEach(([key, { emptyMessage }]) => { - if (emptyMessage) { - emptyMessageByNamespace[key] = emptyMessage.toString(); + for (const [category, el] of commandCategoryRegistry.getEntries()) { + const namespace = el.namespace || "default"; + if (namespace in configByNamespace) { + configByNamespace[namespace].categories.push(category); } - }); + } + + for (const [ + namespace, + { emptyMessage, debounceDelay }, + ] of commandSetupRegistry.getEntries()) { + if (namespace in configByNamespace) { + if (emptyMessage) { + configByNamespace[namespace].emptyMessage = emptyMessage; + } + if (debounceDelay !== undefined) { + configByNamespace[namespace].debounceDelay = debounceDelay; + } + } + } + config = Object.assign( { - categoriesByNamespace, - emptyMessageByNamespace, + configByNamespace, FooterComponent: DefaultFooter, placeholder: env._t("Search for a command..."), providers: commandProviderRegistry.getAll(), diff --git a/addons/web/static/src/core/debug/debug_menu.js b/addons/web/static/src/core/debug/debug_menu.js index b9812afabaa..dd7e39d973d 100644 --- a/addons/web/static/src/core/debug/debug_menu.js +++ b/addons/web/static/src/core/debug/debug_menu.js @@ -39,13 +39,16 @@ export class DebugMenu extends DebugMenuBasic { return result; }, }; - const commandPaletteConfig = { - categoriesByNamespace: { - default: defaultCategories, + const configByNamespace = { + default: { + categories: defaultCategories, + emptyMessage: this.env._t("No command found"), + providers: [provider], }, - emptyMessageByNamespace: { default: this.env._t("No command found") }, + }; + const commandPaletteConfig = { placeholder: this.env._t("Choose a debug command..."), - providers: [provider], + configByNamespace, }; return this.command.openPalette(commandPaletteConfig); }, diff --git a/addons/web/static/src/webclient/menus/menu_providers.js b/addons/web/static/src/webclient/menus/menu_providers.js index 474ee926cd5..2817c6cc1fa 100644 --- a/addons/web/static/src/webclient/menus/menu_providers.js +++ b/addons/web/static/src/webclient/menus/menu_providers.js @@ -46,7 +46,7 @@ commandProviderRegistry.add("menu", { apps.forEach((menu) => { const props = {}; if (menu.webIconData) { - const prefix = menu.webIconData.startsWith('P') + const prefix = menu.webIconData.startsWith("P") ? "data:image/svg+xml;base64," : "data:image/png;base64,"; props.webIconData = menu.webIconData.startsWith("data:image") diff --git a/addons/web/static/tests/core/commands/command_palette_tests.js b/addons/web/static/tests/core/commands/command_palette_tests.js index 5f7cc109d3f..66fdac4e70e 100644 --- a/addons/web/static/tests/core/commands/command_palette_tests.js +++ b/addons/web/static/tests/core/commands/command_palette_tests.js @@ -50,7 +50,7 @@ QUnit.module("Command Palette Dialog", { patchWithCleanup(browser, { clearTimeout: () => {}, - setTimeout: (later, wait) => { + setTimeout: (later) => { later(); }, }); @@ -85,10 +85,16 @@ QUnit.test("empty providers", async (assert) => { QUnit.test("custom empty message", async (assert) => { testComponent = await mount(TestComponent, target, { env }); - const emptyMessageByNamespace = { - default: "Empty Default", - "@": "Empty @", - "#": "Empty #", + const configByNamespace = { + default: { + emptyMessage: "Empty Default", + }, + "@": { + emptyMessage: "Empty @", + }, + "#": { + emptyMessage: "Empty #", + }, }; const provide = () => []; const providers = [ @@ -96,7 +102,7 @@ QUnit.test("custom empty message", async (assert) => { { namespace: "#", provide }, ]; const config = { - emptyMessageByNamespace, + configByNamespace, providers, }; env.services.dialog.add(CommandPaletteDialog, { @@ -108,24 +114,74 @@ QUnit.test("custom empty message", async (assert) => { assert.containsOnce(target, ".o_command_palette_listbox_empty"); assert.strictEqual( target.querySelector(".o_command_palette_listbox_empty").textContent, - emptyMessageByNamespace["default"] + configByNamespace["default"].emptyMessage ); await editSearchBar("@"); assert.containsOnce(target, ".o_command_palette_listbox_empty"); assert.strictEqual( target.querySelector(".o_command_palette_listbox_empty").textContent, - emptyMessageByNamespace["@"] + configByNamespace["@"].emptyMessage ); await editSearchBar("#"); assert.containsOnce(target, ".o_command_palette_listbox_empty"); assert.strictEqual( target.querySelector(".o_command_palette_listbox_empty").textContent, - emptyMessageByNamespace["#"] + configByNamespace["#"].emptyMessage ); }); +QUnit.test("custom debounce delay", async (assert) => { + patchWithCleanup(browser, { + clearTimeout: () => {}, + setTimeout: (later, delay) => { + assert.step(delay.toString()); + later(); + }, + }); + + testComponent = await mount(TestComponent, target, { env }); + const configByNamespace = { + "@": { + debounceDelay: 200, + }, + "#": { + debounceDelay: 100, + }, + }; + const action = () => {}; + const provide = () => [ + { + name: "Command1", + action, + }, + { + name: "Command2", + action, + }, + ]; + const providers = [ + { namespace: "@", provide }, + { namespace: "#", provide }, + ]; + const config = { + configByNamespace, + providers, + }; + env.services.dialog.add(CommandPaletteDialog, { + config, + }); + await nextTick(); + assert.containsOnce(target, ".o_command_palette"); + assert.containsNone(target, ".o_command"); + await editSearchBar("com"); + await editSearchBar("@"); + await editSearchBar("#"); + await editSearchBar(""); + assert.verifySteps(["0", "200", "100", "0"]); +}); + QUnit.test("custom placeholder", async (assert) => { testComponent = await mount(TestComponent, target, { env }); const config = { @@ -511,10 +567,6 @@ QUnit.test("open the command palette with a namespace already in the searchbar", QUnit.test("multi provider with categories", async (assert) => { testComponent = await mount(TestComponent, target, { env }); - const categoriesByNamespace = { - default: ["cat1", "cat2"], - "@": ["@cat1", "@cat2"], - }; const action = () => {}; const providers = [ { @@ -560,8 +612,16 @@ QUnit.test("multi provider with categories", async (assert) => { ], }, ]; + const configByNamespace = { + default: { + categories: ["cat1", "cat2"], + }, + "@": { + categories: ["@cat1", "@cat2"], + }, + }; const config = { - categoriesByNamespace, + configByNamespace, providers, }; env.services.dialog.add(CommandPaletteDialog, { @@ -636,9 +696,6 @@ QUnit.test("multi provider with categories", async (assert) => { QUnit.test("don't display by categories if there is a search value", async (assert) => { testComponent = await mount(TestComponent, target, { env }); - const categoriesByNamespace = { - default: ["cat1", "cat2"], - }; const action = () => {}; const providers = [ { @@ -660,8 +717,13 @@ QUnit.test("don't display by categories if there is a search value", async (asse ], }, ]; + const configByNamespace = { + default: { + categories: ["cat1", "cat2"], + }, + }; const config = { - categoriesByNamespace, + configByNamespace, providers, }; env.services.dialog.add(CommandPaletteDialog, { @@ -710,6 +772,7 @@ QUnit.test("click on command", async (assert) => { }, }, ]; + const providers = [ { provide: () => commands, @@ -938,8 +1001,10 @@ QUnit.test("keyboard navigation scroll", async (assert) => { QUnit.test("multi level command", async (assert) => { testComponent = await mount(TestComponent, target, { env }); - const emptyMessageByNamespace = { - default: "Empty Default", + const configByNamespace = { + default: { + emptyMessage: "Empty Default", + }, }; const commands = [ { @@ -957,7 +1022,7 @@ QUnit.test("multi level command", async (assert) => { }, ]; const config = { - emptyMessageByNamespace, + configByNamespace, FooterComponent, placeholder: "placeholder test", providers,