fix(conversion): restore caret after conversion though the Inline Toolbar and API (#2699)

* fix caret loosing after caret

* Refactor convert method to return Promise in Blocks API

* changelog upd

* Fix missing semicolon in blocks.cy.ts and BlockTunes.cy.ts

* add test for inline toolbar conversion

* Fix missing semicolon in InlineToolbar.cy.ts

* add test for toolbox shortcut

* api caret.setToBlock now can accept block api or index or id

* eslint fix

* Refactor test descriptions in caret.cy.ts

* rm tsconfig change

* lint

* lint

* Update CHANGELOG.md

Author: neSpecc

docs/CHANGELOG.md

@@ -2,17 +2,19 @@
 
 ### 2.30.0
 
-– `New` – Block Tunes now supports nesting items
-– `New` – Block Tunes now supports separator items
-– `New` – "Convert to" control is now also available in Block Tunes
+- `New` – Block Tunes now supports nesting items
+- `New` – Block Tunes now supports separator items
+- `New` – "Convert to" control is now also available in Block Tunes
 - `Improvement` — The ability to merge blocks of different types (if both tools provide the conversionConfig)
 - `Fix` — `onChange` will be called when removing the entire text within a descendant element of a block.
 - `Fix` - Unexpected new line on Enter press with selected block without caret
 - `Fix` - Search input autofocus loosing after Block Tunes opening
 - `Fix` - Block removing while Enter press on Block Tunes
-– `Fix` – Unwanted scroll on first typing on iOS devices
+- `Fix` – Unwanted scroll on first typing on iOS devices
 - `Fix` - Unwanted soft line break on Enter press after period and space (". |") on iOS devices
 - `Fix` - Caret lost after block conversion on mobile devices.
+- `Improvement` - The API `blocks.convert()` now returns the new block API
+- `Improvement` - The API `caret.setToBlock()` now can accept either BlockAPI or block index or block id
 
 ### 2.29.1
 

src/components/modules/api/blocks.ts

@@ -1,4 +1,4 @@
-import { BlockAPI as BlockAPIInterface, Blocks } from '../../../../types/api';
+import type { BlockAPI as BlockAPIInterface, Blocks } from '../../../../types/api';
 import { BlockToolData, OutputBlockData, OutputData, ToolConfig } from '../../../../types';
 import * as _ from './../../utils';
 import BlockAPI from '../../block/api';
@@ -327,7 +327,7 @@ export default class BlocksAPI extends Module {
    * @param dataOverrides - optional data overrides for the new block
    * @throws Error if conversion is not possible
    */
-  private convert = (id: string, newType: string, dataOverrides?: BlockToolData): void => {
+  private convert = async (id: string, newType: string, dataOverrides?: BlockToolData): Promise<BlockAPIInterface> => {
     const { BlockManager, Tools } = this.Editor;
     const blockToConvert = BlockManager.getBlockById(id);
 
@@ -346,7 +346,9 @@ export default class BlocksAPI extends Module {
     const targetBlockConvertable = targetBlockTool.conversionConfig?.import !== undefined;
 
     if (originalBlockConvertable && targetBlockConvertable) {
-      BlockManager.convert(blockToConvert, newType, dataOverrides);
+      const newBlock = await BlockManager.convert(blockToConvert, newType, dataOverrides);
+
+      return new BlockAPI(newBlock);
     } else {
       const unsupportedBlockTypes = [
         !originalBlockConvertable ? capitalize(blockToConvert.name) : false,

src/components/modules/api/caret.ts

@@ -1,5 +1,6 @@
-import { Caret } from '../../../../types/api';
+import { BlockAPI, Caret } from '../../../../types/api';
 import Module from '../../__module';
+import { resolveBlock } from '../../utils/api';
 
 /**
  * @class CaretAPI
@@ -96,21 +97,23 @@ export default class CaretAPI extends Module {
   /**
    * Sets caret to the Block by passed index
    *
-   * @param {number} index - index of Block where to set caret
-   * @param {string} position - position where to set caret
-   * @param {number} offset - caret offset
+   * @param blockOrIdOrIndex - either BlockAPI or Block id or Block index
+   * @param position - position where to set caret
+   * @param offset - caret offset
    * @returns {boolean}
    */
   private setToBlock = (
-    index: number,
+    blockOrIdOrIndex: BlockAPI | BlockAPI['id'] | number,
     position: string = this.Editor.Caret.positions.DEFAULT,
     offset = 0
   ): boolean => {
-    if (!this.Editor.BlockManager.blocks[index]) {
+    const block = resolveBlock(blockOrIdOrIndex, this.Editor);
+
+    if (block === undefined) {
       return false;
     }
 
-    this.Editor.Caret.setToBlock(this.Editor.BlockManager.blocks[index], position, offset);
+    this.Editor.Caret.setToBlock(block, position, offset);
 
     return true;
   };

src/components/modules/toolbar/conversion.ts

@@ -183,16 +183,14 @@ export default class ConversionToolbar extends Module<ConversionToolbarNodes> {
   public async replaceWithBlock(replacingToolName: string, blockDataOverrides?: BlockToolData): Promise<void> {
     const { BlockManager, BlockSelection, InlineToolbar, Caret } = this.Editor;
 
-    BlockManager.convert(this.Editor.BlockManager.currentBlock, replacingToolName, blockDataOverrides);
+    const newBlock = await BlockManager.convert(this.Editor.BlockManager.currentBlock, replacingToolName, blockDataOverrides);
 
     BlockSelection.clearSelection();
 
     this.close();
     InlineToolbar.close();
 
-    window.requestAnimationFrame(() => {
-      Caret.setToBlock(this.Editor.BlockManager.currentBlock, Caret.positions.END);
-    });
+    Caret.setToBlock(newBlock, Caret.positions.END);
   }
 
   /**

src/components/modules/toolbar/inline.ts

@@ -427,6 +427,10 @@ export default class InlineToolbar extends Module<InlineToolbarNodes> {
 
     this.nodes.togglerAndButtonsWrapper.appendChild(this.nodes.conversionToggler);
 
+    if (import.meta.env.MODE === 'test') {
+      this.nodes.conversionToggler.setAttribute('data-cy', 'conversion-toggler');
+    }
+
     this.listeners.on(this.nodes.conversionToggler, 'click', () => {
       this.Editor.ConversionToolbar.toggle((conversionToolbarOpened) => {
         /**

src/components/ui/toolbox.ts

@@ -356,7 +356,7 @@ export default class Toolbox extends EventsDispatcher<ToolboxEventMap> {
     Shortcuts.add({
       name: shortcut,
       on: this.api.ui.nodes.redactor,
-      handler: (event: KeyboardEvent) => {
+      handler: async (event: KeyboardEvent) => {
         event.preventDefault();
 
         const currentBlockIndex = this.api.blocks.getCurrentBlockIndex();
@@ -368,11 +368,9 @@ export default class Toolbox extends EventsDispatcher<ToolboxEventMap> {
          */
         if (currentBlock) {
           try {
-            this.api.blocks.convert(currentBlock.id, toolName);
+            const newBlock = await this.api.blocks.convert(currentBlock.id, toolName);
 
-            window.requestAnimationFrame(() => {
-              this.api.caret.setToBlock(currentBlockIndex, 'end');
-            });
+            this.api.caret.setToBlock(newBlock, 'end');
 
             return;
           } catch (error) {}

src/components/utils/api.ts

@@ -0,0 +1,21 @@
+import type { BlockAPI } from '../../../types/api/block';
+import { EditorModules } from '../../types-internal/editor-modules';
+import Block from '../block';
+
+/**
+ * Returns Block instance by passed Block index or Block id
+ *
+ * @param attribute - either BlockAPI or Block id or Block index
+ * @param editor - Editor instance
+ */
+export function resolveBlock(attribute: BlockAPI | BlockAPI['id'] | number, editor: EditorModules): Block | undefined {
+  if (typeof attribute === 'number') {
+    return editor.BlockManager.getBlockByIndex(attribute);
+  }
+
+  if (typeof attribute === 'string') {
+    return editor.BlockManager.getBlockById(attribute);
+  }
+
+  return editor.BlockManager.getBlockById(attribute.id);
+}

test/cypress/tests/api/blocks.cy.ts

@@ -1,5 +1,5 @@
 import type EditorJS from '../../../../types/index';
-import { ConversionConfig, ToolboxConfig } from '../../../../types';
+import type { ConversionConfig, ToolboxConfig } from '../../../../types';
 import ToolMock from '../../fixtures/tools/ToolMock';
 
 /**
@@ -202,7 +202,7 @@ describe('api.blocks', () => {
   });
 
   describe('.convert()', function () {
-    it('should convert a Block to another type if original Tool has "conversionConfig.export" and target Tool has "conversionConfig.import"', function () {
+    it('should convert a Block to another type if original Tool has "conversionConfig.export" and target Tool has "conversionConfig.import". Should return BlockAPI as well.', function () {
       /**
        * Mock of Tool with conversionConfig
        */
@@ -246,20 +246,28 @@ describe('api.blocks', () => {
             existingBlock,
           ],
         },
-      }).then((editor) => {
+      }).then(async (editor) => {
         const { convert } = editor.blocks;
 
-        convert(existingBlock.id, 'convertableTool');
+        const returnValue = await convert(existingBlock.id, 'convertableTool');
 
         // wait for block to be converted
-        cy.wait(100).then(() => {
+        cy.wait(100).then(async () => {
           /**
            * Check that block was converted
            */
-          editor.save().then(( { blocks }) => {
-            expect(blocks.length).to.eq(1);
-            expect(blocks[0].type).to.eq('convertableTool');
-            expect(blocks[0].data.text).to.eq(existingBlock.data.text);
+          const { blocks } = await editor.save();
+
+          expect(blocks.length).to.eq(1);
+          expect(blocks[0].type).to.eq('convertableTool');
+          expect(blocks[0].data.text).to.eq(existingBlock.data.text);
+
+          /**
+           * Check that returned value is BlockAPI
+           */
+          expect(returnValue).to.containSubset({
+            name: 'convertableTool',
+            id: blocks[0].id,
           });
         });
       });
@@ -274,9 +282,10 @@ describe('api.blocks', () => {
           const fakeId = 'WRNG_ID';
           const { convert } = editor.blocks;
 
-          const exec = (): void => convert(fakeId, 'convertableTool');
-
-          expect(exec).to.throw(`Block with id "${fakeId}" not found`);
+          return convert(fakeId, 'convertableTool')
+            .catch((error) => {
+              expect(error.message).to.be.eq(`Block with id "${fakeId}" not found`);
+            });
         });
     });
 
@@ -302,9 +311,10 @@ describe('api.blocks', () => {
         const nonexistingToolName = 'WRNG_TOOL_NAME';
         const { convert } = editor.blocks;
 
-        const exec = (): void => convert(existingBlock.id, nonexistingToolName);
-
-        expect(exec).to.throw(`Block Tool with type "${nonexistingToolName}" not found`);
+        return convert(existingBlock.id, nonexistingToolName)
+          .catch((error) => {
+            expect(error.message).to.be.eq(`Block Tool with type "${nonexistingToolName}" not found`);
+          });
       });
     });
 
@@ -340,9 +350,10 @@ describe('api.blocks', () => {
          */
         const { convert } = editor.blocks;
 
-        const exec = (): void => convert(existingBlock.id, 'nonConvertableTool');
-
-        expect(exec).to.throw(`Conversion from "paragraph" to "nonConvertableTool" is not possible. NonConvertableTool tool(s) should provide a "conversionConfig"`);
+        return convert(existingBlock.id, 'nonConvertableTool')
+          .catch((error) => {
+            expect(error.message).to.be.eq(`Conversion from "paragraph" to "nonConvertableTool" is not possible. NonConvertableTool tool(s) should provide a "conversionConfig"`);
+          });
       });
     });
   });

test/cypress/tests/api/caret.cy.ts

@@ -0,0 +1,113 @@
+import EditorJS from '../../../../types';
+
+/**
+ * Test cases for Caret API
+ */
+describe('Caret API', () => {
+  const paragraphDataMock = {
+    id: 'bwnFX5LoX7',
+    type: 'paragraph',
+    data: {
+      text: 'The first block content mock.',
+    },
+  };
+
+  describe('.setToBlock()', () => {
+    /**
+     * The arrange part of the following tests are the same:
+     *  - create an editor
+     *  - move caret out of the block by default
+     */
+    beforeEach(() => {
+      cy.createEditor({
+        data: {
+          blocks: [
+            paragraphDataMock,
+          ],
+        },
+      }).as('editorInstance');
+
+      /**
+       * Blur caret from the block before setting via api
+       */
+      cy.get('[data-cy=editorjs]')
+        .click();
+    });
+
+    it('should set caret to a block (and return true) if block index is passed as argument', () => {
+      cy.get<EditorJS>('@editorInstance')
+        .then(async (editor) => {
+          const returnedValue = editor.caret.setToBlock(0);
+
+          /**
+           * Check that caret belongs block
+           */
+          cy.window()
+            .then((window) => {
+              const selection = window.getSelection();
+              const range = selection.getRangeAt(0);
+
+              cy.get('[data-cy=editorjs]')
+                .find('.ce-block')
+                .first()
+                .should(($block) => {
+                  expect($block[0].contains(range.startContainer)).to.be.true;
+                });
+            });
+
+          expect(returnedValue).to.be.true;
+        });
+    });
+
+    it('should set caret to a block (and return true) if block id is passed as argument', () => {
+      cy.get<EditorJS>('@editorInstance')
+        .then(async (editor) => {
+          const returnedValue = editor.caret.setToBlock(paragraphDataMock.id);
+
+          /**
+           * Check that caret belongs block
+           */
+          cy.window()
+            .then((window) => {
+              const selection = window.getSelection();
+              const range = selection.getRangeAt(0);
+
+              cy.get('[data-cy=editorjs]')
+                .find('.ce-block')
+                .first()
+                .should(($block) => {
+                  expect($block[0].contains(range.startContainer)).to.be.true;
+                });
+            });
+
+          expect(returnedValue).to.be.true;
+        });
+    });
+
+    it('should set caret to a block (and return true) if Block API is passed as argument', () => {
+      cy.get<EditorJS>('@editorInstance')
+        .then(async (editor) => {
+          const block = editor.blocks.getById(paragraphDataMock.id);
+          const returnedValue = editor.caret.setToBlock(block);
+
+          /**
+           * Check that caret belongs block
+           */
+          cy.window()
+            .then((window) => {
+              const selection = window.getSelection();
+              const range = selection.getRangeAt(0);
+
+              cy.get('[data-cy=editorjs]')
+                .find('.ce-block')
+                .first()
+                .should(($block) => {
+                  expect($block[0].contains(range.startContainer)).to.be.true;
+                });
+            });
+
+          expect(returnedValue).to.be.true;
+        });
+    });
+  });
+});