Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: block copy using settings button #2851

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

laishere
Copy link

Block copy (CMD + C) via settings button is broken in recent editorjs releases.

After debugging, I found the issue is related to this logic:

if (this.search !== undefined) {
this.search.focus();
}

It tries to focus the search input in the popover, but it won't work.
We need to wrap it with requestAnimationFrame.

The old logic did the same:

if (this.search !== undefined) {
requestAnimationFrame(() => {
this.search?.focus();
});
}

Why does this matter?

I found the answer after debugging both the old version and the latest version.

  1. When a keydown event is detected on the document listener, it tries to decide if the event happens inside the editor element by:

    const keyDownOnEditor = (event.target as HTMLElement).closest(`.${this.CSS.editorWrapper}`);

  2. Since the popover does not manage to get the focus on the search element, so when we press CMD + C with the popover is showing, the event.target is actually document itself. So keyDownOnEditor is null.

  3. When keyDownOnEditor === null:

    if (currentBlock !== undefined && keyDownOnEditor === null) {
    this.Editor.BlockEvents.keydown(event);
    return;
    }

    It will call this.Editor.BlockEvents.keydown(event), which will close the popover and unselectBlock.
    if (!this.Editor.CrossBlockSelection.isCrossBlockSelectionStarted && this.Editor.BlockManager.currentBlock) {
    this.Editor.BlockSelection.unselectBlock(this.Editor.BlockManager.currentBlock);
    }

    So we won't have a selected block any more when we do the keydown event handling later.

This PR fixs this issue by making the focus on the search element actually work again when we click the settings button.

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.

1 participant