fix(inline-toolbar): appearance logic improved (#2550)

* fix(inline-toolbar): appearing logic improved

* tests added

* fix tests

* debounce added

* fix test build in github action

* increase closeTo delta for ff

.github/workflows/cypress.yml

@@ -18,4 +18,4 @@ jobs:
         with:
           config: video=false
           browser: ${{ matrix.browser }}
-          build: yarn build
+          build: yarn build:test

docs/CHANGELOG.md

@@ -11,6 +11,7 @@
 - `Fix` — `blocks.render()` won't lead the `onChange` call in Safari
 - `Fix` — Editor wrapper element growing on the Inline Toolbar close
 - `Fix` — Fix errors thrown by clicks on a document when the editor is being initialized
+- `Fix` — Inline Toolbar sometimes opened in an incorrect position. Now it will be aligned by the left side of the selected text. And won't overflow the right side of the text column.
 
 ### 2.28.2
 

package.json

@@ -14,7 +14,8 @@
   ],
   "scripts": {
     "dev": "vite",
-    "build": "vite build",
+    "build": "vite build --mode production",
+    "build:test": "vite build --mode test",
     "lint": "eslint src/ --ext .ts && yarn lint:tests",
     "lint:errors": "eslint src/ --ext .ts --quiet",
     "lint:fix": "eslint src/ --ext .ts --fix",
@@ -26,8 +27,8 @@
     "_tools:build": "git submodule foreach yarn build",
     "_tools:make": "yarn _tools:yarn && yarn _tools:build",
     "tools:update": "yarn _tools:checkout && yarn _tools:pull && yarn _tools:make",
-    "test:e2e": "yarn build && cypress run",
-    "test:e2e:open": "yarn build && cypress open",
+    "test:e2e": "yarn build:test && cypress run",
+    "test:e2e:open": "yarn build:test && cypress open",
     "devserver:start": "yarn build && node ./devserver.js"
   },
   "author": "CodeX",

src/components/modules/toolbar/inline.ts

