From b35f22e9a4e448d26525fdd36be213e7523a00fd Mon Sep 17 00:00:00 2001 From: Alex Gaillard Date: Thu, 12 Mar 2026 16:25:51 -0400 Subject: [PATCH] fix(inline-tools): fix issue with not being able to unlink --- .../inline-tools/inline-tool-link.ts | 113 ++++++++++-------- src/components/modules/toolbar/inline.ts | 24 ++-- .../utils/popover/popover-inline.ts | 27 ++--- 3 files changed, 87 insertions(+), 77 deletions(-) diff --git a/src/components/inline-tools/inline-tool-link.ts b/src/components/inline-tools/inline-tool-link.ts index 0bef25c73..884ae356b 100644 --- a/src/components/inline-tools/inline-tool-link.ts +++ b/src/components/inline-tools/inline-tool-link.ts @@ -67,8 +67,8 @@ export default class LinkInlineTool implements InlineTool { * Elements */ private nodes: { - button: HTMLButtonElement; - input: HTMLInputElement; + button: HTMLButtonElement | null; + input: HTMLInputElement | null; } = { button: null, input: null, @@ -147,49 +147,35 @@ export default class LinkInlineTool implements InlineTool { /** * Handle clicks on the Inline Toolbar icon - * - * @param {Range} range - range to wrap with link */ - public surround(range: Range): void { + public surround(): void { /** * Range will be null when user makes second click on the 'link icon' to close opened input */ - if (range) { - /** - * Save selection before change focus to the input - */ - if (!this.inputOpened) { - /** Create blue background instead of selection */ - this.selection.setFakeBackground(); - this.selection.save(); - } else { - this.selection.restore(); - this.selection.removeFakeBackground(); - } - const parentAnchor = this.selection.findParentTag('A'); + /** + * Save selection before change focus to the input + */ + if (!this.inputOpened) { + /** Create blue background instead of selection */ + this.selection.setFakeBackground(); + this.selection.save(); + } else { + this.selection.restore(); + this.selection.removeFakeBackground(); + } + const parentAnchor = this.selection.findParentTag('A'); - /** - * Unlink icon pressed - */ - if (parentAnchor) { - /** - * If input is not opened, treat click as explicit unlink action. - * If input is opened (e.g., programmatic close when switching tools), avoid unlinking. - */ - if (!this.inputOpened) { - this.selection.expandToTag(parentAnchor); - this.unlink(); - this.closeActions(); - this.checkState(); - this.toolbar.close(); - } else { - /** Only close actions without clearing saved selection to preserve user state */ - this.closeActions(false); - this.checkState(); - } - - return; - } + /** + * Unlink icon pressed + */ + if (parentAnchor) { + this.selection.expandToTag(parentAnchor); + this.unlink(); + this.closeActions(); + this.checkState(); + this.toolbar.close(); + + return; } this.toggleActions(); @@ -202,9 +188,11 @@ export default class LinkInlineTool implements InlineTool { const anchorTag = this.selection.findParentTag('A'); if (anchorTag) { - this.nodes.button.innerHTML = IconUnlink; - this.nodes.button.classList.add(this.CSS.buttonUnlink); - this.nodes.button.classList.add(this.CSS.buttonActive); + if (this.nodes.button) { + this.nodes.button.innerHTML = IconUnlink; + this.nodes.button.classList.add(this.CSS.buttonUnlink); + this.nodes.button.classList.add(this.CSS.buttonActive); + } this.openActions(); /** @@ -212,13 +200,18 @@ export default class LinkInlineTool implements InlineTool { */ const hrefAttr = anchorTag.getAttribute('href'); - this.nodes.input.defaultValue = hrefAttr !== 'null' ? hrefAttr : ''; + if (this.nodes.input) { + this.nodes.input.defaultValue = + hrefAttr !== null && hrefAttr !== 'null' ? hrefAttr : ''; + } this.selection.save(); } else { - this.nodes.button.innerHTML = IconLink; - this.nodes.button.classList.remove(this.CSS.buttonUnlink); - this.nodes.button.classList.remove(this.CSS.buttonActive); + if (this.nodes.button) { + this.nodes.button.innerHTML = IconLink; + this.nodes.button.classList.remove(this.CSS.buttonUnlink); + this.nodes.button.classList.remove(this.CSS.buttonActive); + } } return !!anchorTag; @@ -228,6 +221,16 @@ export default class LinkInlineTool implements InlineTool { * Function called with Inline Toolbar closing */ public clear(): void { + /** + * Restore the original text selection if fake background was set + * (e.g. when user was typing a URL and switched to another tool). + * This must happen before closeActions() so the browser selection + * is on the text, not stuck in the input field. + */ + if (this.selection.isFakeBackgroundEnabled) { + this.selection.restore(); + this.selection.removeFakeBackground(); + } this.closeActions(); } @@ -253,9 +256,11 @@ export default class LinkInlineTool implements InlineTool { * @param {boolean} needFocus - on link creation we need to focus input. On editing - nope. */ private openActions(needFocus = false): void { - this.nodes.input.classList.add(this.CSS.inputShowed); - if (needFocus) { - this.nodes.input.focus(); + if (this.nodes.input) { + this.nodes.input.classList.add(this.CSS.inputShowed); + if (needFocus) { + this.nodes.input.focus(); + } } this.inputOpened = true; } @@ -280,8 +285,10 @@ export default class LinkInlineTool implements InlineTool { currentSelection.restore(); } - this.nodes.input.classList.remove(this.CSS.inputShowed); - this.nodes.input.value = ''; + if (this.nodes.input) { + this.nodes.input.classList.remove(this.CSS.inputShowed); + this.nodes.input.value = ''; + } if (clearSavedSelection) { this.selection.clearSaved(); } @@ -294,7 +301,7 @@ export default class LinkInlineTool implements InlineTool { * @param {KeyboardEvent} event - enter keydown event */ private enterPressed(event: KeyboardEvent): void { - let value = this.nodes.input.value || ''; + let value = (this.nodes.input && this.nodes.input.value) || ''; if (!value.trim()) { this.selection.restore(); diff --git a/src/components/modules/toolbar/inline.ts b/src/components/modules/toolbar/inline.ts index 5aa5f7dab..841e6a41d 100644 --- a/src/components/modules/toolbar/inline.ts +++ b/src/components/modules/toolbar/inline.ts @@ -68,9 +68,12 @@ export default class InlineToolbar extends Module { eventsDispatcher, }); - window.requestIdleCallback(() => { - this.make(); - }, { timeout: 2000 }); + window.requestIdleCallback( + () => { + this.make(); + }, + { timeout: 2000 } + ); } /** @@ -233,7 +236,7 @@ export default class InlineToolbar extends Module { * Prevent InlineToolbar from overflowing the content zone on the right side */ if (realRightCoord > this.Editor.UI.contentRect.right) { - newCoords.x = this.Editor.UI.contentRect.right -popoverWidth - wrapperOffset.x; + newCoords.x = this.Editor.UI.contentRect.right - popoverWidth - wrapperOffset.x; } this.nodes.wrapper!.style.left = Math.floor(newCoords.x) + 'px'; @@ -415,7 +418,7 @@ 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: [ @@ -424,12 +427,17 @@ export default class InlineToolbar extends Module { element: actions, }, ], + onClose: () => { + if (_.isFunction(instance.clear)) { + instance.clear(); + } + }, }; } else { /** * 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); @@ -541,7 +549,7 @@ export default class InlineToolbar extends Module { */ // if (SelectionUtils.isCollapsed) return; - if (!currentBlock.tool.enabledInlineTools) { + if (currentBlock.tool.enabledInlineTools === false) { return; } @@ -573,7 +581,7 @@ export default class InlineToolbar extends Module { */ private checkToolsState(): void { this.tools?.forEach((toolInstance) => { - toolInstance.checkState?.(SelectionUtils.get()); + toolInstance.checkState?.(SelectionUtils.get()!); }); } diff --git a/src/components/utils/popover/popover-inline.ts b/src/components/utils/popover/popover-inline.ts index ebe91223c..475d4806a 100644 --- a/src/components/utils/popover/popover-inline.ts +++ b/src/components/utils/popover/popover-inline.ts @@ -53,15 +53,15 @@ export class PopoverInline extends PopoverDesktop { * once you select tag content in text */ this.items - .forEach((item) => { - if (!(item instanceof PopoverItemDefault) && !(item instanceof PopoverItemHtml)) { - return; - } - - if (item.hasChildren && item.isChildrenOpen) { - this.showNestedItems(item); - } - }); + .forEach((item) => { + if (!(item instanceof PopoverItemDefault) && !(item instanceof PopoverItemHtml)) { + return; + } + + if (item.hasChildren && item.isChildrenOpen) { + this.showNestedItems(item); + } + }); } /** @@ -166,13 +166,8 @@ export class PopoverInline extends PopoverDesktop { protected override handleItemClick(item: PopoverItem): void { if (item !== this.nestedPopoverTriggerItem) { /** - * In case tool had special handling for toggling button (like link tool which modifies selection) - * we need to call handleClick on nested popover trigger item - */ - this.nestedPopoverTriggerItem?.handleClick(); - - /** - * Then close the nested popover + * Close the nested popover without triggering the tool's action. + * The onChildrenClose callback will handle any necessary UI cleanup. */ super.destroyNestedPopoverIfExists(); }