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(merge): incorrect caret position on merge #86

Merged
merged 4 commits into from
Mar 30, 2024
Merged

fix(merge): incorrect caret position on merge #86

merged 4 commits into from
Mar 30, 2024

Conversation

neSpecc
Copy link
Contributor

@neSpecc neSpecc commented Mar 30, 2024

Problem

Right now caret restored at incorrect position after merging:

header merge is ok, paragraph is not:

paragraph-to-paragraph-merge-caret-bug.mov

Can be reproduced in Safari.

Cause

In editor.js blockEvents we have this logic:

https://github.com/codex-team/editor.js/blob/b355f1673c11ec3c0603303aefaa6a546cf3a377/src/components/modules/blockEvents.ts#L488-L503

  private mergeBlocks(targetBlock: Block, blockToMerge: Block): void {
    const { BlockManager, Caret, Toolbar } = this.Editor;

    Caret.createShadow(targetBlock.pluginsContent);

    BlockManager
      .mergeBlocks(targetBlock, blockToMerge)
      .then(() => {
        window.requestAnimationFrame(() => {
          /** Restore caret position after merge */
          Caret.restoreCaret(targetBlock.pluginsContent as HTMLElement);
          targetBlock.pluginsContent.normalize();
          Toolbar.close();
        });
      });
  }

There is 2 problems:

  • redundant requestAnimationFrame. It used only because paragraph this.data setter updates DOM in requestAnimationFrame
  • In paragraph's merge() method we update DOM using innerHTML replacement. Link to the shadow-caret losing at that time

Solution

  • Get rid of this.data setter and getter. They has no sense and it's hard to maintain them: logic should be different in every case:
  • merge() now just appends new content and don't breaks links to existing nodes (eg. shadow caret)

Resolves #73

TatianaFomina
TatianaFomina previously approved these changes Mar 30, 2024
src/utils/makeFragment.js Outdated Show resolved Hide resolved
src/utils/makeFragment.js Outdated Show resolved Hide resolved
src/utils/makeFragment.js Outdated Show resolved Hide resolved
@neSpecc neSpecc merged commit 0591aba into master Mar 30, 2024
@neSpecc neSpecc deleted the fix/merge branch March 30, 2024 17:07
tech-magnety-ai pushed a commit to tech-magnety-ai/editor-js-paragraph that referenced this pull request Apr 28, 2024
fix(merge): incorrect caret position on merge
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.

Hydrate function messes caret when merging paragraphs
3 participants