@@ -138,15 +138,16 @@ export default class InlineToolbar extends Module<InlineToolbarNodes> {
    *                                  Avoid to use it just for closing IT, better call .close() clearly.
    * @param [needToShowConversionToolbar] - pass false to not to show Conversion Toolbar
    */
-  public tryToShow(needToClose = false, needToShowConversionToolbar = true): void {
-    if (!this.allowedToShow()) {
-      if (needToClose) {
-        this.close();
-      }
+  public async tryToShow(needToClose = false, needToShowConversionToolbar = true): Promise<void> {
+    if (needToClose) {
+      this.close();
+    }
 
+    if (!this.allowedToShow()) {
       return;
     }
 
+    await this.addToolsFiltered(needToShowConversionToolbar);
     this.move();
     this.open(needToShowConversionToolbar);
     this.Editor.Toolbar.close();
@@ -187,51 +188,6 @@ export default class InlineToolbar extends Module<InlineToolbarNodes> {
     this.Editor.ConversionToolbar.close();
   }
 
-  /**
-   * Shows Inline Toolbar
-   *
-   * @param [needToShowConversionToolbar] - pass false to not to show Conversion Toolbar
-   */
-  public open(needToShowConversionToolbar = true): void {
-    if (this.opened) {
-      return;
-    }
-    /**
-     * Filter inline-tools and show only allowed by Block's Tool
-     */
-    this.addToolsFiltered();
-
-    /**
-     * Show Inline Toolbar
-     */
-    this.nodes.wrapper.classList.add(this.CSS.inlineToolbarShowed);
-
-    this.buttonsList = this.nodes.buttons.querySelectorAll(`.${this.CSS.inlineToolButton}`);
-    this.opened = true;
-
-    if (needToShowConversionToolbar && this.Editor.ConversionToolbar.hasTools()) {
-      /**
-       * Change Conversion Dropdown content for current tool
-       */
-      this.setConversionTogglerContent();
-    } else {
-      /**
-       * hide Conversion Dropdown with there are no tools
-       */
-      this.nodes.conversionToggler.hidden = true;
-    }
-
-    /**
-     * Get currently visible buttons to pass it to the Flipper
-     */
-    let visibleTools = Array.from(this.buttonsList);
-
-    visibleTools.unshift(this.nodes.conversionToggler);
-    visibleTools = visibleTools.filter((tool) => !(tool as HTMLElement).hidden);
-
-    this.flipper.activate(visibleTools as HTMLElement[]);
-  }
-
   /**
    * Check if node is contained by Inline Toolbar
    *
@@ -268,6 +224,11 @@ export default class InlineToolbar extends Module<InlineToolbarNodes> {
       this.CSS.inlineToolbar,
       ...(this.isRtl ? [ this.Editor.UI.CSS.editorRtlFix ] : []),
     ]);
+
+    if (import.meta.env.MODE === 'test') {
+      this.nodes.wrapper.setAttribute('data-cy', 'inline-toolbar');
+    }
+
     /**
      * Creates a different wrapper for toggler and buttons.
      */
@@ -327,6 +288,33 @@ export default class InlineToolbar extends Module<InlineToolbarNodes> {
     this.enableFlipper();
   }
 
+  /**
+   * Shows Inline Toolbar
+   */
+  private open(): void {
+    if (this.opened) {
+      return;
+    }
+
+    /**
+     * Show Inline Toolbar
+     */
+    this.nodes.wrapper.classList.add(this.CSS.inlineToolbarShowed);
+
+    this.buttonsList = this.nodes.buttons.querySelectorAll(`.${this.CSS.inlineToolButton}`);
+    this.opened = true;
+
+    /**
+     * Get currently visible buttons to pass it to the Flipper
+     */
+    let visibleTools = Array.from(this.buttonsList);
+
+    visibleTools.unshift(this.nodes.conversionToggler);
+    visibleTools = visibleTools.filter((tool) => !(tool as HTMLElement).hidden);
+
+    this.flipper.activate(visibleTools as HTMLElement[]);
+  }
+
   /**
    * Move Toolbar to the selected text
    */
@@ -334,7 +322,7 @@ export default class InlineToolbar extends Module<InlineToolbarNodes> {
     const selectionRect = SelectionUtils.rect as DOMRect;
     const wrapperOffset = this.Editor.UI.nodes.wrapper.getBoundingClientRect();
     const newCoords = {
-      x: selectionRect.x - wrapperOffset.left,
+      x: selectionRect.x - wrapperOffset.x,
       y: selectionRect.y +
         selectionRect.height -
         // + window.scrollY
@@ -342,34 +330,15 @@ export default class InlineToolbar extends Module<InlineToolbarNodes> {
         this.toolbarVerticalMargin,
     };
 
+    const realRightCoord = newCoords.x + this.width + wrapperOffset.x;
+
     /**
-     * If we know selections width, place InlineToolbar to center
+     * Prevent InlineToolbar from overflowing the content zone on the right side
      */
-    if (selectionRect.width) {
-      newCoords.x += Math.floor(selectionRect.width / 2);
+    if (realRightCoord > this.Editor.UI.contentRect.right) {
+      newCoords.x = this.Editor.UI.contentRect.right - this.width - wrapperOffset.x;
     }
 
-
-    /**
-     * Inline Toolbar has -50% translateX, so we need to check real coords to prevent overflowing
-     */
-    const realLeftCoord = newCoords.x - this.width / 2;
-    const realRightCoord = newCoords.x + this.width / 2;
-
-    /**
-     * By default, Inline Toolbar has top-corner at the center
-     * We are adding a modifiers for to move corner to the left or right
-     */
-    this.nodes.wrapper.classList.toggle(
-      this.CSS.inlineToolbarLeftOriented,
-      realLeftCoord < this.Editor.UI.contentRect.left
-    );
-
-    this.nodes.wrapper.classList.toggle(
-      this.CSS.inlineToolbarRightOriented,
-      realRightCoord > this.Editor.UI.contentRect.right
-    );
-
     this.nodes.wrapper.style.left = Math.floor(newCoords.x) + 'px';
     this.nodes.wrapper.style.top = Math.floor(newCoords.y) + 'px';
   }
@@ -529,8 +498,10 @@ export default class InlineToolbar extends Module<InlineToolbarNodes> {
 
   /**
    * Append only allowed Tools
+   *
+   * @param {boolean} needToShowConversionToolbar - pass false to not to show Conversion Toolbar (e.g. for Footnotes-like tools)
    */
-  private addToolsFiltered(): void {
+  private async addToolsFiltered(needToShowConversionToolbar = true): Promise<void> {
     const currentSelection = SelectionUtils.get();
     const currentBlock = this.Editor.BlockManager.getBlock(currentSelection.anchorNode as HTMLElement);
 
@@ -545,6 +516,18 @@ export default class InlineToolbar extends Module<InlineToolbarNodes> {
       this.addTool(tool);
     });
 
+    if (needToShowConversionToolbar && this.Editor.ConversionToolbar.hasTools()) {
+      /**
+       * Change Conversion Dropdown content for current tool
+       */
+      await this.setConversionTogglerContent();
+    } else {
+      /**
+       * hide Conversion Dropdown with there are no tools
+       */
+      this.nodes.conversionToggler.hidden = true;
+    }
+
     /**
      * Recalculate width because some buttons can be hidden
      */

src/components/modules/ui.ts

@@ -350,9 +350,12 @@ export default class UI extends Module<UINodes> {
     /**
      * Handle selection change to manipulate Inline Toolbar appearance
      */
-    this.readOnlyMutableListeners.on(document, 'selectionchange', () => {
+    const selectionChangeDebounceTimeout = 180;
+    const selectionChangeDebounced = _.debounce(() => {
       this.selectionChanged();
-    }, true);
+    }, selectionChangeDebounceTimeout);
+
+    this.readOnlyMutableListeners.on(document, 'selectionchange', selectionChangeDebounced, true);
 
     this.readOnlyMutableListeners.on(window, 'resize', () => {
       this.resizeDebouncer();
@@ -860,9 +863,6 @@ export default class UI extends Module<UINodes> {
 
     const isNeedToShowConversionToolbar = clickedOutsideBlockContent !== true;
 
-    /**
-     * @todo add debounce
-     */
     this.Editor.InlineToolbar.tryToShow(true, isNeedToShowConversionToolbar);
   }
 }

src/env.d.ts

@@ -0,0 +1,11 @@
+interface ImportMetaEnv {
+  /**
+   * Build environment.
+   * For example, used to detect building for tests and add "data-cy" attributes for DOM querying.
+   */
+  readonly MODE: "test" | "development" | "production";
+}
+
+interface ImportMeta {
+  readonly env: ImportMetaEnv;
+}

src/styles/inline-toolbar.css

@@ -2,11 +2,10 @@
   --y-offset: 8px;
 
   @apply --overlay-pane;
-  transform: translateX(-50%) translateY(8px) scale(0.94);
   opacity: 0;
   visibility: hidden;
-  transition: transform 150ms ease, opacity 250ms ease;
-  will-change: transform, opacity;
+  transition: opacity 250ms ease;
+  will-change: opacity, left, top;
   top: 0;
   left: 0;
   z-index: 3;
@@ -14,24 +13,6 @@
   &--showed {
     opacity: 1;
     visibility: visible;
-    transform: translateX(-50%)
-  }
-
-  &--left-oriented {
-    transform: translateX(-23px) translateY(8px) scale(0.94);
-  }
-
-  &--left-oriented&--showed {
-    transform: translateX(-23px);
-  }
-
-  &--right-oriented {
-    transform: translateX(-100%) translateY(8px) scale(0.94);
-    margin-left: 23px;
-  }
-
-  &--right-oriented&--showed {
-    transform: translateX(-100%);
   }
 
   [hidden] {

test/cypress/support/commands.ts

@@ -155,3 +155,82 @@ Cypress.Commands.add('selectText', {
 
   return cy.wrap(subject);
 });
+
+/**
+ * Select element's text by offset
+ * Note. Previous subject should have 'textNode' as firstChild
+ *
+ * Usage
+ * cy.get('[data-cy=editorjs]')
+ *  .find('.ce-paragraph')
+ *  .selectTextByOffset([0, 5])
+ *
+ * @param offset - offset to select
+ */
+Cypress.Commands.add('selectTextByOffset', {
+  prevSubject: true,
+}, (subject, offset: [number, number]) => {
+  const el = subject[0];
+  const document = el.ownerDocument;
+  const range = document.createRange();
+  const textNode = el.firstChild;
+  const selectionPositionStart = offset[0];
+  const selectionPositionEnd = offset[1];
+
+  range.setStart(textNode, selectionPositionStart);
+  range.setEnd(textNode, selectionPositionEnd);
+  document.getSelection().removeAllRanges();
+  document.getSelection().addRange(range);
+
+  return cy.wrap(subject);
+});
+
+/**
+ * Returns line wrap positions for passed element
+ *
+ * Usage
+ * cy.get('[data-cy=editorjs]')
+ *  .find('.ce-paragraph')
+ *  .getLineWrapPositions()
+ *
+ * @returns number[] - array of line wrap positions
+ */
+Cypress.Commands.add('getLineWrapPositions', {
+  prevSubject: true,
+}, (subject) => {
+  const element = subject[0];
+  const document = element.ownerDocument;
+  const text = element.textContent;
+  const lineWraps = [];
+
+  let currentLineY = 0;
+
+  /**
+   * Iterate all chars in text, create range for each char and get its position
+   */
+  for (let i = 0; i < text.length; i++) {
+    const range = document.createRange();
+
+    range.setStart(element.firstChild, i);
+    range.setEnd(element.firstChild, i);
+
+    const rect = range.getBoundingClientRect();
+
+    if (i === 0) {
+      currentLineY = rect.top;
+
+      continue;
+    }
+
+    /**
+     * If current char Y position is higher than previously saved line Y, that means a line wrap
+     */
+    if (rect.top > currentLineY) {
+      lineWraps.push(i);
+
+      currentLineY = rect.top;
+    }
+  }
+
+  return cy.wrap(lineWraps);
+});

test/cypress/support/index.d.ts

@@ -60,6 +60,31 @@ declare global {
        * @param text - text to select
        */
       selectText(text: string): Chainable<Subject>;
+
+      /**
+       * Select element's text by offset
+       * Note. Previous subject should have 'textNode' as firstChild
+       *
+       * Usage
+       * cy.get('[data-cy=editorjs]')
+       *  .find('.ce-paragraph')
+       *  .selectTextByOffset([0, 5])
+       *
+       * @param offset - offset to select
+       */
+      selectTextByOffset(offset: [number, number]): Chainable<Subject>;
+
+      /**
+       * Returns line wrap positions for passed element
+       *
+       * Usage
+       * cy.get('[data-cy=editorjs]')
+       *  .find('.ce-paragraph')
+       *  .getLineWrapPositions()
+       *
+       * @returns number[] - array of line wrap positions
+       */
+      getLineWrapPositions(): Chainable<number[]>;
     }
 
     interface ApplicationWindow {

test/cypress/tests/inline-tools/link.cy.ts

@@ -17,6 +17,7 @@ describe('Inline Tool Link', () => {
       .find('div.ce-block')
       .click()
       .type('{selectall}')
+      .wait(200)
       .type('{ctrl}K');
 
     cy.get('[data-cy=editorjs]')
@@ -30,4 +31,44 @@ describe('Inline Tool Link', () => {
       .find('a')
       .should('have.attr', 'href', 'https://codex.so');
   });
+
+  it('should remove fake background on selection change', () => {
+    cy.createEditor({
+      data: {
+        blocks: [
+          {
+            type: 'paragraph',
+            data: {
+              text: 'First block text',
+            },
+          },
+          {
+            type: 'paragraph',
+            data: {
+              text: 'Second block text',
+            },
+          },
+        ],
+      },
+    });
+
+    cy.get('[data-cy=editorjs]')
+      .find('div.ce-block')
+      .first()
+      .click()
+      .type('{selectall}')
+      .wait(200)
+      .type('{ctrl}K');
+
+    cy.get('[data-cy=editorjs]')
+      .find('div.ce-block')
+      .last()
+      .click()
+      .type('{selectall}')
+      .wait(200);
+
+    cy.get('[data-cy=editorjs]')
+      .find('.ce-paragraph span[style]')
+      .should('not.exist');
+  });
 });

test/cypress/tests/modules/InlineToolbar.cy.ts

@@ -0,0 +1,76 @@
+describe('Inline Toolbar', () => {
+  it('should appear aligned with left coord of selection rect', () => {
+    cy.createEditor({
+      data: {
+        blocks: [
+          {
+            type: 'paragraph',
+            data: {
+              text: 'First block text',
+            },
+          },
+        ],
+      },
+    });
+
+    cy.get('[data-cy=editorjs]')
+      .find('.ce-paragraph')
+      .selectText('block');
+
+    cy.get('[data-cy="inline-toolbar"]')
+      .should('be.visible')
+      .then(($toolbar) => {
+        const editorWindow = $toolbar.get(0).ownerDocument.defaultView;
+        const selection = editorWindow.getSelection();
+
+        const range = selection.getRangeAt(0);
+        const rect = range.getBoundingClientRect();
+
+        expect($toolbar.offset().left).to.closeTo(rect.left, 1);
+      });
+  });
+
+  it('should appear aligned with right side of text column when toolbar\'s width is not fit at right', () => {
+    cy.createEditor({
+      data: {
+        blocks: [
+          {
+            type: 'paragraph',
+            data: {
+              text: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec a diam lectus. Sed sit amet ipsum mauris. Maecenas congue ligula ac quam viverra nec consectetur ante hendrerit. Donec et mollis dolor.',
+            },
+          },
+        ],
+      },
+    });
+
+    cy.get('[data-cy=editorjs]')
+      .find('.ce-paragraph')
+      .as('blockWrapper')
+      .getLineWrapPositions()
+      .then((lineWrapIndexes) => {
+        const firstLineWrapIndex = lineWrapIndexes[0];
+
+        /**
+         * Select last 5 chars of the first line
+         */
+        cy.get('[data-cy=editorjs]')
+          .find('.ce-paragraph')
+          .selectTextByOffset([firstLineWrapIndex - 5, firstLineWrapIndex - 1]);
+      });
+
+    cy.get('[data-cy="inline-toolbar"]')
+      .should('be.visible')
+      .then(($toolbar) => {
+        cy.get('@blockWrapper')
+          .then(($blockWrapper) => {
+            const blockWrapperRect = $blockWrapper.get(0).getBoundingClientRect();
+
+            /**
+             * Toolbar should be aligned with right side of text column
+             */
+            expect($toolbar.offset().left + $toolbar.width()).to.closeTo(blockWrapperRect.right, 3);
+          });
+      });
+  });
+});

test/cypress/tests/utils/flipper.cy.ts

@@ -64,7 +64,8 @@ describe('Flipper', () => {
     cy.get('[data-cy=editorjs]')
       .get('.cdx-some-plugin')
       .focus()
-      .type(sampleText);
+      .type(sampleText)
+      .wait(100);
 
     // Try to delete the block via keyboard
     cy.get('[data-cy=editorjs]')