Skip to content

Proposal/clear link tool state#3000

Open
akulistus wants to merge 6 commits intonextfrom
proposal/clear-link-tool-state
Open

Proposal/clear link tool state#3000
akulistus wants to merge 6 commits intonextfrom
proposal/clear-link-tool-state

Conversation

@akulistus
Copy link
Copy Markdown
Contributor

Fixes issue (#2979 (comment)) preventing text unlinking by clicking link popover item block in toolbar. Also improves the link tool’s behavior when switching between tools in the toolbar

Comment on lines +417 to +418
* Checks if the current selection range starts outside
* of a popover item element
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, link tool should not know about Popover and its implementation. Explain, why it is needed.

}

/**
* Calls instance clear function
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Describe the use case please.

onActivate: (item: PopoverItemParams, event?: PointerEvent) => void;

/**
* Popover item clear handler
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

@akulistus
Copy link
Copy Markdown
Contributor Author

We need a way to restore the initial selection when a different popover item is clicked. Previously, this was handled by calling handleClick on nestedPopoverTriggerItem, but that approach caused linked text to become unlinked:

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
*/
super.destroyNestedPopoverIfExists();
}
super.handleItemClick(item);
}

I see three possible approaches to implement this:

  1. Current approach: detect whether a new selection occurs within the toolbar (on another element), and restore either the initial selection or the new one accordingly.
  2. Introduce a new method (e.g., handleItemChange) that triggers a dedicated function inside the inline-link tool to restore the initial selection.
  3. Pass a flag to distinguish between user-initiated clicks and programmatic ones, and handle selection restoration based on that context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants