From a6473877f62d54e6e084bbb403546a5ff1a82282 Mon Sep 17 00:00:00 2001 From: Tatiana Fomina Date: Wed, 3 Jul 2024 19:22:47 +0300 Subject: [PATCH] Disable flipper in inline toolbar actions (#2756) --- src/components/modules/toolbar/inline.ts | 10 +++-- .../components/popover-item/popover-item.ts | 19 ++++++++++ .../popover-item/popover-item.types.ts | 6 +++ .../utils/popover/popover-desktop.ts | 37 ++++++++++--------- src/components/utils/popover/popover.types.ts | 6 +++ 5 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/components/modules/toolbar/inline.ts b/src/components/modules/toolbar/inline.ts index 3192c7f0..f3150329 100644 --- a/src/components/modules/toolbar/inline.ts +++ b/src/components/modules/toolbar/inline.ts @@ -53,7 +53,7 @@ export default class InlineToolbar extends Module { /** * Currently visible tools instances */ - private toolsInstances: Map = new Map(); + private toolsInstances: Map | null = new Map(); /** * @param moduleConfiguration - Module Configuration @@ -384,7 +384,9 @@ export default class InlineToolbar extends Module { const actions = instance.renderActions(); (popoverItem as WithChildren).children = { - isOpen: instance.checkState(SelectionUtils.get()), + isOpen: instance.checkState?.(SelectionUtils.get()), + /** Disable keyboard navigation in actions, as it might conflict with enter press handling */ + isFlippable: false, items: [ { type: PopoverItemType.Html, @@ -396,7 +398,7 @@ export default class InlineToolbar extends Module { /** * Legacy inline tools might perform some UI mutating logic in checkState method, so, call it just in case */ - instance.checkState(SelectionUtils.get()); + instance.checkState?.(SelectionUtils.get()); } popoverItems.push(popoverItem); @@ -534,7 +536,7 @@ export default class InlineToolbar extends Module { * Check Tools` state by selection */ private checkToolsState(): void { - this.toolsInstances.forEach((toolInstance) => { + this.toolsInstances?.forEach((toolInstance) => { toolInstance.checkState?.(SelectionUtils.get()); }); } diff --git a/src/components/utils/popover/components/popover-item/popover-item.ts b/src/components/utils/popover/components/popover-item/popover-item.ts index 5e0c10db..44c370cc 100644 --- a/src/components/utils/popover/components/popover-item/popover-item.ts +++ b/src/components/utils/popover/components/popover-item/popover-item.ts @@ -107,6 +107,25 @@ export abstract class PopoverItem { return this.params !== undefined && 'children' in this.params && this.params.children?.isOpen === true; } + /** + * True if item children items should be navigatable via keyboard + */ + public get isChildrenFlippable(): boolean { + if (this.params === undefined) { + return false; + } + + if (!('children' in this.params)) { + return false; + } + + if (this.params.children?.isFlippable === false) { + return false; + } + + return true; + } + /** * Returns true if item has children that should be searchable */ diff --git a/src/components/utils/popover/components/popover-item/popover-item.types.ts b/src/components/utils/popover/components/popover-item/popover-item.types.ts index b9096605..9f7a6401 100644 --- a/src/components/utils/popover/components/popover-item/popover-item.types.ts +++ b/src/components/utils/popover/components/popover-item/popover-item.types.ts @@ -30,6 +30,12 @@ export interface PopoverItemChildren { */ isOpen?: boolean; + /** + * False if keyboard navigation should be disabled in the children popover. + * True by default + */ + isFlippable?: boolean; + /** * Items of nested popover that should be open on the current item hover/click (depending on platform) */ diff --git a/src/components/utils/popover/popover-desktop.ts b/src/components/utils/popover/popover-desktop.ts index 74480668..1cce489b 100644 --- a/src/components/utils/popover/popover-desktop.ts +++ b/src/components/utils/popover/popover-desktop.ts @@ -19,7 +19,7 @@ export class PopoverDesktop extends PopoverAbstract { /** * Flipper - module for keyboard iteration between elements */ - public flipper: Flipper; + public flipper: Flipper | undefined; /** * Popover nesting level. 0 value means that it is a root popover @@ -75,18 +75,20 @@ export class PopoverDesktop extends PopoverAbstract { this.addSearch(); } - this.flipper = new Flipper({ - items: this.flippableElements, - focusedItemClass: popoverItemCls.focused, - allowedKeys: [ - keyCodes.TAB, - keyCodes.UP, - keyCodes.DOWN, - keyCodes.ENTER, - ], - }); + if (params.flippable !== false) { + this.flipper = new Flipper({ + items: this.flippableElements, + focusedItemClass: popoverItemCls.focused, + allowedKeys: [ + keyCodes.TAB, + keyCodes.UP, + keyCodes.DOWN, + keyCodes.ENTER, + ], + }); - this.flipper.onFlip(this.onFlip); + this.flipper.onFlip(this.onFlip); + } } /** @@ -137,7 +139,7 @@ export class PopoverDesktop extends PopoverAbstract { } super.show(); - this.flipper.activate(this.flippableElements); + this.flipper?.activate(this.flippableElements); } /** @@ -148,7 +150,7 @@ export class PopoverDesktop extends PopoverAbstract { this.destroyNestedPopoverIfExists(); - this.flipper.deactivate(); + this.flipper?.deactivate(); this.previouslyHoveredItem = null; }; @@ -228,7 +230,7 @@ export class PopoverDesktop extends PopoverAbstract { this.nestedPopover.destroy(); this.nestedPopover.getElement().remove(); this.nestedPopover = null; - this.flipper.activate(this.flippableElements); + this.flipper?.activate(this.flippableElements); this.items.forEach(item => item.onChildrenClose()); } @@ -244,6 +246,7 @@ export class PopoverDesktop extends PopoverAbstract { searchable: item.isChildrenSearchable, items: item.children, nestingLevel: this.nestingLevel + 1, + flippable: item.isChildrenFlippable, }); item.onChildrenOpen(); @@ -264,7 +267,7 @@ export class PopoverDesktop extends PopoverAbstract { nestedPopoverEl.style.setProperty(CSSVariables.NestingLevel, this.nestedPopover.nestingLevel.toString()); this.nestedPopover.show(); - this.flipper.deactivate(); + this.flipper?.deactivate(); return this.nestedPopover; } @@ -414,7 +417,7 @@ export class PopoverDesktop extends PopoverAbstract { /** List of elements available for keyboard navigation considering search query applied */ const flippableElements = data.query === '' ? this.flippableElements : data.items.map(item => (item as PopoverItem).getElement()); - if (this.flipper.isActivated) { + if (this.flipper?.isActivated) { /** Update flipper items with only visible */ this.flipper.deactivate(); this.flipper.activate(flippableElements as HTMLElement[]); diff --git a/src/components/utils/popover/popover.types.ts b/src/components/utils/popover/popover.types.ts index 8e28df71..0af9a11f 100644 --- a/src/components/utils/popover/popover.types.ts +++ b/src/components/utils/popover/popover.types.ts @@ -20,6 +20,12 @@ export interface PopoverParams { */ searchable?: boolean; + /** + * False if keyboard navigation should be disabled. + * True by default + */ + flippable?: boolean; + /** * Popover texts overrides */