Merge pull request #8324 from penpot/azazeln28-fix-editor-fills

🐛 Fix text editor issues

frontend/src/app/main/data/workspace/texts.cljs

@@ -508,12 +508,12 @@
      ptk/EffectEvent
      (effect [_ state _]
        (when (features/active-feature? state "text-editor/v2")
-         (let [instance (:workspace-editor state)
-               styles   (some-> (editor.v2/getCurrentStyle instance)
-                                (styles/get-styles-from-style-declaration :removed-mixed true)
-                                ((comp update-node-fn migrate-node))
-                                (styles/attrs->styles))]
-           (editor.v2/applyStylesToSelection instance styles)))))))
+         (when-let [instance (:workspace-editor state)]
+           (let [styles   (some-> (editor.v2/getCurrentStyle instance)
+                                  (styles/get-styles-from-style-declaration :removed-mixed true)
+                                  ((comp update-node-fn migrate-node))
+                                  (styles/attrs->styles))]
+             (editor.v2/applyStylesToSelection instance styles))))))))
 
 ;; --- RESIZE UTILS
 
@@ -782,12 +782,12 @@
     ptk/EffectEvent
     (effect [_ state _]
       (when (features/active-feature? state "text-editor/v2")
-        (let [instance (:workspace-editor state)
-              attrs-to-override (some-> (editor.v2/getCurrentStyle instance)
-                                        (styles/get-styles-from-style-declaration))
-              overriden-attrs (merge attrs-to-override attrs)
-              styles  (styles/attrs->styles overriden-attrs)]
-          (editor.v2/applyStylesToSelection instance styles))))))
+        (when-let [instance (:workspace-editor state)]
+          (let [attrs-to-override (some-> (editor.v2/getCurrentStyle instance)
+                                          (styles/get-styles-from-style-declaration))
+                overriden-attrs (merge attrs-to-override attrs)
+                styles  (styles/attrs->styles overriden-attrs)]
+            (editor.v2/applyStylesToSelection instance styles)))))))
 
 (defn update-all-attrs
   [ids attrs]

frontend/text-editor/README.md

@@ -82,6 +82,26 @@ The `TextEditor` contains a series of references to DOM elements, one of them is
 
 `ChangeController` is called by the `TextEditor` instance everytime a change is performed on the content of the `contenteditable` element.
 
+### Best practices
+
+#### Use `isType` functions
+
+Instead of handling elements by their properties like this:
+
+```javascript
+if (element.tagName === "SPAN") {
+  ...
+}
+```
+
+Use functions like `isParagraph`, `isTextSpan` or `isLineBreak`:
+
+```javascript
+if (isTextSpan(element)) {
+  ...
+}
+```
+
 ### Events
 
 - `change`: This event is dispatched every time a change is made in the editor. All changes are debounced to prevent dispatching too many change events. This event is also dispatched when there are pending change events and the user blurs the textarea element.

frontend/text-editor/src/editor/TextEditor.js

@@ -326,9 +326,7 @@ export class TextEditor extends EventTarget {
    * @param {FocusEvent} e
    */
   #onBlur = (e) => {
-    if (!this.isEmpty) {
-      this.#changeController.notifyImmediately();
-    }
+    this.#changeController.notifyImmediately();
     this.#selectionController.saveSelection();
     this.dispatchEvent(new FocusEvent(e.type, e));
   };
@@ -685,7 +683,7 @@ export function createRootFromString(string) {
  * Returns true if the passed object is a TextEditor
  * instance.
  *
- * @param {TextEditor} instance
+ * @param {*} instance
  * @returns {boolean}
  */
 export function isTextEditor(instance) {
@@ -702,8 +700,7 @@ export function isEmpty(instance) {
   if (isTextEditor(instance)) {
     return instance.isEmpty;
   }
-  return null;
-  // throw new TypeError('Instance is not a TextEditor');
+  throw new TypeError('Instance is not a TextEditor');
 }
 
 /**
@@ -718,7 +715,6 @@ export function getRoot(instance) {
     return instance.root;
   }
   return null;
-  // throw new TypeError("Instance is not a TextEditor");
 }
 
 /**
@@ -731,10 +727,9 @@ export function getRoot(instance) {
 export function setRoot(instance, root) {
   if (isTextEditor(instance)) {
     instance.root = root;
-    // return instance;
+    return instance;
   }
-  return instance;
-  // throw new TypeError("Instance is not a TextEditor");
+  throw new TypeError("Instance is not a TextEditor");
 }
 
 /**
@@ -759,8 +754,7 @@ export function getCurrentStyle(instance) {
   if (isTextEditor(instance)) {
     return instance.currentStyle;
   }
-  // throw new TypeError("Instance is not a TextEditor");
-  return null;
+  throw new TypeError('Instance is not a TextEditor');
 }
 
 /**
@@ -775,8 +769,7 @@ export function applyStylesToSelection(instance, styles) {
   if (isTextEditor(instance)) {
     return instance.applyStylesToSelection(styles);
   }
-  // throw new TypeError("Instance is not a TextEditor");
-  return null;
+  throw new TypeError('Instance is not a TextEditor');
 }
 
 /**
@@ -790,8 +783,7 @@ export function dispose(instance) {
   if (isTextEditor(instance)) {
     return instance.dispose();
   }
-  // throw new TypeError("Instance is not a TextEditor");
-  return null;
+  throw new TypeError('Instance is not a TextEditor');
 }
 
 export default TextEditor;

frontend/text-editor/src/editor/content/dom/Style.js

@@ -336,20 +336,22 @@ export function getStyle(element, styleName, styleUnit) {
  * @returns {HTMLElement}
  */
 export function setStylesFromObject(element, allowedStyles, styleObject) {
-  if (element.tagName === "SPAN")
-    for (const [styleName, styleUnit] of allowedStyles) {
-      if (!(styleName in styleObject)) {
-        continue;
-      }
-      let styleValue = styleObject[styleName];
-      if (!styleValue) continue;
-
-      if (styleName === "font-family") {
-        styleValue = sanitizeFontFamily(styleValue);
-      }
-
-      setStyle(element, styleName, styleValue, styleUnit);
+  for (const [styleName, styleUnit] of allowedStyles) {
+    if (!(styleName in styleObject)) {
+      continue;
     }
+
+    let styleValue = styleObject[styleName];
+    if (!styleValue) {
+      continue;
+    }
+
+    if (styleName === "font-family") {
+      styleValue = sanitizeFontFamily(styleValue);
+    }
+
+    setStyle(element, styleName, styleValue, styleUnit);
+  }
   return element;
 }
 

frontend/text-editor/src/editor/controllers/SelectionController.js

@@ -1961,7 +1961,8 @@ export class SelectionController extends EventTarget {
         this.setSelection(newTextSpan.firstChild, 0, newTextSpan.firstChild, 0);
       }
       // The styles are applied to the paragraph
-      else {
+      else
+      {
         const paragraph = this.startParagraph;
         setParagraphStyles(paragraph, newStyles);
         // Apply styles to child text spans.
@@ -1969,11 +1970,9 @@ export class SelectionController extends EventTarget {
           setTextSpanStyles(textSpan, newStyles);
         }
       }
-      return this.#notifyStyleChange();
-
-      // If the startContainer and endContainer are different
-      // then we need to iterate through those nodes to apply
-      // the styles.
+    // If the startContainer and endContainer are different
+    // then we need to iterate through those nodes to apply
+    // the styles.
     } else if (startNode !== endNode) {
       const safeGuard = new SafeGuard("applyStylesTo");
       safeGuard.start();
@@ -2022,12 +2021,12 @@ export class SelectionController extends EventTarget {
         }
 
         // We've reached the final node so we can return safely.
-        if (this.#textNodeIterator.currentNode === expectedEndNode) return;
+        if (this.#textNodeIterator.currentNode === expectedEndNode)
+          break;
 
         this.#textNodeIterator.nextNode();
       } while (this.#textNodeIterator.currentNode);
     }
-
     return this.#notifyStyleChange();
   }