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,48 +288,56 @@ 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
    */
   private move(): void {
     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
         wrapperOffset.top +
         this.toolbarVerticalMargin,
     };
 
-    /**
-     * If we know selections width, place InlineToolbar to center
-     */
-    if (selectionRect.width) {
-      newCoords.x += Math.floor(selectionRect.width / 2);
-    }
-
-
-    /**
-     * 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;
+    const realRightCoord = newCoords.x + this.width + wrapperOffset.x;
 
     /**
-     * By default, Inline Toolbar has top-corner at the center
-     * We are adding a modifiers for to move corner to the left or right
+     * Prevent InlineToolbar from overflowing the content zone on the right side
      */
-    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
-    );
+    if (realRightCoord > this.Editor.UI.contentRect.right) {
+      newCoords.x = this.Editor.UI.contentRect.right - this.width - wrapperOffset.x;
+    }
 
     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,36 +2,17 @@
   --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;
 
   &--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);
+});