editable.js icon indicating copy to clipboard operation
editable.js copied to clipboard

Add a rewrite callback to Editable.getContent()

Open jenstroeger opened this issue 4 years ago • 6 comments

First, I think the call to extractContent() in the Editable.getContent() function

https://github.com/livingdocsIO/editable.js/blob/58f9a24e39b21a6c501c9fb28ef837d8ca018edc/src/core.js#L270

should actually be

return content.extractContent(element, false)

to ensure that the argument keepUiElements is a boolean as defined in the function’s documentation:

https://github.com/livingdocsIO/editable.js/blob/58f9a24e39b21a6c501c9fb28ef837d8ca018edc/src/content.js#L80

Technically, undefined is a falsey value, but it’s just not clean 🤓

Request

I’d like to modify/clean up the content of the host element before it’s serialized to HTML by passing in a callback function. Without it, getContent() returns a serialized HTML which I need to parse into a DOM element, clean it up, and serialize it again… which isn’t great.

So, for example:

const content = editable.getContent(elem, function(e) { 
    // Modify and clean up `e`  where `e` is the cloned and scrubbed `elem`.
});

If you’re interested in adding a callback function parameter to getContent() then I can open a PR. (It’s not a big change, but quite useful!)

jenstroeger avatar Aug 27 '21 07:08 jenstroeger

We can also just add another method, which is more expressive than some callbacks. For example return a DocumentFragment with editable.getContentFragment(elem).

marcbachmann avatar Sep 08 '21 17:09 marcbachmann

Oh, instead of returning the serialized HTML?

jenstroeger avatar Sep 08 '21 20:09 jenstroeger

@marcbachmann this is what you meant, correct? If you’re ok with the change then I can open a PR.

diff --git a/src/content.js b/src/content.js
index 6039b8e..3f00065 100644
--- a/src/content.js
+++ b/src/content.js
@@ -78,8 +78,8 @@ export function cleanInternals (element) {
 // @param {DOM node or document fragment} Element where to clean out the innerHTML.
 // If you pass a document fragment it will be empty after this call.
 // @param {Boolean} Flag whether to keep ui elements like spellchecking highlights.
-// @returns {String} The cleaned innerHTML of the passed element or document fragment.
-export function extractContent (element, keepUiElements) {
+// @returns {HTMLElement} The cleaned clone of the passed element or document fragment.
+export function extractContentFragment (element, keepUiElements) {
   const innerHtml = (element.nodeType === nodeType.documentFragmentNode
     ? getInnerHtmlOfFragment(element)
     : element.innerHTML
@@ -98,6 +98,19 @@ export function extractContent (element, keepUiElements) {
   // Remove line breaks at the end of a content block
   removeWhitespaces(clone, 'lastChild')
 
+  return clone
+}
+
+// Extracts the content from a host element. Does not touch or change the host.
+// Just returns the content fragment and removes elements marked for removal by
+// editable.
+//
+// @param {DOM node or document fragment} Element where to clean out the innerHTML.
+// If you pass a document fragment it will be empty after this call.
+// @param {Boolean} Flag whether to keep ui elements like spellchecking highlights.
+// @returns {String} The cleaned innerHTML of the passed element or document fragment.
+export function extractContent (element, keepUiElements) {
+  const clone = extractContentFragment(element, keepUiElements)
   return clone.innerHTML
 }
 
diff --git a/src/core.js b/src/core.js
index 54abda3..f40de6f 100644
--- a/src/core.js
+++ b/src/core.js
@@ -267,7 +267,19 @@ export class Editable {
    * @returns {String} The cleaned innerHTML.
    */
   getContent (element) {
-    return content.extractContent(element)
+    return content.extractContent(element, false)
+  }
+
+  /**
+   * Extract the content from an editable host or document fragment.
+   * This method will remove all internal elements and ui-elements.
+   *
+   * @param {DOM node or Document Fragment} The element or fragment whose cleaned up
+   * content fragment is returned.
+   * @returns {HTMLElement} The cloned and cleaned fragment.
+   */
+  getContentFragment (element)  {
+    return content.extractContentFragment(element, false)
   }
 
   /**

jenstroeger avatar Sep 09 '21 05:09 jenstroeger

@peyerluk what do you think here? Return a div, which is easier to operate on or return a DocumentFragment, which supports easy insertion into the dom?

... Or just use the transform callback 😄

marcbachmann avatar Sep 28 '21 16:09 marcbachmann

I think working with a fragment is more in line with other code in this package (which works with fragments) and it avoids creating elements that have no other purpose than wrapping the fragment — in my use case I’d only unwrap the fragment again.

Either way, the boolean function parameter to content.extractContent() should be fixed.

jenstroeger avatar Sep 28 '21 19:09 jenstroeger

I would return a DocumentFragment. I think this is easy enough to work with.

peyerluk avatar Sep 29 '21 10:09 peyerluk